Skip to content

Standard Surface: Connect thin_walled to the surface constructor#2991

Open
msuzuki-nvidia wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
msuzuki-nvidia:fix-standardsurface-thinwalled
Open

Standard Surface: Connect thin_walled to the surface constructor#2991
msuzuki-nvidia wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
msuzuki-nvidia:fix-standardsurface-thinwalled

Conversation

@msuzuki-nvidia

Copy link
Copy Markdown
Contributor

Connect thin_walled to the surface constructor in standard_surface nodedef.

@jstone-lucasfilm

Copy link
Copy Markdown
Member

Thanks for putting this together, @msuzuki-nvidia. The change looks right on both counts: forwarding thin_walled into the surface constructor, and marking the interface input uniform="true" to match it.

@ld-kerley I'm CC'ing you here since you raised the backwards-compatibility question about changing uniform on a shipped interface input, which is a fair point to weigh. A few concrete things make me think the risk here is quite low:

  • Established precedent: OpenPBR made the identical change (forward plus uniform="true") in Connect geometry_thin_walled to surface in OpenPBR #2759, so this brings standard_surface in line with a convention that has already shipped.
  • It is the norm rather than a new pattern. The booleans that gate surface and BSDF construction are already uniform, including energy_compensation and retroreflective on the BSDF constructors, and the ND_surface constructor's own thin_walled. The non-uniform thin_walled interface on standard_surface is currently the outlier.
  • Required for consistency once connected: the ND_surface input is uniform="true" and the MDL implementation declares it as uniform bool, so a non-uniform interface value feeding it would be a qualifier mismatch.
  • No content regresses. Across all bundled materials and our render test suite, thin_walled is only ever authored as a literal value, and is never connected to an upstream node. The only thing affected would be a third-party document driving it from a varying source, which is not a meaningful operation for a structural surface flag of this kind.

Given that, I believe this is the right next step, but I'd like to confirm this with @ld-kerley before proceeding. If we are aligned, I would suggest we also note the uniform change in our changelog and release notes for discoverability.

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