Skip to content

Fix IECOREUSD_WRITE_CONFORMANT_RENDERMAN_ATTRIBUTES bugs#1544

Merged
murraystevenson merged 3 commits into
ImageEngine:RB-10.6from
johnhaddon:rmanAttributesFixes
Jul 20, 2026
Merged

Fix IECOREUSD_WRITE_CONFORMANT_RENDERMAN_ATTRIBUTES bugs#1544
murraystevenson merged 3 commits into
ImageEngine:RB-10.6from
johnhaddon:rmanAttributesFixes

Conversation

@johnhaddon

Copy link
Copy Markdown
Member

I messed up in 5e2f322, because I didn't realise the full effects of changing AttributeAlgo::nameToUSD(). Despite the function name, this was used when reading attributes too, and was also used when determining the names to use for UsdShadeShader outputs. This lead to several bugs :

  • Failure to read shaders written with IECOREUSD_WRITE_CONFORMANT_RENDERMAN_ATTRIBUTES=0.
  • Bad output names when writing shaders with IECOREUSD_WRITE_CONFORMANT_RENDERMAN_ATTRIBUTES=1.
  • Failure to read renderman attributes written with a different value of IECOREUSD_WRITE_CONFORMANT_RENDERMAN_ATTRIBUTES.

All this stuff is well overdue for a complete simplification where we only write attributes as primvars, but since we need this for 1.6.
x, I've made the most minimal change I can that I think is logical and fixes the problems. The fundamental issue is that because we want to support legacy attributes too, and nameToUSD() is used for reading, there is not a 1-to-1 mapping of Cortex names to USD names. This is reflected in the new internal usdNames() function which lists the relevant attribute names in the order we'd prefer to use them.

We're already in an anonymous namespace, so we don't need these.
We don't need to add the `ri:attributes` prefix for these, as they're not written as primvars.
Don't let it dictate what attribute names we use for _reading_. Otherwise we can't read attributes unless the env-var matches the value used to write them.
@johnhaddon johnhaddon self-assigned this Jul 20, 2026

@murraystevenson murraystevenson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks John, the fixes and improved test coverage look good to me. I agree we're overdue an overhaul here...

@murraystevenson
murraystevenson merged commit b56bb74 into ImageEngine:RB-10.6 Jul 20, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants