[PBR][IBL] do shading in world space instead of camera space, and turn off "double-sided" material support

[bigbike]

Motivation and Context

This diff did the following things:

• Turn off the support for the "double-sided" material due to the "black dashed line" artifacts (models exported from the blender may have problems.);
• Convert the shading from camera space to world space to pave the way for the incoming IBL rendering.
• Optimize the computation in the fragment shader
• small fixes
  

How Has This Been Tested

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have completed my CLA (see CONTRIBUTING)
• I have added tests to cover my changes.
• All new and existing tests passed.

[eundersander]

I left a comment. LGTM.

[eundersander]

Do we expect to see an overall change in brightness for existing PBR users/scenes? If so, is there anyone we should warn?

For context, I'm thinking about the switch from GenericDrawable to PbrDrawable which caused YCB and other objects to unexpectedly darken for many users.

[mosra]

(Sorry for discovering this late, the PR description didn't mention anything sRGB-related so I wasn't paying enough attention.)

@bigbike This isn't the correct way to perform a sRGB conversion, the hardware itself is capable of doing that, without having to add expensive code to the shader. Moreover, doing it in the shader directly ties the shader to a particular texture format and when you use a HDR and/or linear texture instead of sRGB then the output is wrong.

This needs to be resolved at a more generic level by the importer pipeline itself, not in the shader, and you might remember that I listed exactly that on my tasklist. Can you revert this sRGB conversion part for now and wait until this is handled at the engine level? If each shader does something different on its own, it'll be a mess.

[bigbike]

@mosra This time I revisited the https://github.com/KhronosGroup/glTF/tree/master/specification/2.0 where it mentions, and here I quote:
RGB color values use sRGB color primaries.
The baseColorTexture uses the sRGB transfer function and must be converted to linear space before it is used for any computations.
And for emissive texture: This texture contains RGB components encoded with the sRGB transfer function.

So my understanding is that all the textures are encoded in sRGB and such conversion is a must have (I assume blender follow this spec and output the texture like this.). Am I wrong?

The way to rollback it can be simple. In the SRGBtoLINEAR, there is a macro that can turn it off. I can enable this macro for now in the C++.

[mosra]

It's ... complicated :)

RGB color values use sRGB color primaries.

Yes. (In other words, the glTF PBR definition assumes the input is with "the usual" (sRGB) primaries and not in Adobe RGB or something else. This is tangential to the gamma/colorspace encoding.

The baseColorTexture uses the sRGB transfer function and must be converted to linear space before it is used for any computations.

This is correct only in the context of the original and unextended glTF 2.0 specification which forced the textures to be always either 8-bit PNG or 8-bit JPEG, for which, if they store color data, makes sense to have the sRGB curve applied. But it's not a rule and PNG/JPEG files commonly don't bother specifying the colorspace in the metadata, so to ensure the same PNG/JPEG files are understood the same way in all implementations, they tell the implementations to assume sRGB (for colors, but not e.g. normals or alpha). This is mainly why the sentence is there.

However:

• 16-bit PNGs are not so uncommon and there's nothing like "16-bit sRGB", the 16-bit data should be stored linear, so if you get such a texture, decoding it as sRGB is not correct.
• Various glTF extensions add support for formats beyond just PNG and JPEG, and also correspondingly loosen the requirement of sRGB encoding. For example, this is what the KTX/Basis extension says: https://github.com/KhronosGroup/glTF/blob/master/extensions/2.0/Khronos/KHR_texture_basisu/README.md#ktx-v2-images-with-basis-universal-supercompression
• Finally, all this applies to just glTF, while here you have a generic metallic/roughness PBR shader that I think should still be able to correctly load e.g. EXR or KTX textures, outside of the very narrow file format support glTF assumes.

Hope this explains it well enough.

TL;DR: the shader is "too late" in the pipeline to be able to decide what color encoding the input is in, and the importer should take care of that instead by specifying an appropriate image/texture format. (And I have no idea why the glTF PBR example code does this given that the hardware itself is capable of sRGB en/decoding for about a decade already, I assume the author just didn't know, nobody is perfect 🙂)