Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
ceztko opened this issue May 9, 2018 · 22 comments
Closed

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

ceztko opened this issue May 9, 2018 · 22 comments

Comments

@ceztko
Copy link

ceztko commented May 9, 2018

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] microsoft/DirectXTex@d536725
[4] https://developercommunity.visualstudio.com/content/problem/170399/texture-preview-tool-shows-wrong-colors-with-vario.html
[5] MiddleGrayTextures.zip

comparison-picopixel-ddsview

@jaytaoko
Copy link
Contributor

Thank you for reporting! I am looking into it.

@jaytaoko
Copy link
Contributor

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 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.

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
Copy link
Author

ceztko commented May 11, 2018

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:

  • srgbi: interpret input as sRGB encoded;
  • srgbo: expected output is sRGB;
  • srgb: interpret input as sRGB encoded, expected output is sRGB.

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
Copy link
Contributor

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:

  • 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.

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
Copy link
Author

ceztko commented May 12, 2018

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:

  • R8G8B8A8_UNORM: interpret it as sRGB (so keep PicoPixel as is for these textures);
  • BC7_UNORM: interpret it as sRGB (so keep PicoPixel as is for these textures);
  • R32G32B32A32_FLOAT: please, interpret is as colors were stored in linear RGB space!
  • (not in the bundle) BC6H and other floating point textures: please, interpret them as colors were stored in linear RGB space!

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
Copy link
Author

ceztko commented May 12, 2018

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
Copy link
Author

ceztko commented May 12, 2018

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
Copy link
Author

ceztko commented May 12, 2018

To summarize my recommendations:

  • Ubiquitous containers/compressed formats (bmp, tiff, png): keep PicoPixel current behavior;
  • Non marked, non floating point DDS files: keep PicoPixel current behavior;
  • 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;
  • sRGB marked dds files: : assume colors were stored in sRGB and do gamma correction as needed to show correct colors in PicoPixel.

@jaytaoko
Copy link
Contributor

  • 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
Copy link
Contributor

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
Copy link
Author

ceztko commented Jun 19, 2018

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

@ceztko
Copy link
Author

ceztko commented Sep 5, 2018

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

@jaytaoko
Copy link
Contributor

jaytaoko commented Sep 6, 2018

Sorry for the delay. A few days!

@jaytaoko
Copy link
Contributor

jaytaoko commented Sep 9, 2018

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

  • PicoPixel: BGR8_UNORM
  • Requested: Ubiquitous containers/compressed formats (bmp, tiff, png): keep PicoPixel current behavior
  • Actual: Even though the sRGB tag is missing, by default PicoPixel load the image as if it were sRGB. This can be changes in the options ui. The output image is gamma corrected. Toggle srgb input and display gamma buttons for variations in the image process.

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

  • PicoPixel: BC7_UNORM_SRGB
  • Requested: sRGB marked dds files: assume colors were stored in sRGB and do gamma correction as needed to show correct colors in PicoPixel.
  • Actual: The image is automatically loaded as sRGB. That is, it is converted to linear on texture sampling for operations and converted back to sRGB by PicoPixel for display. The ui srgb input and display gamma cannot be interacted with. They are both forced on.

middle-srgb_R8G8B8A8_UNORM_SRGB.DDS
texconv.exe -f R8G8B8A8_UNORM_SRGB -srgb middle-gray-srgb.bmp -> middle-srgb_R8G8B8A8_UNORM_SRGB.DDS

  • PicoPixel: RGBA8_UNORM_SRGB
  • Requested: sRGB marked dds files: assume colors were stored in sRGB and do gamma correction as needed to show correct colors in PicoPixel.
  • Actual: Same as middle-gray-srgb_BC7_UNORM_SRGB.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

  • PicoPixel: RGBA32_FLOAT
  • Requested: 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;
  • Actual: The image is assumed to have linear data. The ui srgb input button cannot be toggled on. The ui display gamma maybe toggled on/off.

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

  • PicoPixel: BC7_UNORM
  • Requested: Non marked, non floating point DDS files: keep PicoPixel current behavior;
  • Actual: By default, PicoPixel loads the image as if it were sRGB (see the option in the settings). Both ui buttons srgb input and display gamma may be turn on/off. This particular image has been converted from a linear data texture. The best way to view it is with srgb input turned off (since the data is already linear) and display gamma turned on.

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

  • PicoPixel: RGBA8_UNORM
  • Requested: Non marked, non floating point DDS files: keep PicoPixel current behavior;
  • Actual: Same as middle-gray-linear_BC7_UNORM.DDS

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

@ceztko
Copy link
Author

ceztko commented Sep 9, 2018

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
Copy link
Contributor

jaytaoko commented Sep 9, 2018

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

@ceztko
Copy link
Author

ceztko commented Sep 9, 2018

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
Copy link
Contributor

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

@ceztko
Copy link
Author

ceztko commented Sep 15, 2018

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:

  • PicoPixel seems now very slow to load the first texture. In the time take to load a texture (around 1sec) I'm able to take a screenshot of the background canvas which looks like a checkerboard. Renderer initialization is slow?
  • The following textures load faster but the loading bar persist for some seconds even after having loaded the texture;
  • Some buttons and label texts have not enough contrast, making them hard to read;
    image
  • The UI layout operations during minimize, maximize, resize of the window are in some case too visible. IMO, all intermediate states should not be rendered only the finalized render operation should bump and be visible to the user. See the video[1];
  • All the bars and the canvas resizes together with the whole window, which is not desiderable. See the video[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).
    image

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
Copy link
Contributor

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
Copy link
Author

ceztko commented Jan 28, 2020

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants