Michael Kowalski makowalski
  • Joined on 2020-09-17
Michael Kowalski commented on pull request blender/blender#116111 2023-12-13 19:19:39 +01:00
bugfix/ci_update

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…
Michael Kowalski approved blender/blender#116078 2023-12-12 12:21:50 +01:00
USD: Apply MaterialBindingAPI to Curves with materials

The change looks good. Thank you for the fix.

Michael Kowalski commented on pull request blender/blender#115808 2023-12-11 23:19:04 +01:00
USD: Automatically Set displayName metadata on Objects, Meshes, and Materials at Export if they contain non-ASCII characters

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…

Michael Kowalski approved blender/blender#115397 2023-12-09 02:50:23 +01:00
Hydra: Unify image writing code between USD and Hydra

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.

Michael Kowalski commented on pull request blender/blender#115808 2023-12-09 00:29:19 +01:00
USD: Automatically Set displayName metadata on Objects, Meshes, and Materials at Export if they contain non-ASCII characters

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.

Michael Kowalski commented on pull request blender/blender#115808 2023-12-09 00:29:18 +01:00
USD: Automatically Set displayName metadata on Objects, Meshes, and Materials at Export if they contain non-ASCII characters

I think you can make the argument a const Object *object, instead of a const void *. That way, you can just return

Michael Kowalski commented on pull request blender/blender#115808 2023-12-09 00:29:18 +01:00
USD: Automatically Set displayName metadata on Objects, Meshes, and Materials at Export if they contain non-ASCII characters

If we keep the switch statement, we need to handle the type OB_MBALL as well.

Michael Kowalski commented on pull request blender/blender#115808 2023-12-08 18:57:42 +01:00
USD: Automatically Set displayName metadata on Objects, Meshes, and Materials at Export if they contain non-ASCII characters

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…
Michael Kowalski commented on pull request blender/blender#115808 2023-12-08 00:10:28 +01:00
USD: Automatically Set displayName metadata on Objects, Meshes, and Materials at Export if they contain non-ASCII characters

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.

Michael Kowalski commented on pull request blender/blender#115808 2023-12-07 23:57:05 +01:00
USD: Automatically Set displayName metadata on Objects, Meshes, and Materials at Export if they contain non-ASCII characters

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

Michael Kowalski commented on pull request blender/blender#115808 2023-12-07 21:39:41 +01:00
USD: Automatically Set displayName metadata on Objects, Meshes, and Materials at Export if they contain non-ASCII characters

I like the approach and exporting the examples works well! I will have some comments and suggestions about the code, to follow.

Michael Kowalski commented on pull request blender/blender#115397 2023-12-07 01:12:17 +01:00
Hydra: Unify image writing code between USD and Hydra

I will get to this review this week. My apologies for the delay.

Michael Kowalski commented on pull request blender/blender#115623 2023-12-06 22:23:48 +01:00
Alembic/USD: use geometry sets to import data

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.

Michael Kowalski pushed to main at blender/blender 2023-12-06 18:18:07 +01:00
b7fa45b68d Fix: USD import: Improve texture color space choice
Michael Kowalski merged pull request blender/blender#113374 2023-12-06 18:18:07 +01:00
bugfix/usd_material_color_space
Michael Kowalski commented on pull request blender/blender#113374 2023-12-06 16:25:21 +01:00
bugfix/usd_material_color_space

@CharlesWardlaw Would you mind running make format on the code, please?

Otherwise, the changes look good to me. Thank you!

Michael Kowalski commented on pull request blender/blender#113374 2023-12-06 15:40:39 +01:00
bugfix/usd_material_color_space