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.

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

Conversation

portsmouth
Copy link
Contributor

@portsmouth portsmouth commented Aug 24, 2024

Add section describing the "recommended logic needed to determine which of the parameters can be disabled".

This describes the parameters which can safely be disabled (e.g. their UI element greyed out) in the current configuration, according to which parts of the material graph are excised, which can be read off from the tree diagram.

This is given in the form of some fairly self-explanatory Python pseudocode, which is easier to understand and more concise than a wordy verbal description. (And probably translates quite easily into the corresponding UI code in whatever form each DCC implements it).

(All of the Arnold plugin integrations will implement such logic).

image

image

(NB, the wrong 4.18 numbering in the heading is an unrelated bug which we're trying to figure out).

@@ -72,6 +72,45 @@
| `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

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).

@portsmouth portsmouth marked this pull request as draft August 27, 2024 11:09
@portsmouth
Copy link
Contributor Author

portsmouth commented Aug 27, 2024

Oops, that pseudocode is not legit Python. Will fix. (Fixed, and in comment above).

@portsmouth
Copy link
Contributor Author

portsmouth commented Aug 28, 2024

I also put together a Python script which will run the exact Python pseudocode given in the spec, and report which parameters should be disabled based on their current state. We could maybe ship this with the spec, to show how it might be done in a plugin? (Can be improved to make it a tiny Python API that could be slotted in, giving the enable state of all controls given the parameter values dict).

Output:

$ python scripts/disabling.py
Non-default params
    specular_weight = 0.0
    transmission_weight = 1.0

=> Resulting enable state:
   base_color                              : **disabled**
   base_diffuse_roughness                  : **disabled**
   base_metalness                          : enabled
   base_weight                             : **disabled**
   coat_color                              : disabled
   coat_darkening                          : disabled
   coat_ior                                : disabled
   coat_roughness                          : disabled
   coat_roughness_anisotropy               : disabled
   coat_weight                             : enabled
   emission_color                          : enabled
   emission_luminance                      : enabled
   fuzz_color                              : disabled
   fuzz_roughness                          : disabled
   fuzz_weight                             : enabled
   geometry_opacity                        : enabled
   geometry_thin_walled                    : enabled
   specular_color                          : **disabled**
   specular_ior                            : **disabled**
   specular_roughness                      : **disabled**
   specular_roughness_anisotropy           : **disabled**
   specular_weight                         : enabled
   subsurface_color                        : disabled
   subsurface_radius                       : disabled
   subsurface_radius_scale                 : disabled
   subsurface_scatter_anisotropy           : disabled
   subsurface_weight                       : enabled
   thin_film_ior                           : disabled
   thin_film_thickness                     : disabled
   thin_film_weight                        : enabled
   transmission_color                      : **enabled**
   transmission_depth                      : **enabled**
   transmission_dispersion_abbe_number     : **enabled**
   transmission_dispersion_scale           : **enabled**
   transmission_scatter                    : **enabled**
   transmission_scatter_anisotropy         : **enabled**
   transmission_weight                     : enabled

Script:

openpbr_params = {

'base_weight' : 1.0,
'base_color' : (0.8, 0.8, 0.8),
'base_metalness' : 0.0,
'base_diffuse_roughness' : 0.0,
'specular_weight' : 1.0,
'specular_color' : (1.0, 1.0, 1.0),
'specular_roughness' : 0.3,
'specular_roughness_anisotropy' : 0.0,
'specular_ior' : 1.5,
'transmission_weight' : 0.0,
'transmission_color' : (1.0, 1.0, 1.0),
'transmission_depth' : 0.0,
'transmission_scatter' : (0.0, 0.0, 0.0),
'transmission_scatter_anisotropy' : 0.0,
'transmission_dispersion_scale' : 0.0,
'transmission_dispersion_abbe_number': 20.0,
'subsurface_weight' : 0.0,
'subsurface_color' : (0.8, 0.8, 0.8),
'subsurface_radius' : 1.0,
'subsurface_radius_scale' : (1.0, 0.5, 0.25),
'subsurface_scatter_anisotropy' : 0.0,
'coat_weight' : 0.0,
'coat_color' : (1.0, 1.0, 1.0),
'coat_roughness' : 0.0,
'coat_roughness_anisotropy' : 0.0,
'coat_ior' : 1.6,
'coat_darkening' : 1.0,
'fuzz_weight' : 0.0,
'fuzz_color' : (1.0, 1.0, 1.0),
'fuzz_roughness' : 0.5,
'emission_luminance' : 0.0,
'emission_color' : (1.0, 1.0, 1.0),
'thin_film_weight' : 0.0,
'thin_film_thickness' : 0.5,
'thin_film_ior' : 1.4,
'geometry_opacity' : 1.0,
'geometry_thin_walled' : False

}

def got_match(p, prefixes):
   if not prefixes: return False
   if not isinstance(prefixes, list): P = [prefixes]
   else:                          P = prefixes
   for prefix in P:
      if p.startswith(prefix):
         return True
   return False

def disable(P, skip=None):
   for p in openpbr_params:
      if got_match(p, P) and not got_match(p, skip):
         param_states[p] = False


param_states = {}

def ui_logic(openpbr_params):

   global param_states
   for P in openpbr_params:
      param_states[P] = True

   fuzz_weight          = openpbr_params['fuzz_weight']
   coat_weight          = openpbr_params['coat_weight']
   base_metalness       = openpbr_params['base_metalness']
   transmission_weight  = openpbr_params['transmission_weight']
   subsurface_weight    = openpbr_params['subsurface_weight']
   specular_weight      = openpbr_params['specular_weight']
   thin_film_weight     = openpbr_params['thin_film_weight']
   geometry_thin_walled = openpbr_params['geometry_thin_walled']

   ############################################################################################################
   # spec pseudo-code
   ############################################################################################################
   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_dielectric_opaque 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)
   ############################################################################################################

   return param_states


if __name__ == '__main__':

   openpbr_params_default = openpbr_params.copy()
   default_states = ui_logic(openpbr_params).copy()

   #############################################
   # Param modifications
   #############################################
   openpbr_params['specular_weight'] = 0.0
   openpbr_params['transmission_weight'] = 1.0

   # Print non-default params
   print("Non-default params")
   for P in openpbr_params:
      val         = openpbr_params[P]
      default_val = openpbr_params_default[P]
      if val != default_val:
         print('    %s = %s' % (P, val))

   # Run disabling logic
   param_states = ui_logic(openpbr_params)

   # Show resulting states, with non-default values highlighted
   print('\n=> Resulting enable state:')

   for P in sorted(openpbr_params.keys()):
      s = 'enabled' if param_states[P] else 'disabled'
      if param_states[P] != default_states[P]:
         s = '**' + s + '**'
      print("   {0:40}: {1}".format(P, s))

@portsmouth portsmouth marked this pull request as ready for review August 28, 2024 18:51
@portsmouth portsmouth changed the base branch from main to dev_1.2 October 1, 2024 19:33
@TomMinor
Copy link

Based on a discussion with @portsmouth, I think the disable logic for base_weight/base_color should be

if (not has_diffuse and not has_metal):     disable(["base_weight", "base_color"])

@portsmouth
Copy link
Contributor Author

Do people think there would be value in including the script as well (In a scripts/ directory perhaps)?

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