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

Image-based lighting #7172

Merged
merged 103 commits into from
Dec 19, 2018
Merged

Image-based lighting #7172

merged 103 commits into from
Dec 19, 2018

Conversation

bagnell
Copy link
Contributor

@bagnell bagnell commented Oct 19, 2018

Adds the ability to add image-based lighting to a model.

  • Model.sphericalHarmonicCoefficients is a property that is an array of 9 Cartesian3 coefficients used to evaluate the diffuse irradiance.
  • Model.specularEnvironmentMaps is a property that is a url to a KTX file that contains a cubmap of the specular contribution plus the specular convolution mipmap levels.

TODO:

  • Tests
  • Update CHANGES.md

NOTE: This is a PR into the hdr branch.

bagnell and others added 30 commits September 25, 2018 19:34
Add octahedral projection
Fix artifacts with Octahedral Projection
Fix Octahedral sampling artifacts
Add Trilinear Filtering to Octahedral Maps
@lilleyse
Copy link
Contributor

Procedural IBL seems to be a lot brighter in this branch compared to master. Maybe tone down the default luminance?

dark

bright

Also the environment map seems a lot darker than it did before. Can that also be adjusted?

dark-env

@lilleyse
Copy link
Contributor

lilleyse commented Nov 20, 2018

It might be good to look at the NYC building while tuning the default lighting. Right now they look washed out.

washed-out

I would say master is too dark. We should aim for something in the middle that has a stronger contrast between the dark and light areas.

nyc-master

@lilleyse
Copy link
Contributor

localhost example

The lighting orientation looks wrong in the NYC tileset compared to a model. After talking offline @bagnell thinks this is because the tiles don't have a model matrix and aren't sampling the environment map/spherical harmonics in the right direction.

ibl-dir
ibl-dir-good

@lilleyse
Copy link
Contributor

In this demo with two models added I see TextureCache.addTexture getting called twice. In a separate demo with 3D Tiles I only see it get called once. Maybe there is some edge case with how the demo adds models.

Demo.

@lilleyse
Copy link
Contributor

Textures will get cleaned up from the cache every 120 frames. Should this be longer?

I think this is fine. The texture cache code looks good.

@bagnell
Copy link
Contributor Author

bagnell commented Nov 29, 2018

Also the environment map seems a lot darker than it did before. Can that also be adjusted?

This is what it looked like before:
image

The reason is because the prototype wasn't using the BRDF LUT. You can see by commenting out this line:
https://github.com/AnalyticalGraphicsInc/cesium/pull/7172/files#diff-6f6a773d137da52954260087ddc76a95R747

@bagnell
Copy link
Contributor Author

bagnell commented Dec 1, 2018

@lilleyse This is ready for another look. The only thing I didn't change was the model in the IBL example. I could barely see any reflection from the environment map on that helicopter model.

@lilleyse
Copy link
Contributor

lilleyse commented Dec 7, 2018

@bagnell Try Pawns.glb.txt which cuts down on the sample model file size.

@bagnell
Copy link
Contributor Author

bagnell commented Dec 18, 2018

@lilleyse This is ready for another review. If you think this is ready, I can update the Sandcastle example image and update CHANGES.

@lilleyse
Copy link
Contributor

Nothing else from me. 👍

@bagnell
Copy link
Contributor Author

bagnell commented Dec 19, 2018

@lilleyse The Sandcastle image and CHANGES have been updated. This should be ready to go.

@lilleyse lilleyse merged commit 700bbb8 into master Dec 19, 2018
@lilleyse lilleyse deleted the ibl branch December 19, 2018 21:17
@TimvanScherpenzeel
Copy link

Hi,

I was looking over the code (great work! 👍), as I'm looking to implement something similar, and was wondering about the spherical harmonics reconstruction in: https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Shaders/Builtin/Functions/sphericalHarmonics.glsl

From what I understand you are using the spherical harmonics generated with cmgen. There are a couple of things to be aware of when doing so because the developers of Filament have tried to pre-compute as much as possible in order to make reconstruction in the shader as cheap as possible.

This means that reconstruction in the shader should be as simple as:

screen shot 2019-01-03 at 12 09 37

As implemented here in the Filament engine:

https://github.com/google/filament/blob/f676d3d3ea55539774c7eefd894ff156c3f9cd86/shaders/src/light_indirect.fs#L127-L147

There is mention about Fd_Lambert() being baked into the SH, not really sure what that is about but could possibly lead to some brightness issues if not taken into account.

https://github.com/google/filament/blob/2c4ecebf11bdcd0099d869197d9c6ee1ff7bcfbf/shaders/src/light_indirect.fs#L442

Another thing I noticed is that cmgen automatically mirrors your cubemap (you can prevent this by passing the --no-mirror flag).

One last thing you might not have been aware of is that in the metadata of the outputted .ktx file the spherical harmonics are embedded meaning you could possibly save some extra manual steps.

screen shot 2019-01-03 at 12 27 16

@romainguy
Copy link

We do bake the division by pi from Lambert in the SH coefficients by default. cmgen can be invoked with --sh-output=file.txt -i to output coefficients with no pre-baking. The pre-baking does keep the reconstruction slightly simpler, it's always nice to save a few instructions (which would matter more when supporting local probes or SH-based light maps):

vec3 Irradiance_SphericalHarmonics(const vec3 n) {
    return max(
          frameUniforms.iblSH[0]
#if SPHERICAL_HARMONICS_BANDS >= 2
        + frameUniforms.iblSH[1] * (n.y)
        + frameUniforms.iblSH[2] * (n.z)
        + frameUniforms.iblSH[3] * (n.x)
#endif
#if SPHERICAL_HARMONICS_BANDS >= 3
        + frameUniforms.iblSH[4] * (n.y * n.x)
        + frameUniforms.iblSH[5] * (n.y * n.z)
        + frameUniforms.iblSH[6] * (3.0 * n.z * n.z - 1.0)
        + frameUniforms.iblSH[7] * (n.z * n.x)
        + frameUniforms.iblSH[8] * (n.x * n.x - n.y * n.y)
#endif
        , 0.0);
}

@OmarShehata OmarShehata mentioned this pull request Oct 16, 2019
8 tasks
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.

8 participants