Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI disabling logic #232

Open
wants to merge 6 commits into
base: dev_1.2
Choose a base branch
from
Open

UI disabling logic #232

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0be550f
Add section describing the "recommended logic needed to determine whi…
portsmouth Aug 24, 2024
5a736f8
tweak
portsmouth Aug 24, 2024
e6d3d28
Wording improvement
portsmouth Aug 24, 2024
1c9c1a8
Fix pseudocode for UI disabling logic to be actual Python
portsmouth Aug 28, 2024
3564c96
Fix bug in base_weight/base_color disabling logic
portsmouth Oct 11, 2024
3218a94
Merge branch 'dev_1.2' into ui_logic
portsmouth Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions parametrization.md.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,43 @@
| `geometry_coat_normal` | Coat Normal | `vector3` | N/A | | unperturbed normal | |
| `geometry_coat_tangent` | Coat Tangent | `vector3` | N/A | | unperturbed tangent | |


## UI disabling logic
Copy link
Member

Choose a reason for hiding this comment

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

Would there be a value in broadening the scope of this proposal, moving it to the Standard UI Attributes section of the MaterialX specification? That might help to build consistency between implementations of OpenPBR Surface and implementations of other popular shading models such as Standard Surface and glTF PBR:

https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/documents/Specification/MaterialX.Specification.md#standard-ui-attributes

Copy link
Contributor Author

@portsmouth portsmouth Aug 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Since OpenPBR is not MaterialX-specific, I don't think it would make sense to describe the logic only for MaterialX (though not sure what was being proposed exactly). The section here simply describes in a simple unambiguous way which parameters can be ignored given the state of the rest of the parameters, which can avoid confusion in practice when artists move a slider and it does nothing (instead of the slider being disabled).

It would be valuable to figure out how to express the described enabling/disabling logic in MaterialX form though (or if it currently isn't possible, to figure out how to make it possible).


The structure of the OpenPBR tree of layer and mix operations implies that some parameters have no effect if other parameters take certain values.
It can be a helpful cue for artists if the UI design for the shader reflects this by disabling (e.g. greying out) those controls which are irrelevant in the current context.

To facilitate this aspect of the UI design, we provide here in Python pseudocode form the recommended logic needed to determine which of the parameters can be disabled. This employs a function `disable(P, skip=[])` where the arguments `P` and the optional `skip` are a prefix string (or list of prefix strings). Then all parameters with names matching any of the prefixes in `P` should be disabled, except for those matching any in `skip`.

```python
has_fuzz = (fuzz_weight > 0.0)
has_coat = (coat_weight > 0.0)
has_metal = (base_metalness > 0.0)
has_dielectric = (base_metalness < 1.0)
has_dielectric_transp = has_dielectric and (transmission_weight > 0.0)
has_dielectric_opaque = has_dielectric and (transmission_weight < 1.0)
has_subsurface = has_dielectric_opaque and (subsurface_weight > 0.0)
has_diffuse = has_dielectric_opaque and (subsurface_weight < 1.0)
if not has_fuzz: disable("fuzz", skip="fuzz_weight")
if not has_coat:
disable("coat", skip="coat_weight")
disable("geometry_coat_tangent")
if not has_dielectric: disable("specular_ior")
if not has_dielectric_transp: disable("transmission", skip="transmission_weight")
if (not has_diffuse and not has_metal): disable(["base_weight", "base_color"])
if not has_subsurface: disable("subsurface", skip="subsurface_weight")
if not has_diffuse: disable("base_diffuse_roughness")
if (specular_weight == 0): disable("specular", skip="specular_weight")
if (thin_film_weight == 0): disable("thin_film", skip="thin_film_weight")
# In thin-walled mode, the volumetric medium is infinitesimally thin so the parameters controlling
# the MFP of the medium are irrelevant
if (geometry_thin_walled):
disable("transmission", skip=["transmission_weight", "transmission_color"])
disable("subsurface_radius") # (NB, also disables subsurface_radius_scale)
```



<!-- Markdeep: -->
<style class="fallback">
body { visibility:hidden }
Expand All @@ -84,3 +121,6 @@
<script>
window.alreadyProcessedMarkdeep || (document.body.style.visibility="visible")
</script>