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

Restore energy loss in indirect (IBL) specular lighting via multiscattering approximation #15644

Merged
merged 1 commit into from Jan 29, 2019

Conversation

Projects
None yet
7 participants
@jsantell
Copy link
Contributor

jsantell commented Jan 26, 2019

Currently, for metal surfaces (MeshPhysicsMaterial, MeshStandardMaterial), as the roughness of the material increases, the surface darkens if lit via IBL+PMREM (possibly #7489?). For fully metal surfaces (conductors), all incoming irradiance should equal outgoing radiance (F0=1) -- like a mirror (metalness=1, roughness=0). Even if the metal is rough (metalness=1, roughness=1), the same amount of energy should be reflected off of the material.

There is the white furnace test, which has a uniformly lit enironment with fully metal, white spheres with varying degrees of roughness. As these surfaces are fully metal, all incoming light must be reflected, and should be indistinguishable from the environment. If the spheres are visible, then some energy is either lost or gained.

Currently, energy is lost as metal surfaces increase in roughness:

furnace-before

BRDFs only account for a single scattering of light -- for rough, metal surfaces, light will bounce around microfacets after the initial scattering before reaching the viewer, which isn't accounted for in BRDF models.

diagram_single_vs_multi_scatter
via Filament's beautiful docs

There are several sampling and preprocessing methods that aid in real-time approximation of multiscattering light, but ultimately, I was able to get Fdez-Agüera's method (A Multiple-Scattering Microfacet
Model for Real-Time Image-based Lighting
) working well, and used a DFG approximation from Filament instead of a DFG LUT .

comparison-beforeaftersmall

Above, on the left is r100, on the right is this branch -- from top to bottom, metal surface with increasing roughness, rough surface with increasing metalness, metal surface with increasing roughness, nonmetal increasing rough surface). Much more energy is retained on rough, metal objects with this change.

specular-multiscatter-pieces-small

Above is the single-scattering specular indirect lighting only (left), only the multiscattering (center), and both (combined). Currently in r100, only the single-scattering is visible.

furnace-after

Perhaps the least exciting image, above is the furnace test with these changes. All incoming light equals the outgoing light, making the spheres invisible.

Now a handful of questions:

  • Should this be MeshPhysicalMaterial only?
  • What should the conditions for this be? Currently, it's defined( USE_ENVMAP ) && defined( ENVMAP_TYPE_CUBE_UV ), but mostly I'm checking to see if a radiance map exists.
  • Needs a good correctness check
  • Performance check (seems good, but needs a rigorous test)
  • Is the DFG approximation valid?
  • Do we need to separate the indirect specular and diffuse lighting functions? They're combined here to save computations.
  • Maybe this should only be on desktop, depending on perf
@WestLangley

This comment has been minimized.

Copy link
Collaborator

WestLangley commented Jan 26, 2019

Sweet. Thanks for this!

Should this be MeshPhysicalMaterial only?

We'll see. It depends on the performance ramifications.

Your remaining questions are basically rhetorical...

If you want to refactor the shader code, that would be fine with me. I find the #define RE_Direct chunks, etc., somewhat confusing.

@mrdoob mrdoob added this to the r101 milestone Jan 26, 2019

@mrdoob

This comment has been minimized.

Copy link
Owner

mrdoob commented Jan 26, 2019

@bhouston @spidersharma03 do these changes look good to you?

@jsantell

This comment has been minimized.

Copy link
Contributor Author

jsantell commented Jan 26, 2019

Some perf testing, it looks like this might be slighty slightly less performant than the current implementation, but practically the same. Haven't given too much of an eye to optimization yet. It'd be helpful if someone could check it out on iOS.

Test case, 64 objects mostly filling the screen

Mobile Results

device browser current FPS proposed FPS
Pixel 2 Chrome r71 ~59 ~59
Pixel 2 Firefox v64 ~57 ~57
Pixel 1 Chrome r71 ~39 ~38
Pixel 1 Firefox v64 ~47 ~46

Linux Laptop, 800x800

browser DPR current FPS proposed FPS
Firefox v64 1 ~58 ~58
Firefox v64 2 ~32 ~30
Firefox v64 3 ~19 ~19
Chromium v71 2 ~44 ~43

If you want to refactor the shader code, that would be fine with me. I find the #define RE_Direct chunks, etc., somewhat confusing.

Sounds great, I can take care in a follow up

@WestLangley

This comment has been minimized.

Copy link
Collaborator

WestLangley commented Jan 26, 2019

FWIW: iPhone 6, Safari: ~48 fps current; ~42 fps proposed.

@donmccurdy

This comment has been minimized.

Copy link
Collaborator

donmccurdy commented Jan 26, 2019

iPhone SE, Safari: 33fps before and after.

@bhouston

This comment has been minimized.

Copy link
Contributor

bhouston commented Jan 26, 2019

@looeee

This comment has been minimized.

Copy link
Collaborator

looeee commented Jan 27, 2019

Moto G6 (Adreno 506): 6fps before and after

@jsantell jsantell force-pushed the jsantell:energy-conservation branch 2 times, most recently from 563c76c to 6985e72 Jan 28, 2019

@jsantell

This comment has been minimized.

Copy link
Contributor Author

jsantell commented Jan 28, 2019

Updated PR with preserveEnergy boolean on MeshPhysicalMaterial that #define PRESERVE_ENERGY's when set with a PMREM so it could possibly be more opt in. Also consolidated the integrated BRDF generation code which was mostly what the current indirect specular was generating.

Also as a quick double check, this is what the DFG approximation looks like generated in integrateBRDF:

localhost_8080_examples_texture html 5

@mrdoob

This comment has been minimized.

Copy link
Owner

mrdoob commented Jan 28, 2019

Shouldn't preserveEnergy be true by default? The improved output seems worth the slight loss of fps.

@WestLangley

This comment has been minimized.

Copy link
Collaborator

WestLangley commented Jan 28, 2019

Shouldn't preserveEnergy be true by default? The improved output seems worth the slight loss of fps.

I'd hardwire it, then, and remove the preserveEnergy flag.

@jsantell

This comment has been minimized.

Copy link
Contributor Author

jsantell commented Jan 28, 2019

Shouldn't preserveEnergy be true by default? The improved output seems worth the slight loss of fps.

Wasn't sure about the (small) performance delta, and figured this would be the best way for opt in -- happy to remove the flag if should be enabled for all MeshPhysicalMaterials with PMREMs -- which maybe MeshPhysicalMaterial over MeshStandardMaterial is sufficient opt-in for potentially higher cost for more accurate renderings

@mrdoob

This comment has been minimized.

Copy link
Owner

mrdoob commented Jan 28, 2019

I think this should apply to both MeshStandardMaterial and MeshPhysicalMaterial and be enabled by default. Good looking by default is best. If people need extra ms they could opt-out.

@jsantell

This comment has been minimized.

Copy link
Contributor Author

jsantell commented Jan 28, 2019

I think this should apply to both MeshStandardMaterial and MeshPhysicalMaterial and be enabled by default. Good looking by default is best. If people need extra ms they could opt-out.

SGTM -- should it continue being a boolean for folks to disable?

@mrdoob

This comment has been minimized.

Copy link
Owner

mrdoob commented Jan 28, 2019

Is there any other reason for that boolean apart from a few extra ms? If not, I would remove the boolean altogether (and avoid complexity).

@jsantell

This comment has been minimized.

Copy link
Contributor Author

jsantell commented Jan 28, 2019

@mrdoob that's the only reason, and maybe my lack of confidence with physics 😄

@jsantell jsantell force-pushed the jsantell:energy-conservation branch 2 times, most recently from 4d423e1 to ec8bb95 Jan 28, 2019

@jsantell

This comment has been minimized.

Copy link
Contributor Author

jsantell commented Jan 28, 2019

@selfshadow saw your comment right before I force pushed latest changes, so sorry if I blew it away! I did update the coefficient to 1/21 however. Thanks!

@mrdoob Removed preserveEnergy property and enable multiscatter for both Standard/Physical if ENVMAP_TYPE_CUBE_UV defined

@jsantell jsantell force-pushed the jsantell:energy-conservation branch 4 times, most recently from cdafc23 to feadf90 Jan 29, 2019

Restore energy loss in indirect specular lighting by approximating
multiscattering via Fdez-Agüera's "A Multiple-Scattering Microfacet
Model for Real-Time Image-based Lighting".

@jsantell jsantell force-pushed the jsantell:energy-conservation branch from feadf90 to 4e8486f Jan 29, 2019

@jsantell

This comment has been minimized.

Copy link
Contributor Author

jsantell commented Jan 29, 2019

Updated the styling of the glsl strings 👍

@mrdoob mrdoob merged commit 7e99346 into mrdoob:dev Jan 29, 2019

1 of 2 checks passed

LGTM analysis: JavaScript Running analyses for revisions
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mrdoob

This comment has been minimized.

Copy link
Owner

mrdoob commented Jan 29, 2019

Thanks!

@jsantell jsantell deleted the jsantell:energy-conservation branch Jan 29, 2019

@jsantell

This comment has been minimized.

Copy link
Contributor Author

jsantell commented Jan 29, 2019

@mrdoob noted, will take a look!

@WestLangley

This comment has been minimized.

Copy link
Collaborator

WestLangley commented Jan 29, 2019

webgl_materials_standard looks definitely much better.

What exactly are you seeing? I see no visible difference. Maybe I am missing something...

I have only seen significant visible difference in real-world examples (that is, non-furnace tests) for rough-metallic surfaces. Maybe @jsantell has seen otherwise...

@mrdoob

This comment has been minimized.

Copy link
Owner

mrdoob commented Jan 30, 2019

webgl_materials_standard looks definitely much better.

What exactly are you seeing? I see no visible difference. Maybe I am missing something...

If you open both links in different tabs and switch between one and the other you'll see the new version has more light details.

@looeee

This comment has been minimized.

Copy link
Collaborator

looeee commented Jan 30, 2019

The difference is extremely subtle. Using www.diffchecker.com/image-diff on webgl_materials_standard:

diff

@jsantell

This comment has been minimized.

Copy link
Contributor Author

jsantell commented Jan 30, 2019

webgl_materials_standard looks definitely much better.

What exactly are you seeing? I see no visible difference. Maybe I am missing something...

I have only seen significant visible difference in real-world examples (that is, non-furnace tests) for rough-metallic surfaces. Maybe @jsantell has seen otherwise...

The only lighting that's changed is an extra specular lobe for metal surfaces so the change is most noticeable on very rough metallic surfaces, where previously the specular light was lost

@jsantell

This comment has been minimized.

Copy link
Contributor Author

jsantell commented Jan 30, 2019

Potential fix for the dark fresnel border: #15673

@Mugen87

This comment has been minimized.

Copy link
Collaborator

Mugen87 commented Feb 1, 2019

@jsantell I think we should note in the migration guide that the rendering of MeshPhysicsMaterial/ MeshStandardMaterial slightly changed with R101. How would you phrase the most important aspects of the PR?

@jsantell

This comment has been minimized.

Copy link
Contributor Author

jsantell commented Feb 1, 2019

@Mugen87 to take a crack at it, "MeshStandardMaterial and MeshPhysicalMaterial now preserve energy for IBL lighting, resulting in brighter, more accurate colors for metallic materials with high roughness values when lit via a map generated by PMREMGenerator."

@Mugen87

This comment has been minimized.

Copy link
Collaborator

Mugen87 commented Feb 1, 2019

Perfect! I've added your suggestion to the guide.

@mrdoob

This comment has been minimized.

Copy link
Owner

mrdoob commented Feb 8, 2019

@looeee

The difference is extremely subtle.

I guess my eyes can see extremely subtle things at this point 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment