Skip to content

Some minor rewording improvements/clarifications#218

Merged
jstone-lucasfilm merged 29 commits intoAcademySoftwareFoundation:dev_1.2from
portsmouth:wording_fixes
Jan 31, 2025
Merged

Some minor rewording improvements/clarifications#218
jstone-lucasfilm merged 29 commits intoAcademySoftwareFoundation:dev_1.2from
portsmouth:wording_fixes

Conversation

@portsmouth
Copy link
Copy Markdown
Contributor

@portsmouth portsmouth commented Jun 12, 2024

Some minor changes are needed to improve the clarity of the wording (around the implementation of the coat details). I discovered these while using the spec myself to implement the Arnold version.

image

image

image

image

jstone-lucasfilm and others added 4 commits June 8, 2024 11:25
This changelist updates the OpenPBR default example, matching its values to the latest default values of the shading model.
From 1.5 to 1.4.  

As this won't make much difference to the look in implementations that ignore the adjacent IORs of the film.

But for those that take it into account, this will make the film visible rather than invisible by default (since `specular_ior` is 1.5 by default, and `coat_ior` 1.6).
@portsmouth portsmouth changed the base branch from main to dev_1.1 June 12, 2024 16:35
This changelist enables Zeltner sheen in the reference implementation of OpenPBR, leveraging the new functionality in MaterialX 1.39.

Additionally, the open_pbr_velvet.mtlx example has been updated to account for the visual differences between Conty-Kulla and Zeltner sheen.
@portsmouth
Copy link
Copy Markdown
Contributor Author

portsmouth commented Jun 24, 2024

Needs a review. cc @jstone-lucasfilm @virtualzavie @peterkutz

portsmouth and others added 3 commits June 25, 2024 09:37
Comment thread index.html
This changelist updates the types associated with physical color values for subsurface scattering in OpenPBR, aligning with the conclusions of recent threads on ASWF Slack channels.

- Change `subsurface_radius_scale` from a `vector3` to a `color3` in the specification, aligning with the MaterialX implementation of OpenPBR.
- Change the `radius` input of `subsurface_bsdf` from a `vector3` to a `color3` in the MaterialX implementation, aligning with the current definition of the `subsurface_bsdf` node in MaterialX 1.39.
Copy link
Copy Markdown
Contributor

@virtualzavie virtualzavie left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Comment thread index.html
@portsmouth
Copy link
Copy Markdown
Contributor Author

image

@jstone-lucasfilm
Copy link
Copy Markdown
Member

This change looks great to me, though it needs to be proposed as a change to dev_1.2!

@portsmouth portsmouth changed the base branch from dev_1.1 to dev_1.2 October 1, 2024 19:31
@portsmouth
Copy link
Copy Markdown
Contributor Author

portsmouth commented Oct 5, 2024

The formula we give for the fuzz layering can be written in a more simplified form (the algebra is trivial). This makes it a bit clearer why the MaterialX form of the layering works, i.e.:

   <!-- Fuzz Layer -->
    <sheen_bsdf name="fuzz_bsdf" type="BSDF">
      <input name="weight" type="float" interfacename="fuzz_weight" />
      <input name="color" type="color3" interfacename="fuzz_color" />
      <input name="roughness" type="float" interfacename="fuzz_roughness" />
      <input name="normal" type="vector3" interfacename="geometry_normal" />
      <input name="mode" type="string" value="zeltner" />
    </sheen_bsdf>

    <layer name="fuzz_layer" type="BSDF">
      <input name="top" type="BSDF" nodename="fuzz_bsdf" />
      <input name="base" type="BSDF" nodename="coat_layer" />
    </layer>

which does $\mathbf{layer}(\texttt{F} f_\mathrm{fuzz}, f_c)$ (i.e. just layer the fuzz BRDF scaled by the fuzz weight), which is what the formula in green is doing below (in albedo-scaling form). Intuitively, layering a partially present BRDF on a base, in albedo-scaling approximation is equivalent to just scaling the BRDF by the presence weight.

Whereas it's not (immediately) obvious that red formula does that as well.

Before 30293bc
image

After 30293bc
image

Though one must assume also that the MaterialX albedo-scaling ignores the fuzz color in the fuzz brdf albedo computation (which it does in the test render implementations), since the fuzz BRDF contains a color factor which is explicitly ignored in the albedo used in the layering formula given.

@portsmouth
Copy link
Copy Markdown
Contributor Author

@jstone-lucasfilm can be merged?

@portsmouth
Copy link
Copy Markdown
Contributor Author

Added a small clarification to the text to remind that the specular_weight needs to be appropriately clamped to prevent Fresnel > 1 in the darkening calculation:

image

@portsmouth
Copy link
Copy Markdown
Contributor Author

@jstone-lucasfilm Can this be merged?

Copy link
Copy Markdown
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks @portsmouth!

@jstone-lucasfilm jstone-lucasfilm merged commit 1a53140 into AcademySoftwareFoundation:dev_1.2 Jan 31, 2025
portsmouth added a commit to portsmouth/OpenPBR that referenced this pull request Jun 10, 2025
…dation#218)

Some minor changes are needed to improve the clarity of the wording (around the implementation of the coat details). I discovered these while using the spec myself to implement the Arnold version.
AdrienHerubel pushed a commit to AdrienHerubel/OpenPBR that referenced this pull request Apr 17, 2026
…dation#218)

Some minor changes are needed to improve the clarity of the wording (around the implementation of the coat details). I discovered these while using the spec myself to implement the Arnold version.
jstone-lucasfilm pushed a commit that referenced this pull request Apr 17, 2026
This PR increases the version to 1.1.1 and backports any missing change in the branch, it also adds a complete changelog for 1.1 and 1.1.1.

Backported PRs : 
- #235
- #230 
- #218 
- #289 
- #236 
- #262
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