The changes look good! However, I still get errors in the python USD tests. E.g., when I run
ctest --output-on-failure -C Release -R usd
in the build directory, I get
C:\Users\mak…
The change looks good. Thank you for the fix.
You are solving a tricky problem and I had to give it some thought.
I like the core idea of your solution a lot. Good stuff!
However, I think we might need to adjust the implementation…
The changes related to usd_writer_material.cc look good to me, so I will approve this. However, I am not the right person to review the hydra related changes in depth, so I leave that to the other reviewers. Thank you.
A few more comments, which we can discuss on Monday.
I think it's possible that this won't account for name clashes with existing objects in the scene that don't have to be renamed. E.g., if he token is "Mesh" and there is already a "Mesh" object in the scene but not in computed_names_
, then this function will return "Mesh" as the unique name and you might get a name clash in the USD prim path.
I think you can make the argument a const Object *object
, instead of a const void *
. That way, you can just return
If we keep the switch statement, we need to handle the type OB_MBALL as well.
is_data
is not used.
To add to that, if we were to use this approach, we would need to guard against null data, maybe assuming that this is an Empty, in that case:
const ID *id = static_cast<ID *>(object->data);
c…
We can discuss, but it might be preferable to keep the const
qualifier on the functions here and below and to instead declare the members that are being changed mutable
.
I believe you can look up the data type name with the function BKE_idtype_idcode_to_name()
, so you could replace invoking the function with the switch statement with the direct call
I like the approach and exporting the examples works well! I will have some comments and suggestions about the code, to follow.
I will get to this review this week. My apologies for the delay.
I finished an initial review (focusing on the USD code) and the changes look good. Loading test cases works as expected. I hope to finish the review/testing tomorrow.
@CharlesWardlaw Would you mind running make format
on the code, please?
Otherwise, the changes look good to me. Thank you!