IECoreUSD : Load prototype paths inside a PointInstancer as relative #1451
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danieldresser-ie
force-pushed
the
usdInstRel
branch
from
3d273a2 to
4aa69e8
Compare
|
One other known issue - the tests are currently weird: I wrote 3 tests for the 3 different env var configurations that make sense, but they rely on the person running the tests to set the env vars appropriately in order to run them. Gaffer has tests which set env vars and then relaunch themselves in a subprocess, but I wasn't sure how to do that in the Cortex test setup. |
johnhaddon
reviewed
Feb 3, 2025
danieldresser-ie
force-pushed
the
usdInstRel
branch
from
4aa69e8 to
059b91c
Compare
|
OK, I think I've now addressed everything outstanding, and updated the Changes file. |
|
Thanks Daniel. I've pushed a fixup which tweaks the env-var name, slightly simplifies the logic and expands the test coverage. Could you give that a quick check over? |
|
Changes LGTM. |
If you set the env var IECOREUSD_POINTINSTANCER_RELATIVEPROTOTYPES=1, if an instancer named "/inst" contains an internal prototype path like "/inst/Prototypes/proto", that prototype path will now be loaded as "./Prototypes/proto". This is compatible with how point instancers will now be handled in Gaffer, allowing them to be relocated within the scene hierarchy.
johnhaddon
force-pushed
the
usdInstRel
branch
from
c498a77 to
1560cef
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements the approach to relative PointInstancer prototype paths we've discussed.
Known issues:
I've made this check whether GAFFERSCENE_INSTANCER_EXPLICITABSOLUTEPATHS, and use future-style relative paths if Gaffer supports them. It's probably weird to make this depend on a Gaffer env var ... should I just hardcode to prefix the paths with "./", and then we'll have to do another cleanup pass to fix this at some indeterminate point in the future?
I've currently made the env var to enable this "GAFFERUSD_POINTINSTANCER_RELATIVEPROTOTYPES". Since this ends up being implemented in Cortex, I guess it should be "IECOREUSD_POINTINSTANCER_RELATIVEPROTOTYPES"? Not that this is a distinction that users are likely to care about.