Skip to content

Fix specular_weight not removing base reflection under a coat - #29

Open
peterkutz wants to merge 1 commit into
mainfrom
peterkutz/specular_weight
Open

Fix specular_weight not removing base reflection under a coat#29
peterkutz wants to merge 1 commit into
mainfrom
peterkutz/specular_weight

Conversation

@peterkutz

Copy link
Copy Markdown
Collaborator

Description

This PR contains a minimal surgical fix to ensure that the relative IOR goes to one as the specular_weight goes to zero.

The fix modulates the base IOR relative to that surrounding medium (coat or exterior) and keeps it as an absolute IOR, so reflection, refraction, and thin film all stay consistent. Default (weight 1) behavior is unchanged, as is uncoated behavior when the exterior medium is air (the common case). If the renderer tracks nested dielectrics (exterior_ior != 1), weight zero now index-matches the base to the exterior/ambient IOR instead of to air, which is what the spec calls for.

Related Issue

#28

Motivation and Context

The original code assumed that the specular_weight was supposed to be applied to the absolute IOR of the base surface, which is why the coat–base interface was reflective when the specular_weight was zero. However, the OpenPBR spec says weight zero should match the base to its surrounding medium: the coat, not air.

How Has This Been Tested?

Calculating the results for a few scenarios and running the minimal C++ example.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests 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.

3 participants