inalogic / pico-pixel-public

Public issue tracker for Pico Pixel http://pixelandpolygon.com
MIT License
7 stars 0 forks source link

PicoPixel shows wrong colors with various linear RGB/sRGB textures #26

Closed ceztko closed 4 years ago

ceztko commented 6 years ago

PicoPixel shows wrong colors with textures in various formats, both gamma corrected or not. The issue is creating several problems in game productions when both developers and artists uses PicoPixel as a preview tool for textures.

The easiest way to show this problem is to create various textures filled with the middle gray color as defined by sRGB (#808080, or R=128,G=128,B=128). The reference texture is provided (middle-gray-srgb.bmp). The following textures (attached) opened with PicoPixel show a darker tone of gray:

middle-gray-srgb_BC7_UNORM_SRGB.DDS middle-gray-srgb_R8G8B8A8_UNORM_SRGB.DDS middle-gray-linear_R32G32B32A32_FLOAT.DDS middle-gray-linear_BC7_UNORM.DDS middle-gray-linear_R8G8B8A8_UNORM.DDS.

All these textures, when the renderer has been configured correctly, should show the middle gray color as the reference texture. The textures have been created using the texconv[1] tool with the following commands:

> texconv.exe -f BC7_UNORM_SRGB -srgb middle-gray-srgb.bmp -> middle-gray-srgb_BC7_UNORM_SRGB.DDS
> texconv.exe -f R8G8B8A8_UNORM_SRGB -srgb middle-gray-srgb.bmp -> middle-srgb_R8G8B8A8_UNORM_SRGB.DDS
> texconv.exe -f R32G32B32A32_FLOAT -srgbi middle-gray-srgb.bmp -> middle-gray-linear_R32G32B32A32_FLOAT.DDS # Temporary texture to do sRGB -> linear conversion
> texconv.exe -f BC7_UNORM middle-gray-linear_R32G32B32A32_FLOAT.DDS -> middle-gray-linear_BC7_UNORM.DDS
> texconv.exe -f R8G8B8A8_UNORM middle-gray-linear_R32G32B32A32_FLOAT.DDS -> middle-gray-linear_R8G8B8A8_UNORM.DDS

These textures have been tested on DDSView[2] sample program available as part of the official DirectXTex suite. Latest git must be used since it received updates to address similar issues[3]. With this program all the attached textures show same correct middle gray color. VisualStudio preview tool should not be used as reference: it has has similar issues[4] that need to be fixed as well. Some textures that are not working correctly with PicoPixel are shown correctly with VS, though.

I attached a comparison of thie BC7_UNORM_SRGB texture opened with PicoPixel and same opened with reference program DDSView, that shows the problem. Sample middle-gray textures are attached[5].

[1] https://github.com/Microsoft/DirectXTex/wiki/Texconv [2] https://github.com/Microsoft/DirectXTex/tree/master/DDSView [3] https://github.com/Microsoft/DirectXTex/commit/d5367253204887c14b52fd6b64eda3e94559bdee [4] https://developercommunity.visualstudio.com/content/problem/170399/texture-preview-tool-shows-wrong-colors-with-vario.html [5] MiddleGrayTextures.zip

comparison-picopixel-ddsview

jaytaoko commented 6 years ago

Thank you for reporting! I am looking into it.

jaytaoko commented 6 years ago

I had a look at this case:

texconv.exe -f BC7_UNORM_SRGB -srgb middle-gray-srgb.bmp -> middle-gray-srgb_BC7_UNORM_SRGB.DDS

Image middle-gray-srgb.bmp has internal format _GLRGBA8 in PicoPixel. With the texconv command above, every texel in image middle-gray-srgb_BC7_UNORM_SRGB.DDS has RGB value (0x80,0x80,0x80). Also middle-gray-srgb_BC7_UNORM_SRGB.DDS has internal fomat _GL_COMPRESSED_SRGB_ALPHA_BPTCUNORM in PicoPixe. Because of that, when the texture is sampled, opengl decodes the texels from gamma SRGB space to linear SRGB before returning the sampled texel to the shader.

0x80/0xff = 0.5 pow(0.5, 2.2) = 0.21 More details at here

This explains why middle-gray-srgb_BC7_UNORM_SRGB.DDS appears darker than middle-gray-srgb.bmp . The opengl extension provides a way to disable the decoding from gamma SRGB to linear. However by default, SRGB textures are decoded to linear space.

In the other cases, the options -srgb, -srgbi and -srgbo influence the operation of texconv but as long a long as a texture as an opengl internal format with SRGB, the process I described applies.

Let me know!

ceztko commented 6 years ago

Image middle-gray-srgb.bmp has internal format GL_RGBA8 in PicoPixel. With the texconv command above, every texel in image middle-gray-srgb_BC7_UNORM_SRGB.DDS has RGB value (0x80,0x80,0x80). Also middle-gray-srgb_BC7_UNORM_SRGB.DDS has internal fomat GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM in PicoPixe. Because of that, when the texture is sampled, opengl decodes the texels from gamma SRGB space to linear SRGB before returning the sampled texel to the shader.

Yes, this is correct and expected, and happens also on DX11 renderer. It's easier to verify step-by-step from the beginning taking middle-gray-srgb_R8G8B8A8_UNORM_SRGB.DDS, which is raw and you can inspect with hexadecimal editor: the value of the individual color components is 0x80 (0.5 in normalized float) which is 8bit gamma compressed on the source. Because the texture is marked as sRGB it means that it will be automatically decoded to linear RGB space, which by your calculations is 0.21 in normalized float. If you configured correctly your render, you will sample to a floating point number in the shader, to not loose any accuracy. Now, what it seems missing to me in the rendering pipeline to correctly render the colors is that also any intermediate buffer and the final backbuffer target views must be marked as sRGB. In this way you'll also get automatic conversion from Linear RGB -> sRGB. This will also work for textures not marked as sRGB: these won't be decoded-reencoded. In short: the sRGB chain must not be broken.

In the other cases, the options -srgb, -srgbi and -srgbo influence the operation of texconv

The meaning of srgb, srgbi an srgbo in textconv is not easy to digest but, looking at the sources of textconv, I read it this way:

Any conversion linear RGB <-> sRGB will happen only when input and output are in discord. In the first command:

texconv.exe -f BC7_UNORM_SRGB -srgb middle-gray-srgb.bmp -> middle-gray-srgb_BC7_UNORM_SRGB.DDS

There's no conversion since input and output are in accord. Instead in the command:

texconv.exe -f R32G32B32A32_FLOAT -srgbi middle-gray-srgb.bmp -> middle-gray-linear_R32G32B32A32_FLOAT.DDS

A sRGB -> Linear RGB conversion is performed and expected to decode the sRGB value to Linear RGB and store it in a (big) floating point number.

but as long a long as a texture as an opengl internal format with sRGB, the process I described applies.

Yes, but because of what you said to me my guess is that your OpenGL renderer may not be correctly configured to render sRGB textures with automatic conversion linear RGB <-> sRGB in all the steps of the pipeline. If you are proficient also with DX11, I recommend you to have a look at DDSView sample, which can render correctly all the textures in the bundle.

jaytaoko commented 6 years ago

I see your point.

For instance, in the case of middle-gray-srgb_BC7_UNORM_SRGB.DDS, PicoPixel doesn't write the image back to gamma corrected SRGB for final presentation. And this is wrong.

Textures are rendered with a shader. I see two options to correct this:

The first option is more flexible in the case of PicoPixel. This solution would apply only to texture with an SRGB format.

What about textures that are not in an SRGB format? For instance middle-gray-srgb.bmp! The processing for such textures remains unchanged. They will go through unaffected.

Let me know!

ceztko commented 6 years ago

Re-encode sampled texels to gamma SRGB before writing to the render target Enable GL_FRAMEBUFFER_SRGB_EXT which looks like the solution you added for DDSView with D3D The first option is more flexible in the case of PicoPixel. This solution would apply only to texture with an SRGB format.

I assume you would go for first option because, as you say, it's more flexible for you and for PicoPixel in general.

What about textures that are not in an SRGB format? For instance middle-gray-srgb.bmp! The processing for such textures remains unchanged. They will go through unaffected.

The textures I gave you non marked as sRGB really contains the floating point linear RGB color stored in the target texture format. In was my intention that these textures really expected a gamma correction to occur. If you follow me, there's a little ambiguity with letting textures without any mark:

  1. Do we really mean them to contain a color in linear RGB space?
  2. Are we unaware of the distinction between linear RGB and sRGB so we just store textures without any mark and "hope for good"?

I expect 2) to be very common outside here.

If we take my "non marked" textures and we do gamma correction it happens this:

  1. middle-gray-linear_R8G8B8A8_UNORM.DDS: loss of quality. The original float linear rgb color stored in a 8bit component loose accuracy.
  2. middle-gray-linear_BC7_UNORM.DDS: possible loss of quality for similar reasons to 1) ? I don't know how BC7 works but I know it's targeted for 8bit per component RGB textures
  3. middle-gray-linear_R32G32B32A32_FLOAT.DDS: no loss of quality, the original float linear rgb color fits in the texture.

So what should PicoPixel do for these textures? I gave you a bundle of textures and said: these should all shows middle-gray. This is true only if we assume no mark means that the color was stored in linear RGB space. It's really up to you what to do for these textures but I can give you a recommendation for the formats, also to not break many users out there:

In short, only for floating point textures, please do a gamma correction if needed to show correct colors in PicoPixel. This seems to me a good tradeoff between interpreting correctly advanced textures (like floating point ones) and backward compatibility. If you want, you could also provide switches/configurations to override these behaviors.

ceztko commented 6 years ago

In short, only for floating point textures, please do a gamma correction if needed to show correct colors in PicoPixel.

Only for floating and sRGB marked textures, as already said, of course.

ceztko commented 6 years ago

For instance middle-gray-srgb.bmp

This was the original textures and here colors were intended to be sRGB encoded. I think that for all these container/compressed formats (bmp, tiff, png..) the current behavior of PicoPixel is correct.

ceztko commented 6 years ago

To summarize my recommendations:

jaytaoko commented 6 years ago
  • Ubiquitous containers/compressed formats (bmp, tiff, png): keep PicoPixel current behavior;
  • Non marked, non floating point DDS files: keep PicoPixel current behavior;

It is often the cases that bmp, png and other similar image files are in the sRGB space. A user option should allow these files to be treated as in sRGB space or linear space in PicoPixel. If the user ask for a png file to be considered as having sRGB data, internaly Pico Pixel would process it as if the image has an OpenGL pixel format marked with SRGB.

  • Non marked, floating point DDS files: assume colors were stored in linear RGB and do gamma correction as needed to show correct colors in PicoPixel;

I agree that float and half-float formats should be considered to have values in linear space.

sRGB marked dds files: : assume colors were stored in sRGB and do gamma correction as needed to show correct colors in PicoPixel.

Agree.

I have started the implementation of a fix for textures marked as sRGB. Thanks for your feedback!

jaytaoko commented 6 years ago

An update on this issue. For an RGB8 texture that is not srgb, an option to instruct PicoPixel to load the texture as if it from and srgb image file. Also an option to apply gamma correction to the sampled texel on display.

rgba

These options are not available if the texture is srgb. In which case, it is always decoded to linear when sampling and texels are gamma corrected for presentation.

srgb_format

ceztko commented 6 years ago

Looks good! Looking forward for the next release to give you some feedback.

ceztko commented 6 years ago

Hello! Any update on this issue? Release seemed close. :)

jaytaoko commented 6 years ago

Sorry for the delay. A few days!

jaytaoko commented 6 years ago

Here is pre-release build PicoPixel 0.7

There might be a few bugs yet but try it and see how the image a processed. Your feedback is appreciated.

Thank you for your patience!

middle-gray-srgb.bmp

__middle-gray-srgb_BC7_UNORM_SRGB.DDS__ _texconv.exe -f BC7_UNORM_SRGB -srgb middle-gray-srgb.bmp -> middle-gray-srgb_BC7_UNORMSRGB.DDS

__middle-srgb_R8G8B8A8_UNORM_SRGB.DDS__ _texconv.exe -f R8G8B8A8_UNORM_SRGB -srgb middle-gray-srgb.bmp -> middle-srgb_R8G8B8A8_UNORMSRGB.DDS

middle-gray-linear_R32G32B32A32_FLOAT.DDS texconv.exe -f R32G32B32A32_FLOAT -srgbi middle-gray-srgb.bmp -> middle-gray-linear_R32G32B32A32_FLOAT.DDS # Temporary texture to do sRGB -> linear conversion

middle-gray-linear_BC7_UNORM.DDS texconv.exe -f BC7_UNORM middle-gray-linear_R32G32B32A32_FLOAT.DDS -> middle-gray-linear_BC7_UNORM.DDS

middle-gray-linear_R8G8B8A8_UNORM.DDS texconv.exe -f R8G8B8A8_UNORM middle-gray-linear_R32G32B32A32_FLOAT.DDS -> middle-gray-linear_R8G8B8A8_UNORM.DDS

Note that the ui color swatch indicates the texel RGBA value as it is read from the file.

ceztko commented 6 years ago

Thank you very much! Unfortunately, on Windows10 1803 64bit it seems to me PicoPixel is nonfunctional. The window looks like a Metro app, with no borders/bars, completely black and textures loading is not working at all, both with drag and drop or "Open With" dialog. Please look at the video in the link[1].

[1] https://www.dropbox.com/s/rv0hnd42gk4x9ag/PicoPixel-0.7-testin1.mp4?dl=0

jaytaoko commented 6 years ago

Wow, this is bad! May I ask which GPU you have on your system?

ceztko commented 6 years ago

Good catch! I have optimus system integrated+discrete GPU (intel HD graphics+ Nvidia gtx 860M). Integrated is definitely picked first because if I force discrete I get usable UI. As ususl Nvidia gpu is definitely more tollerant than intel :). I just briefly tested, I will give you more info later.

jaytaoko commented 6 years ago

Good to know! Still, I corrected a bug that may improve things overall. Please try this verion PicoPixel 0.7.1

ceztko commented 6 years ago

Sorry for late reply, I got extraordinary busy last week. Unfortunately 0.7.1 didn't fix the integrated vs discrete GPU problem. I attached the listing of GPUs in my system sysinfo-gpu.txt. Still, forcing PicoPixel to use nvidia I can provide you some feedback. First: interpretation of textures is now perfect and reflects exactly what we agreed on, very good job! Thanks for finding the time to properly deal with colors/gamma correction issues. It follows now some feedback on usage of the new version of PicoPixel:

I don't need to use PP much nowadays, but I guess I gave you enough to work on anyway, sorry :D .

[1] https://www.dropbox.com/s/tewtqqpeomik0ko/PP-resize-layout-events.mp4?dl=0

jaytaoko commented 6 years ago

Thank you for your feedback. I don't have a setup with an integrated intel GPU to try but I will look into it. The issues you mentioned are indeed very valid points. I am in the process of cleaning up and fixing many issues in PicoPixel before a relaunch. It is a bit slow but progress is steady. This srgb issue as actually sped things up a notch! Thanks!

ceztko commented 4 years ago

I saw a new version all seems now handled way better! Only one point didn't change but I guess it has less importance[1]. I guess we can close this.

[1] In the options menu, clicking "sRGB input" and "Gamma correction" should have immediate, visible side-effect (eventually resetting any user override in the main UI bar).

jaytaoko commented 4 years ago

I will take a look at point [1]. Thanks for your feedback.