Disable HDR by Default

[ProjectBarks]

HDR does not provide enough (or any) of a visual quality improvement to justify being on by default. The problem is very little of the Cesium rendering engine outputs high dynamic range values so tone-mapping is ineffective and only darkens the scene.

This PR is connected to #7966

[cesium-concierge]

Thanks for the pull request @ProjectBarks!

• ✔️ Signed CLA found.
• ❔ Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[mramato]

As @lilleyse stated in the linked issue's primary description, we want to retain HDR for gltf 2.0 PBR tonemapping.

[ProjectBarks]

@lilleyse, @mramato this is ready for review

[mramato]

Code looks good to me (I didn't run it). @lilleyse can you take a pass and merge if you're happy?

[cesium-concierge]

Thanks for the awesome work @ProjectBarks! Your contribution is about to launch to millions of users with the next release. 🚀

Do you mind if we tweet about it? CC @OmarShehata @slchow.

[lilleyse]

To fix the washed out look of the PBR models make sure gamma correction only gets applied once.

LINEARtoSRGB in processPbrMaterials (when HDR is off) and czm_inverseGamma in acesTonemapping.glsl are basically double-applying the gamma correction and making the model look too bright.

I would remove czm_inverseGamma from acesTonemapping.glsl and move it to the end of AcesTonemappingStage.glsl instead. Deleting LINEARtoSRGB is the alternative but then the model would be using scene.gamma instead of the hardcoded 2.2, and I'm not sure it's a good idea for non-HDR rendering to use a property intended for HDR mode.

Finally, I would remove this block:

        fragmentShader += '#ifndef HDR \n';
        fragmentShader += '    gl_FragColor = vec4(czm_acesTonemapping(gl_FragColor.rgb), gl_FragColor.a); \n';
        fragmentShader += '#endif \n';

and combine it into LINEARtoSRGB:

        fragmentShader +=
            'vec3 LINEARtoSRGB(vec3 linearIn) \n' +
            '{\n' +
            '#ifndef HDR \n' +
            '    return pow(czm_acesTonemapping(linearIn), vec3(1.0/2.2));\n' +
            '#else \n' +
            '    return linearIn;\n' +
            '#endif \n' +
            '}\n\n';

[ProjectBarks]

Good sandcastle for testing.

[lilleyse]

The Sandcastle looks good locally. Screenshots of No HDR vs. HDR below. Minimal difference in model shading which is good (aside from shadows).

No HDR

HDR

[lilleyse]

Will there be a built-in Sandcastle example to toggle HDR? Or is that in the other PR?

[ProjectBarks]

@lilleyse I can make that. Do we want to add that to an existing sandcastle (like I did above) or should we make a new one all together.

@mramato any ideas for a sandcastle?

[mramato]

Let's use a separate PR that just adds a new Sandcastle for this/FXAA and any other rendering quality/options that we think are worth it. That way we don't need to hold this or the image scaling one up.

[lilleyse]

For the sandcastle PR - A scene with just the ground vehicle like the screenshots above is good. If you want to get fancy maybe add a slider for scene.gamma, toggle for auto exposure, and a dropdown for different tonemapping algorithms.