sRGB support

[20k]

Branch: Master
Backend: imgui_impl_glfw + imgui_impl_opengl3
Compilers tested: windows + msys2/mingw64, on AMD

Screenshots
ImGui rendering a rectangle with a colour of IM_COL(128, 0, 0, 255) (incorrect)

ImGui with sRGB support enabled, rendering the same rectangle:

Technical background:
Currently ImGui is sRGB unaware - which means that blitting operations and general mathematical operations are done in sRGB, rather than the correct colour space which is linear colour. This is incorrect (though very commonly done), and while it can often be worked around, proper linear colour support is necessary for subpixel font rendering, of which this is (hopefully!) the first patch. This follows on from the discussion over here #2468

What this patch does:
This patch adds an optional linear colour mode to ImGui, which can be enabled with ImGui::SetStyleLinearColor(true); This converts ImGui's style table from sRGB to linear colour (or vice versa), and sets the config flag ImGuiConfigFlags_IsSrgb appropriately. It also adds optional helper functions to help manage styles correctly (push/getstylelinearcolor etc), though these can be removed if deemed not necessary

Caveats:
This patch currently stores the sRGB enabled-ness flag in ImGuiConfigFlags, with ImGuiConfigFlags_IsSrgb which is more of a user accessible flag. I'm not sure this is ideal - if this is fine I'll update the comments, otherwise I can rework this patch to store the status somewhere else

Test code:
There's a simple test for this over here https://github.com/20k/imgui-test-srgb/blob/master/main.cpp

[ocornut]

Hello James,

Just a quick word to say Thank you for this.
I do not know when we'll have time to tackle this seriously but this seems like a solid base.

Linking to #1724 and #578 as well.

[20k]

No problem! I'm active in the discord, so feel free to give me a ping whenever you have time to hash out the details of how this PR should look

[ocornut]

Spent a little time today trying to parse all the super useful information written in e.g. #578, still struggling to turn that into actions which will allow dear imgui to be used by everyone in every situations. Thanks for your patience and bearing with my confusion :)

On your PR as it stands today, when enabling srgb in your PR the general style colors are noticeably different. As I understand this is due to colors defined in the style as using blending and because we cannot possibly provide a perfect style conversion for those colors. Am i wrong? Would you recommend any workaround? Should we be authoring style for both modes? Should we aim to rely less on alpha blending styles to attenuate the issues?

The current default styles rely on colors using Alpha <1.0f as a convenience because it indirectly allows creating more suitable variants of colors accross multiple background colors. But in cases where we can assume what the background color was, we could perfectly replicate the same final color with no alpha blending. That is to say, we could decide to transition existing styles to use less colors with Alpha < 1.0f, and that transition could technically occur ahead of the rest of srgb/linear work if we decide so. Based on the take aways, when the time happens to redesign the styling system we can decide to design them with less transparency if required.

(PS: I have just added a simple B&W gradient (with configurable numbers of steps) in Demo>Examples>Custom Rendering.)

(tagging didactic color space grandmaster @matt77hias)

[matt77hias]

Hey,

I have no experience with subpixel font rendering, so please feel free to correct me if I am missing the bigger picture of which this PR is the first step.

Given that colors can be converted to linear on the CPU side, I wonder why the default vertex is still defined as (or am I missing some IMGUI_OVERRIDE_DRAWVERT_STRUCT_LAYOUT?):

struct ImDrawVert
{
    ImVec2  pos;
    ImVec2  uv;
    ImU32   col;
};

col has 8 bit color components (R8G8B8A8), which is sufficient for sRGB, but not for linear RGB. So if you want to do the linear-to-sRGB color conversions on the CPU, I would expect some float4/half4, etc. to be passed to the GPU. Furthermore, does this include the color picker window as well?

Why not just providing a macro to enable the sRGB-to-linear conversion in the vertex shader? (And doing the lerping of ShadeVertsLinearColorGradientKeepAlpha in linear RGB instead of sRGB color space).

[20k]

@ocornut Hi there!

So the reason why styles look different is that I only convert the colour component to linear colour, and ignore alpha

It would be possible to fix this in two ways

1. Multiply the sRGB colour by alpha, and then convert the result to linear colour and set alpha to 1
2. Convert the raw sRGB colour to linear colour, and then calculate the alpha value that makes that linear colour map to the value in 1. once blending is applied

Both options will still result in different colours if the user changes the alpha themselves though. I'm happy to implement a solution to this when I hit my desktop again on monday

Multiplying sRGB colours by alpha is generally incorrect, so less of that in the styles would be ideal if there is a future effort to redo this

[20k]

@matt77hias So you're 100% right on the problem of 8-bit linear colour in the vertex definition, however imgui's API overall actually suffers from this extensively, so making it floating point actually wouldn't help much. Eg, lots of APIs here take ImU32s which is a problem, and so avoiding crushed colours in the darker end is pretty much an end to end revamp, which is why I avoided making that change (ie to take advantage of the float vertex colours, imgui needs a big rework)

I'm unsure on the colour picker as this patch was a while back and I need to check exactly how imgui handles it, I can check on monday when I hit my regular environment again

So: sRGB to linear in the vertex shader is a bad idea basically, because it codifies imgui's internals as being sRGB, which is a heavily mistake prone colour space (eg as we can already see with the 'linear' colour gradient function). People expect to be able to perform mathematical operations on their colours, which isn't true. Having to convert sRGB to linear and back to perform simple maths is very expensive, whereas you should ideally generally treat sRGB as a black box. This makes linear colour a good exposed API type, with the exception of when people are likely to plug colours in off the internet

Colour space conversions on a GPU aren't free either so putting that into a shader can make a noticable performance impact. Its also not that extensible to users that use other colour spaces with different transfer functions (eg apple P3)

In terms of just modifying the functions to be correct, the main problem is that imgui currently doesn't have a defined colour space at all. Some of it assumes linear colour, some assumes sRGB, and requires constant awareness not to do the wrong thing. Generally people are not that great and handling this kind of thing, not to mention the performance implications of colour space conversions

Ideally, imgui would use some sort of obvious strong type at the API boundary (eg linear_srgb_col) such that users cannot easily stick sRGB colours into it, deprecate all the old ImU32 APIs, fix up the internals so that they're always linear (even if sRGB mode isn't enabled!), and then purely use the sRGB flag to turn off and on sRGB framebuffers. This is retrofittable into the current design here, and would be the long term goal

[20k]

I've pushed a version that shrinks the differences in styles between sRGB and linear colour mode. I should also note here that the colour pickers are currently 'incorrect' and need to be explicitly reworked to work in sRGB, as that's one of the few cases where using sRGB is correct

[sergeyn]

Hi,
Would fp16 be a good format for colors ?
I'd suggest modifying ImDrawVert's 'col' to be fp16 and auto-convert when writing to it as a start until all places use some common color type. Converting to fp16 is not a big deal performance wise, intel CPUs (and AMD to my knowledge) can do conversion in hardware and I expect no noticeable performance difference.

Regards.

[20k]

Hi sergeyn!

FP16 is a good format for linear colour here, because it distributes precision somewhat similarly to gamma correction. That said, 16-bit integer per channel would also be more than enough precision for linear colour here, and might be less complex to implement. Either's fine basically

I plan to try a full redo of ImGui to fix all the colour management in the future (although this is a reasonably large job) once I've finished up the implementation of a colour system I intend to propose for C++ standardisation. It will likely be quite some time before this is ready, and even longer before I get around to ImGui

[sergeyn]

with FP16 you can also specify HDR colors. I think it's not that much work to start with internals, ie. - the vertex format, and when that works, push up the color up to user-interface level.

[20k]

That's also true. The main issue with converting ImGui to be correctly linear is that it has no consistent concept of a colour space, eg in this branch the colour pickers are incorrect (because they really are meant to interpolate in sRGB as coded). Fixing all this requires deprecating all the existing APIs which use colour, and auditing all the usages of colour internally - as well as probably breaking changes to existing styles to some degree. Additionally, linear colour vs sRGB is generally not well understood by developers, so on a more pragmatic note in terms of maintenance burden I'd like to ensure that a change like this can't be accidentally gradually un-fixed over time (or misused by users who need to do this correctly), which means in practice such a massive API change wants to come with strong typing and a bit of library support

On an even more pragmatic note though, there was strong committee support for a C++ colour proposal in Prague in SG13, so if the proposal turns out to be any good, using (future) standardised functionality in the user facing API would be ideal. If it does turn out to be suitable, I'll probably also create a patch set for SFML too so the interop there would be nice

That said, if you do want to start some of this work I'm happy to help, this PR was to try and get the ball rolling on what actually needed to be done to get the very basics into ImGui that could be built on

[sergeyn]

Vertex format is part of the ImGui interface and messing with it will screw up existing integrations. I personally like not to care much about backwards compatibility in my own code, but this most certainly doesn't cut with maturity of this library. As I see it, keeping it backwards compatible would require a flag on which vertex format to output, while the rest of logic can assume Linear color space. This way without explicitly opting in for new high-res color format in vertices you get things still working but with crushed blacks by default. Omar should probably comment on how he sees this problem.

[sanbox-irl]

any update on this?

[tksuoran]

It would be good to keep track of the current color space of the style. That would allow this function to do the right thing even if style already was in the requested color space. Currently, you need to externally figure out if calling this function will work, otherwise style colors will be in broken color space.

[20k]

Hiyas, thanks for the comment. You're right in that styles could document whether or not they are linear colour, so that users could use this function a little more freely in more general contexts, and convert their existing styles safely and on the fly

The original design for this PR was that there'd be no mix and matching of colour spaces however - once ImGui was in linear colour mode, absolutely everything would be in linear colour - so this function is essentially a bit of a hack for the built in imgui styles

That said this PR seems to have become a tad stuck, so I'm not really sure where I'm going with this!

[tksuoran]

I think these commits are really close to be something really useful.
It might help to split this PR to smaller ones.

I think the absolute minimum is ForceStyleColorSpaceConversion() and the conversion functions it needs. IMHO it would be better if we did not call it 'force' if it is not aware of the current color space. Instead, if we call it something like ConvertFromSRGBToLinear() / ConvertFromLinearToSRGB(), then it is slightly more visible to the user that the function just does a color conversion blindly, and user better know.

I would be very happy if there was a PR which only provided this utility function and nothing else. It would break nothing, and would have value of allowing people to get correct rendering more easily. The other fixes can be done later as separate PR.

[20k]

any update on this?

I'm unsure - this PR is somewhat waiting on direction of what to do. There's a few different choices that need to be made by someone who isn't me

1. Should the user facing colour type of the API be sRGB or linear colour?
2. Do the user facing colour APIs need to be fully rewritten using strong typing, or is the breakage too high if everything gets deprecated?
3. Should ImGui be rewritten using linear colour internally (less difficult than it sounds, this PR does a lot of that), or should it use sRGB and convert in a shader?
4. Should ImGui be strongly opinionated in the colour space of the user facing API (eg 2.), or should it be flexible like in this patch where the colour space depends on a global flag?

Personally I would advocate: 1. Linear, 2. Rewrite and deprecate, 3. Linear internally, and 4. Strongly opinionated. This however is an argument from a technical perspective, and not one from the perspective of managing a large and widely used project like ImGui that has stability guarantees

I am happy to do the work if consensus on direction can be achieved, and there's sufficient interest on the development team to review and merge something like this (because I still use my linear colour font rendering branch to this day!)

[sanbox-irl]

Hmm that's interesting. IMO I am actually nearly the opposite!

Just to be clear on how simplistic of a job ImGUI could do here:

Simply just running the EOTF (sRGB -> linear) in a shader, imgui's colors can be converted to be in linear space and life is fine (assuming the framebuffer is set up to be in linear space -- we can have a #DEFINE for that)

This would require simply updating the example implementations for each backend, and any users who are using them would be automagically good, and in the future, users who are looking at the example impls for their own implementation, would see that shader and add it to their code.

This is very inelegant but DOES get us to the place where imgui works correctly with its colors. In fact, this is already what the wgpu-imgui-rs backend impl does here: https://github.com/Yatekii/imgui-wgpu-rs/blob/master/src/imgui.frag#L21

I would personally be in favor of something a bit stronger than that, but simply:

1. sRGB (the color picker is the one we care about, and photoshop's color picker is in sRGB, which is much better anyway)
2. No break
3. Linear, and ammend the vertices produced to make a float 4 array instead of the u8 4 currently. This will be a breaking change.
4. I think it should be strongly opinionated that it's producing linear colors. Honestly, the worst that happens is the colors are slightly wrong for people who have set up their graphics backends incorrectly, which isn't the worst thing.

The most important thing, imo, to keep in mind, is that only, saying this as one!, nerds care about this.

Like most artists don't even care about this -- they can fiddle with the colors enough that the incorrect tinting that happens when working in sRGB instead of linear doesn't matter and produces the colors they want. So our breaking change, if made, needs to be gentle and simple. It shouldn't require understanding what "electro-optical transfer function" means, because people will just check out of that convo

(edited slightly for clarity on the shader)

[20k]

This would require simply updating the example implementations for each backend, and any users who are using them would be automagically good, and in the future, users who are looking at the example impls for their own implementation, would see that shader and add it to their code.

The fragment shader approach there isn't a complete fix unfortunately, because while ImGui is a wonderful piece of software, its approach to colour management is similarly haphazard to everybody else. At least some of ImGuis internals are actually incorrect when it comes to colour management, so at the very least some changes need to happen within ImGui to fix this. This PR is very minimal though, because the intent was partly to get some discussion started on direction - but there are eg blending functions within ImGui that are incorrect. Up above I went through some more detailed reasons for why I'm not convinced that codifying ImGui's internals as being sRGB is a good move

Someone needs to go over it with a fine tooth comb and redo it all to be correct, and in a way that makes it easy for future developers to keep it correct. Ideally internally it'd all be linear, and type safe as well. Its less work than it sounds - ImGui mostly passes through colours to the backend without touching them much, but ensuring maintainability into the future by people who aren't massively colour management aware will likely involve breaking API changes. The other alternative - sRGB internally, convert on the fly, is... bug prone at best, and anything which does any kind of colour blending would involve expensive colour space conversions

sRGB (the color picker is the one we care about, and photoshop's color picker is in sRGB, which is much better anyway)

Its worth noting that the colour pickers could still remain in sRGB, but the API could simply deal in linear colours. There's a slight performance loss here as you have to colour convert the output, but it wouldn't be too noticable as its only one value. Linear colour as an API choice in general is less expensive if imgui is linear internally, but given that 32bit sRGB -> linear can be implemented as a table lookup, its not the end of the world either way. With a strongly typed API, the implicit conversion issue with a colour picker could be avoided entirely

The most important thing, imo, to keep in mind, is that only, saying this as one!, nerds care about this.

Linear colour management is fairly important within the games industry for eg PBR, it crops up a bunch. Professional art software (eg photoshop), or video editing software needs to work correctly as well, and at least some professional artists are very aware of colour space issues! The motivating reason for this PR is good text rendering though. ImGui's text rendering for freetype fonts is pretty blurry by default, and without something like this it can't do subpixel font rendering correctly unfortunately. This has cropped up a few times in issues here and there, I get pinged every now and again by people asking me if there's been any movement on the font rendering or anything

[sanbox-irl]

The fragment shader approach there isn't a complete fix unfortunately,

Did not realize this! Yes, converting ImGui to a strongly typed linear color management internally sounds necessary to fix then. Especially for the font issues at the end.

nerds

I agree, color management is important. I think switching over to internally use linear but present an sRGB exterior would be preferable.

[tksuoran]

I would be perfectly happy with compile time configuration, I see no need for runtime support multiple color spaces. I would also go all linear, with float per color component.

[sanbox-irl]

Imo, breaking the color interface from ImColor32 to linear color isn't worth it. We should just wrap those into linear and go from there and then everything inside can be in linear space.

[AnyOldName3]

Would it be wildly inelegant to leave the current functions that take an ImColor32 in the interface as wrappers that do a from-sRGB conversion and then call a new set of overloads that take a new ImColorFloat64 that become the new recommended interface for people who understand colour spaces? I've not dug too deeply into how Dear ImGui uses colours, but on the face of it, this seems like it'd allow the fast use-linear-everywhere approach for people who know what they're doing while minimising breakage for people who don't and have probably been entering sRGB colours the whole time.

[meshula]

I'm not sure a jump to double is necessary? The math doesn't demand that precision. The type could be satisfied by being more explicit, ImColorLin32 rather than taking different behavior on bit depth?

[AnyOldName3]

I was meaning four half-floats at sixteen bits each for sixty-four bits total as the ImColor32 in the last comment before mine was referring to for 8-bit ints by their combined total.

[meshula]

Thanks, now I get it, I didn't make that mental leap ;)