buildaworldnet / IrrlichtBAW

Build A World fork of Irrlicht
http://www.buildaworld.net
Apache License 2.0
122 stars 27 forks source link

CEGUI sRGB patch #263

Closed manhnt9 closed 4 years ago

manhnt9 commented 5 years ago

Regarding issue #254, here are our concerns about CEGUI:

Workflow:

Task list:

Chat log from Discord:

devsh Last Wednesday at 8:07 PM the only problem you have is that the color picker is generated as linear color... as evidenced here https://github.com/kphillisjr/cegui/blob/master/cegui/src/CommonDialogs/ColourPicker/Conversions.cpp Thats ok, but the GL_SRGB8 is not appropriate format for the color picker texture, nor possibly the sliders (depends on where the slider colors come from, a GUI skin texture, or are they a gradient of vertex colors or a CPU generated texture ?) so basically assume every texture is SRGB, except for the color picker one XD

manhnt9 Last Wednesday at 8:08 PM check this: https://bitbucket.org/cegui/cegui/src/v0-8/cegui/src/CommonDialogs/ColourPicker/

devsh Last Wednesday at 8:08 PM or convert the color picker colors to SRGB texture (pow(rgb,1.0/2.23)) before upload to GL @manhnt9 its functionaly the same as previous

oh btw, @manhnt9 @no1tuloveR this line is awful https://bitbucket.org/cegui/cegui/src/ab7eaa765cef59c85ccf87752f0884d268b657a7/cegui/src/CommonDialogs/ColourPicker/Controls.cpp#lines-1302 Makes a texture (now RGB but when you switch, SRGB) that shows blended alpha result (not true alpha) linearly.

manhnt9 Last Wednesday at 8:13 PM could we convert in the shader instead?

devsh Last Wednesday at 8:14 PM no because the user needs to see the true colors they're choosing and irrbaw will be GL_FRAMEBUFFER_SRGB always enabled

you'd have to correct those background colors with sRGB to linear RGB conversion (raise to power of 2.23), because the old colors were actually sRGB but no one realized XD https://bitbucket.org/cegui/cegui/src/ab7eaa765cef59c85ccf87752f0884d268b657a7/cegui/src/CommonDialogs/ColourPicker/Controls.cpp#lines-1311

however you still keep linear blend between the two colors (no change necessary) https://bitbucket.org/cegui/cegui/src/ab7eaa765cef59c85ccf87752f0884d268b657a7/cegui/src/CommonDialogs/ColourPicker/Controls.cpp#lines-1328

then here you either make sure that the texture is GL_SRGB8 or you apply a linear RGB to sRGB (raise to power of 1.0/2.23) transform on the output array https://bitbucket.org/cegui/cegui/src/ab7eaa765cef59c85ccf87752f0884d268b657a7/cegui/src/CommonDialogs/ColourPicker/Controls.cpp#lines-1303

devshgraphicsprogramming commented 5 years ago

Will the shader have to change for sRGB?

NOTE: This Is Extremely Important! THE ALPHA CHANNEL IS NEVER AFFECTED BY THE GAMMA CURVE ! IT IS ALWAYS LINEAR.

If your output (screen) framebuffer is created with an SRGB format (different depending on SDL2, WGL, GLX) + you glEnable(GL_FRAMEBUFFER_SRGB) then there is no conversion needed on the shader output. IrrBAW supports the above on the sRGB branch.

In order to not need conversion on the shader input, you need to specify all textures with the internal format that contains *SRGB* in the name.

However there is an exception to this rule:

  1. Non-image & non-texture shader inputs, such as vertex colors or uniform constants
  2. Linear Gradient textures (such as those in the color picker)

Let me explain nr. 1

Technically you (and the CEGUI) should have been using linear colors all along, however you haven't been so now everything in the GUI and its drawing was tweaked using colors in gamma space. So if you have some old settings/colors you should pass them through a powf(oldColor/255.f,2.2333333)*255.f function. THIS DOES NOT APPLY TO ANY RENDERING BRDF SHADER INPUT VERTEX COLORS OR UNIFORM CONSTANTS, ONLY GUI SO YOU GET COLORS CONSISTENT WITH THE OLD (WRONG) COLORS.

Let me explain nr. 2

Some gradient images (like in the CEGUI color picker) are inherently WRONG, that is the pixel colors were generated assuming a linear space but then almost reinterpret_cast into gamma space when being rendered on screen.

This is the only time when you don't want the "patched version" to look the same as the "old version". Note that this only applies to procedurally generated texture colors in CEGUI, and the color picker is the only example of using such functionality that I am aware of.

If you want to use an SRGB texture format for the gradient image, then you need to apply pow(,1.f/2.33333f) to the final pixel data just before upload. If you want to keep your colors generation function as-is then you need to use non-SRGB linear teture format JUST FOR the color gradient texture while using SRGB for all other GUI elements.

manhnt9 commented 5 years ago

Thanks, I'll refer to it. I'm preparing the CEGUI branch.

manhnt9 commented 5 years ago

I have question on texture loader. Do we need to load sRGB values or reading RGB and upload as sRGB is enough? If so, which library will we use? CEGUI has these. Screenshot_2019-04-17_15-08-50

devshgraphicsprogramming commented 5 years ago

I would rather not have you do the work twice, and use an irrbaw loader.

However from what I've gathered CEGUI loads values that are really sRGB, but falsely declares the OpenGL texture format as GL_RGB(A)8 which is a lie.

Basically all the RGB values throughout CEGUI are treated as if they were linear but they are loaded from images that are really sRGB and displayed on screens that reinterpret linear as sRGB

It (sRGB mishandling) is actually the only time where two wrongs make a right.

manhnt9 commented 5 years ago

OK, so CEGUI patch will be hold until we integrate the GUI into the BRDF Explorer of irrbaw repository.

devshgraphicsprogramming commented 5 years ago

Overall this whole PR is garbage, but this one line in this one file actually did something for PNG images https://github.com/buildaworldnet/IrrlichtBAW/pull/252/files#diff-fc43a988d27b8e7a30eecc70c94b1188R189

devshgraphicsprogramming commented 4 years ago

@AnastaZIuk you will notice that once IrrlichtBaW loads the textures for the CEGUI renderer, the "brightness/contrast" might change