USD Export: Adding the ability to choose Stage Up Axis at export time. #121226

Charles Wardlaw · 2024-04-29T21:13:44+02:00

Charles Wardlaw commented

2024-04-29 21:13:44 +02:00

This patch allows the user to choose the Stage Up Axis at export time, allowing the export to match another target software package. The most common is reorienting to Y up. The up axis is written to stage metadata and a top-level orientation is applied to reorient objects in the Stage.

Charles Wardlaw added 1 commit 2024-04-29 21:13:56 +02:00

924c94c321

USD Export: Adding the ability to choose Stage Up Axis at export time.

Dhruv Govil commented

2024-04-29 22:17:01 +02:00

First-time contributor

This PR is really appreciated. The difference in up axis between DCC's is one of the big pain points our artists and consumers hit when trying to integrate Blender into an existing USD workflow.

Jesse Yurkovich commented

2024-04-29 22:19:34 +02:00

Blender already authors its upAxis USD document setting. Is that not enough for other DCCs to respect? During import we re-orient based on that value and that's what I thought it's there for? The description should probably explain why this isn't enough.

Dhruv Govil commented

2024-04-29 23:51:45 +02:00

First-time contributor

The issue is when the file is integrated with USD composition. USD doesn't implicitly re-orient referenced layers for you when the axis metadata is misaligned.

So for example, if I predominantly have a Y up pipeline, when I reference in a Blender authored file, it'll come in sideways.
Some apps do try and intercept this by trying to apply a transform onto the target prim that is referencing in the Blender asset, but given that composition is unbounded, this can only be done in very specific cases.

This gets extra confusing for some folks because they'll import an asset into Blender for a quick edit and think it's facing the right way up. If this asset is being referenced in an existing environment, the resulting export will be flipped sideways. This makes it harder for creators to transition parts of their pipeline to Blender.

A specific case recently that came up was a customer using Houdini to procedurally place geometry around their environment. Each geometry points to an individual file that artists can customize from there. Artists who'd use Blender would end up breaking the pipeline accidentally.

The issue is when the file is integrated with USD composition. USD doesn't implicitly re-orient referenced layers for you when the axis metadata is misaligned. So for example, if I predominantly have a Y up pipeline, when I reference in a Blender authored file, it'll come in sideways. Some apps do try and intercept this by trying to apply a transform onto the target prim that is referencing in the Blender asset, but given that composition is unbounded, this can only be done in very specific cases. This gets extra confusing for some folks because they'll import an asset into Blender for a quick edit and think it's facing the right way up. If this asset is being referenced in an existing environment, the resulting export will be flipped sideways. This makes it harder for creators to transition parts of their pipeline to Blender. A specific case recently that came up was a customer using Houdini to procedurally place geometry around their environment. Each geometry points to an individual file that artists can customize from there. Artists who'd use Blender would end up breaking the pipeline accidentally.

Jesse Yurkovich commented

2024-04-30 00:05:14 +02:00

Is this not fixable in Houdini or the USD spec itself? Why even allow z-up otherwise? Is this worth a thread on AOUSD or is there already a thread maybe?

Dhruv Govil commented

2024-04-30 00:26:18 +02:00

First-time contributor

The stage metadata is not meant to reinterpret content through composition because of potential performance concerns, and how it might play through layers of composition. e.g someone overlays just the points of a mesh into a prim, but gets the axes and units wrong, they'd incur a fairly heavy cost.

The USD recommendation is that pipelines conform to a single setup or explicitly transform to mitigate it. I agree that this is not necessarily intuitive.

Houdini cannot fix the issue either unless a pipeline mandates that every edit then have to go through Houdini again to fix it. But if you take the example where Houdini has statically created the environment, Houdini itself has no knowledge when someone modifies the files after they're created. In that scenario, Houdini may never come up again in the pipeline for the environment.

I believe the FBX exporter in Blender also does a similar axis selection (https://docs.blender.org/manual/en/2.80/addons/io_scene_fbx.html#:~:text=Blender%20uses%20Y%20Forward%2C%20Z,Forward%2C%20Y%20Up%20is%20needed.) for a similar reason (albeit, its an easier problem to solve in FBX due to the lack of composition)

The stage metadata is not meant to reinterpret content through composition because of potential performance concerns, and how it might play through layers of composition. e.g someone overlays just the points of a mesh into a prim, but gets the axes and units wrong, they'd incur a fairly heavy cost. The USD recommendation is that pipelines conform to a single setup or explicitly transform to mitigate it. I agree that this is not necessarily intuitive. Houdini cannot fix the issue either unless a pipeline mandates that every edit then have to go through Houdini again to fix it. But if you take the example where Houdini has statically created the environment, Houdini itself has no knowledge when someone modifies the files after they're created. In that scenario, Houdini may never come up again in the pipeline for the environment. I believe the FBX exporter in Blender also does a similar axis selection (https://docs.blender.org/manual/en/2.80/addons/io_scene_fbx.html#:~:text=Blender%20uses%20Y%20Forward%2C%20Z,Forward%2C%20Y%20Up%20is%20needed.) for a similar reason (albeit, its an easier problem to solve in FBX due to the lack of composition)

Jesse Yurkovich requested changes 2024-05-03 10:30:56 +02:00

Dismissed

source/blender/editors/io/io_usd.cc Outdated

						
				@ -213,0 +214,4 @@

				  const bool convert_orientation = RNA_boolean_get(op->ptr, "convert_orientation");

				  int global_forward = RNA_enum_get(op->ptr, "export_global_forward_selection");

				  int global_up = RNA_enum_get(op->ptr, "export_global_up_selection");

Jesse Yurkovich commented

2024-05-03 09:17:42 +02:00

Make these 2 vars const as well.

CharlesWardlaw marked this conversation as resolved

source/blender/editors/io/io_usd.cc Outdated

						
				@ -259,6 +268,11 @@ static void wm_usd_export_draw(bContext *C, wmOperator *op)

				    col = uiLayoutColumn(box, true);

				    uiItemR(col, ptr, "selected_objects_only", UI_ITEM_NONE, nullptr, ICON_NONE);

				    uiItemR(col, ptr, "visible_objects_only", UI_ITEM_NONE, nullptr, ICON_NONE);

				    uiItemR(col, ptr, "convert_orientation", UI_ITEM_NONE, nullptr, ICON_NONE);

Jesse Yurkovich commented

2024-05-03 09:55:21 +02:00

This needs to be moved outside the if conditional here as we want them drawn normally, for Collection Export, in all situations. Maybe shift them down towards the root_prim_path option?

This needs to be moved outside the if conditional here as we want them drawn normally, for Collection Export, in all situations. Maybe shift them down towards the `root_prim_path` option?

CharlesWardlaw marked this conversation as resolved

source/blender/editors/io/io_usd.cc Outdated

						
				@ -459,0 +493,4 @@

				                  false,

				                  "Convert Orientation to Y Up",

				                  "When checked, the USD exporter will convert orientation axis to Y up, matching "

				                  "other applications");

Jesse Yurkovich commented

2024-05-03 09:53:03 +02:00

The can just say "Convert Orientation" since it allows more than just Y Up. For the description maybe "Convert orientation axis to a different convention to match other applications"

Jesse Yurkovich commented

2024-05-06 21:49:06 +02:00

Seems was missed? We had prior UI module feedback for removing the "When checked" type wording in these so just noting again.

Charles Wardlaw commented

2024-05-06 22:12:56 +02:00

This is the first I'm hearing of the "when checked" language removal-- taking it out now.

CharlesWardlaw marked this conversation as resolved

source/blender/editors/io/io_usd.cc Outdated

						
				@ -459,0 +508,4 @@

				                      IO_AXIS_Y,

				                      "Up Axis",

				                      "Global Up axis for export");

				  RNA_def_property_update_runtime(prop, up_axis_update);

Jesse Yurkovich commented

2024-05-03 09:52:44 +02:00

Our other exporters don't bother with the "Global Up axis for export" type messages. It's probably ok to leave the descriptions blank here too.

CharlesWardlaw marked this conversation as resolved

source/blender/io/usd/intern/usd_writer_transform.cc Outdated

						
				@ -14,3 +16,4 @@

				namespace blender::io::usd {

				static const float UNIT_M4[4][4] = {

Jesse Yurkovich commented

2024-05-03 10:29:55 +02:00

Can you move this inside do_write and make it constexpr instead of static const.

Can you move this inside `do_write` and make it `constexpr` instead of static const.

CharlesWardlaw marked this conversation as resolved

source/blender/io/usd/intern/usd_writer_transform.cc

						
				@ -19,0 +49,4 @@

				    return false;

				  }

				  if (BLI_strnlen(usd_export_context_.export_params.root_prim_path, 1024) != 0) {

Jesse Yurkovich commented

2024-05-03 10:12:09 +02:00

We've been standardizing on just checking root_prim_path[0] != '\0' instead of doing a length check for most strings like this across blender.

We've been standardizing on just checking `root_prim_path[0] != '\0'` instead of doing a length check for most strings like this across blender.

CharlesWardlaw marked this conversation as resolved

source/blender/io/usd/usd.hh Outdated

						
				@ -25,6 +25,24 @@ struct GeometrySet;

				namespace blender::io::usd {

				typedef enum USD_global_forward_axis {

Jesse Yurkovich commented

2024-05-03 10:07:08 +02:00

Are these enums necessary? They're pretty much the same as eIOAxis inside IO_orientation.hh. Can also see how they're used in the other exporters.

Are these enums necessary? They're pretty much the same as `eIOAxis` inside `IO_orientation.hh`. Can also see how they're used in the other exporters.

CharlesWardlaw marked this conversation as resolved

Jesse Yurkovich commented

2024-05-03 10:32:37 +02:00

Are the "recommendations" consolidated somewhere? It would be helpful if they were so we could mention the scenario in our commit description etc.

That said it does look like there's some precedent out there. I feel a bit better about this now. One software, also z-up, has a direct option in its export settings. Another allows to set it indirectly through it's own preferences. And another does not have this as an option at all currently, but does have an open bug to do something about it.

@DhruvGovil Does the solution here match what you'd expect? Basically the structure of the USD would now be as follows:

def Xform root
    [the newly added xformOp:rotateXYZ which does the rotate]

    def Xform "Cube" ...
    def Xform "Camera" ...
    def Xform "Light" ...

Are the "recommendations" consolidated somewhere? It would be helpful if they were so we could mention the scenario in our commit description etc. That said it does look like there's some precedent out there. I feel a bit better about this now. One software, also z-up, has a direct option in its export settings. Another allows to set it indirectly through it's own preferences. And another does not have this as an option at all currently, but does have an open bug to do something about it. @DhruvGovil Does the solution here match what you'd expect? Basically the structure of the USD would now be as follows: ``` def Xform root [the newly added xformOp:rotateXYZ which does the rotate] def Xform "Cube" ... def Xform "Camera" ... def Xform "Light" ... ```

Charles Wardlaw added 2 commits 2024-05-04 00:24:16 +02:00

718656785c

Changed enums to use values from IO_orientation.hh.

67f9aef957

Addressing other comments from @deadpin .

Dhruv Govil commented

2024-05-04 02:05:03 +02:00

First-time contributor

I'll ask and see if I can find the recommendations. I suspect it's largely spread across the forums, but I agree it's something that should be consolidated.

But yes, having a rotate op would make sense to me to have the easiest form of the fix. The alternate is just multiplying the rotation matrix by everything but that can get tricky.

I'll ask and see if I can find the recommendations. I suspect it's largely spread across the forums, but I agree it's something that should be consolidated. But yes, having a rotate op would make sense to me to have the easiest form of the fix. The alternate is just multiplying the rotation matrix by everything but that can get tricky.

Jesse Yurkovich reviewed 2024-05-06 21:52:00 +02:00

source/blender/io/usd/intern/usd_writer_transform.cc

						
				@ -33,0 +65,4 @@

				  pxr::UsdGeomXformable xform = create_xformable();

				  if (!xform) {

				    printf("INTERNAL ERROR: USDTransformWriter: couldn't create xformable.\n");

Jesse Yurkovich commented

2024-05-06 21:52:00 +02:00

Missed this the first time through but we should remove the raw printf. Can you add a CLOG here as follows:

#include "CLG_log.h"
static CLG_LogRef LOG = {"io.usd"};

Then just do:

CLOG_ERROR(&LOG, "USDTransformWriter: couldn't create xformable");

Missed this the first time through but we should remove the raw printf. Can you add a CLOG here as follows: ``` #include "CLG_log.h" static CLG_LogRef LOG = {"io.usd"}; ``` Then just do: ``` CLOG_ERROR(&LOG, "USDTransformWriter: couldn't create xformable"); ```

CharlesWardlaw marked this conversation as resolved

Charles Wardlaw added 2 commits 2024-05-06 23:47:58 +02:00

11a20cf865

Removing "when checked."

buildbot/vexp-code-patch-lint Build done. Details

buildbot/vexp-code-patch-darwin-x86_64 Build done. Details

buildbot/vexp-code-patch-linux-x86_64 Build done. Details

buildbot/vexp-code-patch-darwin-arm64 Build done. Details

buildbot/vexp-code-patch-windows-amd64 Build done. Details

buildbot/vexp-code-patch-coordinator Build done. Details

c824f51920

Switching to use CLOG for error print.

Jesse Yurkovich commented

2024-05-07 22:30:07 +02:00

@blender-bot build

Charles Wardlaw added 1 commit 2024-05-08 16:08:18 +02:00

buildbot/vexp-code-patch-lint Build done. Details

buildbot/vexp-code-patch-linux-x86_64 Build done. Details

buildbot/vexp-code-patch-darwin-x86_64 Build done. Details

buildbot/vexp-code-patch-windows-amd64 Build done. Details

buildbot/vexp-code-patch-darwin-arm64 Build done. Details

buildbot/vexp-code-patch-coordinator Build done. Details

29105ad93c

Ran make format

Charles Wardlaw commented

2024-05-08 16:08:48 +02:00

@blender-bot build

Blender Bot commented

2024-05-08 16:08:50 +02:00

Only blender organization members with write access can start builds. See documentation for details.

Only blender organization members with write access can start builds. See [documentation](https://projects.blender.org/infrastructure/blender-bot/src/branch/main/README.md) for details.

Ray molenkamp commented

2024-05-08 16:11:02 +02:00

@blender-bot build

Jesse Yurkovich approved these changes 2024-05-08 19:41:16 +02:00

Jesse Yurkovich merged commit 2415380061 into main

2024-05-08 19:57:05 +02:00

Jesse Yurkovich referenced this issue from a commit

2024-05-08 19:57:07 +02:00

USD Export: Adding the ability to choose Stage Up Axis at export time.

Sign in to join this conversation.

No reviewers