@deadpin I will review this PR in the next couple of days. My apologies for the delay.
@makowalski If you don't mind, I'll be updating this PR to address my feedback above and to add in code to read velocity and width data.
Sure, that would be fine. My apologies for the delay…
I committed the animation changes and also addressed the comments by @deadpin.
I'm approving this pull request now, as I'll be out all of next week.
I'm fine with either "Import Custom Properties" or "Import Custom Attributes". I don't feel strongly about either option.
I'll resolve this conversation as I'm fine with always adding the namespace.
It would be cleaner if we remove the add_properties_namespace
option if it's always true, but I don't feel strongly…
Do you think I should flip the order and prefer the real Velocities data first?
I don't see any guideline about this one way or the other. The way you have it now is good, in my opinion.
I think const
is necessary in the function definition but not the declaration, as per this guideline:
FYI, I will work on merging in the animation-related changes today. Will also try to address the comments by @deadpin today.
There is some code that does this for primvar:normals as well but I don't know the history of why that was done or why the preferred data is taken from the primvar instead of the real…
That comment is not accurate, as the properties are not deprecated, as you say. I think this was just an oversight.
An immediate use for these properties could be in the USD export hook, to…
Ah, author_blender_name
is there, indeed. Not sure how my text search missed it. Thanks for the correction.
You're right that adding the userProperties
namespace is standard, but I do…