AwesomesauceLabs / piglet-viewer

Demo viewer app for Piglet glTF Importer
MIT License
6 stars 4 forks source link

Potentially wrong color space with URP shaders #5

Closed oprogramadorreal closed 2 years ago

oprogramadorreal commented 2 years ago

In Piglet.UnityGLTF.Extensions.SchemaExtensions.ToUnityColor there is the following comment:

In glTF, color factors (...) are stored as linear values. However, Unity shaders expect the values for Color shader properties to be given in Gamma space (...)

However, I notice the material Piglet creates uses a custom shader called URPMetallicRoughnessOpaqueOrMask in Piglet/Resources/Shaders/URP. It's not a standard Unity shader.

I wonder if the conversion to "gamma" performed in ToUnityColor is still required. At least for my application, if I remove the conversion to gamma the colors look better.

I'd appreciate a look into this. Thanks in advance!

benvvalk commented 2 years ago

Hi @oprogramadorreal,

The call to .gamma in Piglet.UnityGLTF.Extensions.SchemaExtensions.ToUnityColor corrects the values of color factors (e.g. _baseColorFactor), whereas the corrections in the URP shaders correct the values sampled from the textures (e.g. _metallicRoughnessTexture). Color factors are constants (e.g. white = (1, 1, 1, 1)) that are multiplied with every value sampled from the corresponding texture (e.g. _baseColorFactor and _baseColorTexture). In other words, the purpose of color factors is to "tint" the overall color of a texture.

The only reason that the texture values need to be color-corrected in the URP shaders is that UnityWebRequestTexture does not load linear textures correctly (e.g. _metallicRoughnessTexture). The problem is that UnityWebRequestTexture lacks a linear parameter and just assumes that all input textures are sRGB-encoded (i.e. gamma-encoded). This means that UnityWebRequestTexture performs an unwanted sRGB -> linear conversion on linear textures, and I therefore need to reverse that conversion in the URP shaders. (There is a long comment about this in Assets/Piglet/Resources/Shaders/URP/URPColorspaceCorrection.shadersubgraph.)

Color spaces are confusing to begin with, but the quirky implementation of UnityWebRequestTexture makes things worse. I've done my best, but it's quite possible that I've made a mistake somewhere and the colors aren't exactly correct. So far, the way that I test for color correctness is to visually compare with the Babylon Sandbox glTF viewer. In the case of unlit models (e.g. the UnlitTest model), it is possible to an exact comparison using an eyedropper tool. But for lit models, doing a direct comparison is much harder, because the lighting calculations affect the final color values. In those cases, I just check that the colors look approximately the same between Babylon Sandbox and Piglet.

benvvalk commented 2 years ago

I forgot to mention:

In general, colors look much better if you set the color space mode to "Linear" under Edit -> Project Settings... -> Player -> PC, Mac, & Linux tab -> Other Settings -> Color Space.

As far as I understand, professionals use "Linear" mode whenever possible because it is more accurate/correct. "Gamma" mode is an approximation and is just there to support legacy hardware that doesn't support linear colors (e.g. old smart phones).

oprogramadorreal commented 2 years ago

Hi, Ben! Thanks for the really good explanation.

This is the sample file that I'm using: Gltf.zip

Indeed https://sandbox.babylonjs.com/ shows the same visual results that I see when I import this model in Unity using Piglet runtime GLTF importer.

What is weird to me is that the baseColorFactor values stored in the GLTF are ~(0.45, 0.49, 0.07) (as well as in the application that generated this GLTF), but in Unity (and in babylonjs) these values are different (because of the gamma correction) and the visuals are different too.

If I simply remove the gamma correction in Piglet, the visuals look the same as in the application that generated my GLTF. So I don't quite understand what the gamma correction is doing in the case of this sample file that I provided.

My project already uses linear color space setting.

Thanks again!

benvvalk commented 2 years ago

@oprogramadorreal,

I think you have made two important observations:

(1) Babylon Sandbox and Piglet produce the same color, and it is not the original color of the model. (Btw, the color in Microsoft 3D Viewer also matches with Babylon Sandbox and Piglet.) (2) The color is corrected by removing the .gamma conversion in the Piglet code.

These observations imply that the problem is not in the Piglet code but in the program/library that is exporting the glTF file. More specifically, it seems that the glTF exporter is missing a gamma -> linear conversion before it writes the color values to the glTF file.

Based on the metadata in the glTF file, it looks like you the generating the glTF file with the assimp library, yes? If so, the proper fix (and the most time consuming) is to add the missing gammar -> linear conversion in the assimp code. You have already discovered the quickest (and hackiest) workaround, which is to remove the .gamma call in the Piglet code. Another possible workaround is to write your own program/script that fixes the color values in the .gltf file, after it has been exported by assimp.

Re: your question about "what is the gamma correction doing?": I think what will help the most is to read some introductory articles about linear vs. gamma color space. This is something that every graphics/game programmer needs to learn, and it's something that I still struggle with myself. I recommend starting with Unity own documentation here: https://docs.unity3d.com/Manual/LinearLighting.html.

The TLDR is that the human eye does not respond to color intensity in a linear fashion -- it is more sensitive to differences between dark colors than differences between bright colors. (Perhaps it is a survival trait for seeing predators in the dark.) Image files (e.g. PNG/JPG files) take advantage of this quirk of the human visual system by storing the color data as gamma-encoded (sRGB) values, rather than simple linear values. The result is that dark colors are stored more accurately and than bright colors, while keeping the overall image file size the same (e.g. 8 bits for per RGBA channel = 32 bits per pixel).

oprogramadorreal commented 2 years ago

@benvvalk many thanks for such detailed response. My file was indeed generated with assimp and you nailed the root cause and all possible workarounds. Really nice that you gave me this better understanding of what is going on. So it's something that I have to work on my side and nothing you have to do on piglet. Therefore I'm gonna close this issue. Thanks again!