@CharlesWardlaw I'm done with the revisions I had in mind and have a couple of questions/comments. I welcome your feedback. Thanks!
I don't believe 'add_properties_namespace' and 'author_blender_name' are not actually exposed in the UI. I wonder if they should be? For example, I can imagine cases where automatically adding the 'userProperties' namespace might not be desirable, so maybe this behavior should be optional. I'm open to other opinions about this, of course.
I wonder whether we should rename this "Import Custom Attributes" to make it very clear that this only applies to custom attributes and not primvars. Likewise, maybe the related enums should have "custom" in the name (e.g., rename USD_ATTR_IMPORT_NONE to USD_CUSTOM_ATTR_IMPORT_NONE). But am open to feedback on this, as maybe this is already clear.
@deadpin I just removed the WIP tag, as I believe this PR is now ready for review.
IMHO, using std::optional
here and elsewhere might not be necessary as it can be easily replaced with parameter pxr::UsdTimeCode motionSampleTime = pxr::UsdTimeCode::Default()
. I'll make this change, but am certainly open to discussing this and we can change it back if necessary.
I noticed that this call to set_props()
in create_object()
here and in the other reader implementations can cause a bug, in that if use_parent_xform() evaluates to false then the prim properties are duplicated on both the object and its data. E.g., if you load the custom_attr.usda
you can see the duplicated properties.
Added support for bool, asset, half and arbitrary tuple types on import and bool type on export.
@Matt-McLin @deadpin I can probably merge in my proposed changes for the animating attributes into this PR this week. I'll add a comment here when I've started working on this, so we don't…
Per discussion with @CharlesWardlaw, I'm extending the current implementation to handle a wider range of USD attribute types. I will mark the pull request as WIP until I've completed these changes.