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

KHR_materials_emissive_strength #1994

Merged
merged 21 commits into from
May 23, 2022
Merged

KHR_materials_emissive_strength #1994

merged 21 commits into from
May 23, 2022

Conversation

emackey
Copy link
Member

@emackey emackey commented Jun 11, 2021

View the README.

This supersedes #1971.

Fixes #1083.

@lyuma
Copy link

lyuma commented Jun 12, 2021

Why is this extension incompatible with KHR_materials_pbrSpecularGlossiness?
I don't see any reason why it can't be used equivalently in a spec/gloss workflow. I would advocate for removing that exclusion.

@donmccurdy
Copy link
Contributor

donmccurdy commented Jun 12, 2021

@lyuma — on its own, no, there's no reason this extension is necessarily incompatible with KHR_materials_pbrSpecularGlossiness. But a number of other KHR_materials_* extensions are coming to glTF or available already (see https://github.com/KhronosGroup/glTF/blob/master/extensions), with some general implications:

  1. Asking engines to support complex new features (clearcoat, transmission, volume, sheen, ...) in both PBR workflows is increasingly untenable.
  2. The KHR_materials_pbrSpecularGlossiness extension is less important: with KHR_materials_ior and KHR_materials_specular, the metal/rough workflow can replicate all of the features of the spec/gloss workflow, and an exporter can losslessly convert metal/rough -> spec/gloss.

For those reasons we are focusing future development on the metal/rough workflow, and encouraging implementations to do the same. IMHO this also aligns well with external directions like Autodesk Standard Surface, and the metal/rough workflow in glTF has been much more widely adopted. This effectively simplifies the glTF material model — there is only one PBR workflow, and advanced PBR features can be stacked on top of that.


I'm not necessarily opposed to relaxing the restriction for this particular extension, because it's somewhat of a "bug fix" to the existing emission value, but the overall trend away from KHR_materials_pbrSpecularGlossiness is something to be aware of.

@emackey
Copy link
Member Author

emackey commented Jul 26, 2021

@lyuma Thanks, I pulled the exclusion out. We do want to encourage everyone using KHR_materials_pbrSpecularGlossiness to switch to KHR_materials_specular at their earliest convenience, but there's nothing about this emission multiplier that interacts with either of those extensions. Core material emission was already expected to be compatible with both.

@bghgary
Copy link
Contributor

bghgary commented Oct 27, 2021

Do we have a sample model for this?

@atteneder
Copy link
Contributor

Is that how this is intended to be used?

No.

The original glTF 2.0 schema accidentally clamped emissiveFactor to a maximum value of [1.0, 1.0, 1.0].

This extension undoes the accidental clamp, by multiplying a new value into emissiveFactor.

Thank you very much for explaining and clarifying!

I understand and agree that it's outside of a glTF's scope to be aware of the external exposure and tone-mapping settings of the viewer.

Let me ask another way: Wouldn't it lead to much more consistent results to to recommend it this way?

I mean according to the spec, the product emissiveFactor * emissiveTexture has the unit nits. Shouldn't we recommend to artists to enter physically plausible values for emissiveStrength instead of arbitrary, viewer specific multipliers?

I'm just afraid that if this is under-specified and left to viewers/artists, no emissive glTF material will look consistent across viewers.

Thanks for considering

@emackey
Copy link
Member Author

emackey commented Dec 3, 2021

Units don't work that way. For example:

x meters * y meters = xy square meters (length * length = area)

x meters * y scalar = xy meters (length * scalar = length)

So following that logic,

x nits (factor) * y nits (multiplier) = xy square nits (nonsensical units) final output

x nits (factor) * y scalar (unitless) = xy nits, final output.

The multiplier isn't arbitrary. It must be unitless, to preserve the physical units on the emissive system in glTF.

This extension isn't introducing emission to glTF, merely adjusting the strength of emission that already has physically defined units.

@emackey
Copy link
Member Author

emackey commented Dec 3, 2021

And yes, that does mean that if your emissiveFactor is 1.0, and emissiveStrength is 200, the result is:

1 nit * 200 (scalar) = 200 nits

@atteneder
Copy link
Contributor

And yes, that does mean that if your emissiveFactor is 1.0, and emissiveStrength is 200, the result is:

1 nit * 200 (scalar) = 200 nits

Thanks, agreed! I don't want to suggest giving emissiveStrength a unit (sorry if I explained that poorly).

What I'm suggesting is the final value of emissiveFactor.rgb * sRGB_to_Linear(emissiveTexture.rgb) * emissiveStrength (in nits) should always be a physically plausible value. For that to be true on many real world examples (lamps or screens), the emissiveStrength value has to be in a range well above 100.

@donmccurdy 's desk.glb example above shows a voxely screen with an emissiveStrength of 5. Presuming there's no emissive texture and the emissiveFactor is pure white, this resulting emission is 5 nits, right? 5 nits does not look that bright in real world, does it?

A justification could be that the 2.0 spec does not require viewers to work with physical units for emission at all and so in Don's example emissiveStrength is just an arbitrary multiplier value.

I'd argue that if consistency across viewers is a priority, this extension should require nits as a unit (for the emissiveFactor * emissiveTexture term).

Hope that made it clearer and I'm curious what your thoughts are.

P.S.:
I'm currently prototyping this in Unity and indeed struggle to get it consistent across platforms/render pipelines.

@emackey
Copy link
Member Author

emackey commented Dec 3, 2021

5 nits does not look that bright in real world, does it?

It depends on camera exposure. Almost any number of nits from 1 to a million can be underexposed or overexposed simply by adjusting real-world camera exposure settings. In the case of this one model of a glowing monitor, we can read the specs on a variety of real-world monitor screens, and find that typical values range from 80 to 500 nits depending on the screen, and so 5 nits seems far too dim for this model. But what then? For a while we had 1000 nits as the default, telling people to expect 1000 to exactly equal fully exposed white, and then at a different stage we had 100 nits as the default, saying 100 was white. In both cases, there are far more real-world situations where those numbers are wrong vs. right. We can't tell people what full exposure is here in a material definition, as it depends on the environment, which could be anything from a starry moonless night, where 5 nits is a bright glow, to a harsh summer day outdoors, where hundreds of nits are still too dim to read.

Compounding the problem, most renderers don't consider physical units. Instead, they treat 1.0 as fully exposed, and anything above 1.0 as overexposed or in need of clamping, bloom, tone-mapping, and/or other such treatments. Most of the glTF models we've encountered in the wild so far use this metric instead of the physical one, where "5" means 5x overexposed, which is not necessarily 5 nits (but could be on a dark night). I don't know of an easy fix for this problem, but we've found that simply picking an arbitrary number of nits based on the brightness of a typical computer monitor is not the correct solution.

@emackey
Copy link
Member Author

emackey commented Dec 3, 2021

One solution might be a completely separate extension, where the glTF scene and/or camera object is given exposure controls, indicating how many nits are needed to exactly reach a full white exposure.

Such an extension could default to 1.0 for backwards compatibility. The model of the glowing monitor could maybe use an emissiveStrength of 500 nits paired with an exposure value of 100 nits, giving that 5x factor intended by its author.

@rsahlin
Copy link

rsahlin commented Dec 4, 2021

What I'm suggesting is the final value of emissiveFactor.rgb * sRGB_to_Linear(emissiveTexture.rgb) * emissiveStrength (in nits) should always be a physically plausible value. For that to be true on many real world examples (lamps or screens), the emissiveStrength value has to be in a range well above 100.

Great suggestion @atteneder
This is precisely what I am looking and something we really need in our QA process for instance.
One part to get there is the by adding an extension for mapping of values to display:

PR displaymapping #2083

This will give a plausible range of min/max values and a theoretical range of 0 - 10000 cd/m2 (when displays catch up)
To this extension I am considering adding automatic aperture, or perhaps called auto-average-luminance , which will control overall scene brightness so that a bright outdoor scene and a moonlit scene can be rendered with expected results.
I believe this would give the sceneReferenceValue you describe earlier.

Do you think this would work for your usecase Andreas?

@atteneder
Copy link
Contributor

Thanks for your thoughts, @emackey and @rsahlin

I'm not able to draw a conclusion yet...I need to do more research.

I agree, specifying an arbitrary value (in nits) that's supposed to give white (1.0) emission makes no sense as it depends on the scene/camera exposure.

About bringing exposure controls into glTF scene or camera: That's a good idea, but it only works when the scene/camera settings are not separate to the actual object/model. For lots of use-cases this is not true or desired. As an artist (or manufacturer) I want my glTF to look consistent across viewers.

@rsahlin I agree we should keep display-mapping in mind for this one, thanks for pointing it out. Ultimately I think those extensions should work independently, but I probably need to educate myself more about display-mapping first.

@TerenceRussell
Copy link

Holy cats, still waiting for this bug fix! What's with this discussion of "real lighting units"? Its a scalar workaround for a clamping issue with the original emissiveFactor color scalar.

Copy link
Contributor

@abwood abwood left a comment

Choose a reason for hiding this comment

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

This extension has been ratified by Khronos on May 20, 2022.

@emackey emackey merged commit 6aceb05 into main May 23, 2022
@emackey emackey deleted the KHR_materials_emissive_strength branch May 23, 2022 20:00
0b5vr added a commit to pixiv/three-vrm that referenced this pull request May 25, 2022
This is required in order to support KHR_materials_emissive_strength extension

See: KhronosGroup/glTF#1994
Ref: https://github.com/mrdoob/three.js/pull/23867/files

I could have `_emissive` and `_emissiveStrength` and multiply them on JS side instead, but I decided to multiply on shader side because of code difficulty
proog128 pushed a commit to DassaultSystemes-Technology/glTF that referenced this pull request May 29, 2022
KHR_materials_emissive_strength added.

* initial commit for materials_emissive
* change dir name
* Update physical units, set default to 100.
* Rename folder
* Add emissive strength schema.
* Fix leftover value from earlier draft.
* Contributor list update
* Revise default value and units descriptions based on recent discussion.
* Update description to match jon-adoc branch.
* Remove text redundant with core spec.
* Remove section number, per offline review
* Update to Release Candidate status
* Update copyright message from main glTF registry.
* Update contributors
* fix: Added myself as contributor to KHR_materials_emissive_strength (KhronosGroup#2138)
* Changes per review
* Mark KHR_materials_emissive_strength as ratified.

Co-authored-by: Ben Houston <neuralsoft@gmail.com>
Co-authored-by: Andreas Atteneder <andreas.atteneder@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HDR values for emissiveFactor?