vsg-dev / vsgXchange

Utility library for converting data+materials to/from VulkanSceneGraph
MIT License
65 stars 39 forks source link

Add option to assign SRGB format to diffuse and emissive textures #176

Closed timoore closed 8 months ago

timoore commented 11 months ago

This is more desirable than converting sRGB textures to linear in a fragment shader because the hardware performs filtering correctly. It pairs nicely with setting up an sRGB swap target, so shaders operate only in linear shading space.

robertosfield commented 11 months ago

Are the formats of the images themselves not able to provide hints of sRGB? Or might assimp provide such a hint? I'm just wondering if we can avoid having to passing in options to enable use of sRGB.

timoore commented 11 months ago

Are the formats of the images themselves not able to provide hints of sRGB? Or might assimp provide such a hint? I'm just wondering if we can avoid having to passing in options to enable use of sRGB.

The colors are whatever are provided by the model, and in practice this is always sRGB for diffuse and and emissive colors, except for some very specialized cases. The option is for the benefit of the VSG. The current shaders expect sRGB values stored in UNORM texture formats and convert the colors from sRGB to liinear and back again. vsgCs, on the other hand, uploads its color textures in sRGB format and sets up an sRGB swap target, so its shaders don't do any conversion.

I haven't found anything about whether or not the sRGB encoding of a texture is exposed through assimp. JPEG is always sRGB, PNG can be linear or sRGB depending on the gamma value in the header. I don't think that stbi does anything with it.

Instead of this PR, we could always store color textures with sRGB format and change the default VSG shaders, but I thought was a bit radical. With an option, applications that want to do everything in linear space can use sRGB textures.

robertosfield commented 8 months ago

I have merged this PR as a branch:

https://github.com/vsg-dev/vsgXchange/tree/timoore-srgb-textures

And standardized the setting of the options so we can now enable the sRGBTextures option on the command line with the commit:

https://github.com/vsg-dev/vsgXchange/commit/ad2449995dcbffc2d8ee51ec6a75cf096474eed8

I have also looked at the standard_pbr.frag shader with:

#define CONVERT_TO_SRGB

#ifdef CONVERT_TO_SRGB
outColor = LINEARtoSRGB(vec4(color, baseColor.a));
#else
outColor = vec4(color, baseColor.a);
#endif

When running:

vsgviewer FlightHelmet/glTF/FlightHelmet.gltf --sRGBTextures --sRGB

It looks OK when I leave the LINEARtoSRGB conversion in the frag shader, but it's too dark when I remove it. From what I've read it looked like a sRGB colour buffer would lead to an automatic conversion. So... I'm left wondering if I've misunderstood something along the line.

robertosfield commented 8 months ago

The branch is now merged with vsgXchange master.

timoore commented 8 months ago

I have also looked at the standard_pbr.frag shader with:

#define CONVERT_TO_SRGB

#ifdef CONVERT_TO_SRGB
outColor = LINEARtoSRGB(vec4(color, baseColor.a));
#else
outColor = vec4(color, baseColor.a);
#endif

When running:

vsgviewer FlightHelmet/glTF/FlightHelmet.gltf --sRGBTextures --sRGB

It looks OK when I leave the LINEARtoSRGB conversion in the frag shader, but it's too dark when I remove it. From what I've read it looked like a sRGB colour buffer would lead to an automatic conversion. So... I'm left wondering if I've misunderstood something along the line.

The swapchain surface format needs to be set up with

traits->swapchainPreferences.surfaceFormat = {VK_FORMAT_B8G8R8A8_SRGB, VK_COLOR_SPACE_SRGB_NONLINEAR_KHR};

I don't think you've changed vsgviewer to provide that option?

timoore commented 8 months ago

Duh, you have in fact added that option. I'll try out your new vsgXchange code.

robertosfield commented 8 months ago

I added support to toggle on the sRGB format to vsgviewer via a --sRGB command line option. This is checked in vsgExamples master and the 1.1.0 release I made this morning so it it can handle --sRGB --sRGBTextures and it'll be able to load .gltf etc. models with vsgXchange::assimp setting the image.

The background colour is also automatically converted to sRGB by Window.cpp in VSG master/1.1.0.

With these two options I'm finding that the models roughly look that same, but this is without any changes to the shaders which I'm surprised by.