Fix for ignored primvars:crypto_asset#2711
Open
cpichard wants to merge 4 commits into
Open
Conversation
Collaborator
Author
|
✅ CI passed (build 11). |
|
Chorus detected one or more security issues with this pull request. See the Checks tab for more details. As a reminder, please follow the secure code review process as part of the Secure Coding Trust Commitment requirement. |
This file contains hidden or 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
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.
Changes proposed in this pull request
Issue: primvars:crypto_asset (and similarly resolved inherited primvars) was being ignored when an asset was added through an instanceable reference, causing the Cryptomatte asset AOV to come out empty.
Root cause: The primvar is authored as a constant value on a parent Xform and inherited down to the geometry. When resolved through an instanceable reference, it arrived at the shape as a single-element uniform (per-element) array rather than a true constant value. Arnold declared this as a size-1 uniform array, but then tried to index it per-face, producing an out-of-range error (index N of 1) and silently failing to apply the value.
Fix (libs/common/parameters_utils.cpp): DeclareAndAssignParameter now treats any single-element array as constant user data, unless it's explicitly a "constant array" scope (scope == constantArray). Previously it only did this for scope == constant or the special-cased displayColor attribute — so uniform/varying/indexed single-element arrays fell through and got declared as arrays, which Arnold couldn't index correctly. Now a single value correctly applies to the whole shape regardless of interpolation scope.
Test: testsuite/test_2699 sets up an instanceable reference with an inherited primvars:crypto_asset, and asserts on the render log (no out-of-range/failed-lookup errors) rather than pixel values, since Cryptomatte hashes differ across reading paths.
I tested it with HTOA + nuke cryptomatte.
Issues fixed in this pull request
Fixes #2699