AcademySoftwareFoundation / OpenPBR

Specification and reference implementation for the OpenPBR Surface shading model
Apache License 2.0
459 stars 18 forks source link

Add emission_weight #231

Open portsmouth opened 1 month ago

portsmouth commented 1 month ago

Implements the change described in https://github.com/AcademySoftwareFoundation/OpenPBR/issues/229.

image

image

portsmouth commented 2 weeks ago

emission_weight luminance emission_luminance needs to default to 1000 in both the text and the parametrization table.

virtualzavie commented 2 weeks ago

emission_weight luminance needs to default to 1000 in both the text and the parametrization table.

Ah right, I see the parameter list still has a luminance default value of 0 (assuming you meant emission_luminance, not emission_weight). If we have two parameters, indeed, one of them must have a default value different from 0.

The default value 1000 sounds reasonable to me since it's the order of magnitude of the maximum luminance of typical displays screens (smartphones and TVs).

jstone-lucasfilm commented 6 days ago

This looks fine to me, though it would be ideal to update the reference implementation in sync with this change, so that we don't accidentally create divergence between them.

portsmouth commented 2 days ago

This looks fine to me, though it would be ideal to update the reference implementation in sync with this change, so that we don't accidentally create divergence between them.

Done in https://github.com/AcademySoftwareFoundation/OpenPBR/pull/231/commits/eed50221cb278fb03eb30d1ae9937ad52044000a