-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix compatibility for AbcCoreOgawa::ReadArchive with Alembic>=1.7.9 #825
Fix compatibility for AbcCoreOgawa::ReadArchive with Alembic>=1.7.9 #825
Conversation
Filed as internal issue #USD-5231 |
#if ALEMBIC_LIBRARY_VERSION >= 10709 | ||
TF_DEFINE_ENV_SETTING( | ||
USD_ABC_READ_ARCHIVE_USE_MMAP, false, | ||
"Collapse Xforms containing a single geometry into a single geom Prim in USD"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please update the help string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to have let this one slip. Fix is pushed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks :)
@@ -95,6 +95,12 @@ TF_DEFINE_ENV_SETTING( | |||
USD_ABC_XFORM_PRIM_COLLAPSE, true, | |||
"Collapse Xforms containing a single geometry into a single geom Prim in USD"); | |||
|
|||
#if ALEMBIC_LIBRARY_VERSION >= 10709 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about updating FindAlembic to generate this automatically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now we would need to define ALEMBIC_LIBRARY_VERSION
manually but the FindAlembic cmake module could have some logic included to set it automatically when it finds the libraries. Just an idea though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm mistaken ALEMBIC_LIBRARY_VERSION is defined in the Alembic headers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, you're right. https://github.com/alembic/alembic/blob/master/lib/Alembic/Util/Config.h.in#L42 perfect :)
…mbic Fix compatibility for AbcCoreOgawa::ReadArchive with Alembic>=1.7.9 (Internal change: 1973396)
Description of Change(s)
Alembic 1.7.9 introduced an API change in
Alembic::AbcCoreOgawa::ReadArchive
: a new boolean argument is required. This PR conforms to the new function signature for Alembic >= 1.7.9The behavior of
Alembic::AbcCoreOgawa::ReadArchive
can be configured by a new environment variable:USD_ABC_READ_ARCHIVE_USE_MMAP
By default,
Alembic::AbcCoreOgawa::ReadArchive
keep on using the pre 1.7.9 behaviorFixes Issue(s)