imgui icon indicating copy to clipboard operation
imgui copied to clipboard

FreeType subpixel rendering support

Open loicmolinari opened this issue 4 years ago • 6 comments

The FreeType Atlas builder is extended with support for horizontal and vertical LCD rendering, dedicated LCD hinting algorithms and a light LCD color filtering option. Each flag can be passed on a per font basis and overridden by a global BuildFontAtlas() parameter.

If at least a font requests LCD rendering, BuildFontAtlas() writes to both Alpha8 and RGBA32 textures, if no fonts request LCD rendering, BuildFontAtlas() writes only to the Alpha8 texture. If the RGBA32 texture has been written to, ImFontAtlasBuildRenderDefaultTexData() now writes default texture data in there as well.

Subpixel rendering requires dual source blending for efficient and correct blending over any destination color. That implies changes in shaders and states to set the right blending factors when LCD rendering is used. This change takes care not to break ImGui batching by keeping the nice feature of being able to render UI elements and texts using the same states. The SDL/OpenGL3 example has been updated for people to test the new feature and understand what changes should be applied in their implementations if they want FreeType LCD rendering. The FreeType version of the example can be built with a dedicated make target:

$ make example_sdl_opengl3_freetype

taking care of not breaking the default one. The FreeType README.md now contains updated test code in order to fully test the new flags.

GetTexDataAsRGBA32() has been changed to fill the R, G and B channels with the Alpha8 value instead of 0 in order to be able to use the same shading and blending states for 8-bit modes (STB, FreeType Mono and Gray) and 32-bit modes (FreeType horizontal and vertical LCD).

Changing all the colors and alpha values of ImGui themes seems too intrusive to reflect the required switch to blending in linear RGB space. So this change shows how to do the conversion in the vertex shader expecting users to either keep it that way or manually set fixed up colors in their own implementations. Not sure what's the best solution though.

Screenshot of example_sdl_opengl3_freetype:

imgui_freetype_subpixel

loicmolinari avatar Jan 17 '20 11:01 loicmolinari

Potential breaks:

  • Note sure if some users are writing to the RGB channels of the data retrieved by GetTexDataAsRGBA32(), but using these now could potentially be considered an API/ABI break.

  • Some ImGuiFreeType::RasterizerFlags have been reordered and renamed in an attempt to make them clearer. I can revert that if that's an issue.

loicmolinari avatar Jan 17 '20 12:01 loicmolinari

Thank you Loic, this is looking good.

As with many ambitious PR it may take a while until we can review all the implications of this, but I presume as is the PR will be useful to some already.

Note sure if some users are writing to the RGB channels of the data retrieved by

Using the CustomRect api people are using this to blit colorful icons into RGBA channels and have them mapped into font codepoints. Would your setup+shader be compatible with that?

ocornut avatar Jan 17 '20 12:01 ocornut

Using the CustomRect api people are using this to blit colorful icons into RGBA channels and have them mapped into font codepoints. Would your setup+shader be compatible with that?

Good catch. The CustomRect feature is clearly something I overlooked and that breaks with the subpixel fragment shader.

loicmolinari avatar Jan 17 '20 13:01 loicmolinari

Another issue is that images/textures are frequently displayed in UI and if that requires a shader change we might want to rework default back-ends. e.g. atlas texture use subpixel fragment shader, other textures could use another shader by default, but if we have "default" applied on every call that would also override the possibility to use ImDrawList callback to manually change the shader.

Would be good to explore possible solutions to that. For what it matters, only a few users are currently using colorful icons and that's still quite an under-documented feature.

ocornut avatar Jan 17 '20 13:01 ocornut

Hello! I had a look at this PR. I have no idea if this feedback is welcome, but I figured I might as well. This seems to both be a patch that implements gamma correction, as well as subpixel font rendering

// Alpha premultiplication and sRGB to linear RGB color conversion. Vertex alpha is linearized in an attempt to better match original theme colors. " float Gamma = 2.2;\n" " float LinearAlpha = pow(Color.a, 1.0 / Gamma);\n" " Frag_Color = vec4(pow(Color.rgb * vec3(LinearAlpha), vec3(Gamma)), Color.a);\n"

I'm not 100% convinced this is correct, alpha is never sRGB so you shouldn't need to linearise it - however you are actually converting linear alpha to sRGB here before multiplying it against sRGB colour, and then making it linear. This isn't right, but it works-ish because Color.rgb is actually sRGB, mostly (see caveats below). Premultiplied alpha is (linear colour * alpha), so the correct equation is Frag_Color = vec4(srgbToLinear(Color.rgb) * Color.a, Color.a) (pseudocode)

Its also worth noting that both of these equations are (though very commonly used) a bit slow and inaccurate. See here for more detail, but basically the approximations are faster and more accurate. That said, here you could get away with using a table implementation which is just a lookup, though generating them is a pain

One potential issue that you mentioned that i think is worth expanding on is that this unconditionally assumes that all colour coming into the vertex shader is sRGB and unconditionally gamma corrects which is a bit problematic. One of the wider issues around libraries like imgui or SFML (or really pretty much all open source libraries) is that they do not have a defined colour space (because its hard). So for example, there is code within dear imgui that looks like this

const ImU32 col_midgrey = IM_COL32(128,128,128,style_alpha8);

int r = ImLerp((int)(col0 >> IM_COL32_R_SHIFT) & 0xFF, (int)(col1 >> IM_COL32_R_SHIFT) & 0xFF, t); int g = ImLerp((int)(col0 >> IM_COL32_G_SHIFT) & 0xFF, (int)(col1 >> IM_COL32_G_SHIFT) & 0xFF, t); int b = ImLerp((int)(col0 >> IM_COL32_B_SHIFT) & 0xFF, (int)(col1 >> IM_COL32_B_SHIFT) & 0xFF, t);

That won't be correct under this patch (nor is it correct currently). This isn't meant as a knock on imgui (because its a great library), but literally every open source project I've ever looked at makes this error. SFML is particularly guilty here

Everyone assumes two conflicting things: That 1. you can paste colour values you find off the internet into your source code and it'll work great, and 2. you can multiply colours by values and get a sensible result out. These both can't be true unfortunately

In the end, the ideal is actually linear colour everywhere. sRGB is a black box that you can't do any maths on, so exposing it to users means guaranteed mistakes when they try and lerp colours together (combined with very expensive colour space conversions), plus you don't have to perform expensive colour space conversions in your shaders. Even more ideally, functions should clearly signpost what colour space they take and use a strong colour type, because in a linear colour everywhere world, pushstylecolor is immediately going to be misused. That said, a complicating factor is that imgui uses ImU32 everywhere which means precision loss in linear colour

I'm not really sure what the best approach is, but I thought its probably worth pointing out given that this PR might bake in some assumptions that may or may not be desirable

The other thing its worth pointing out is:

#if defined(IMGUI_IMPL_FREETYPE) // FreeType requires blending in linear space. Blending operator is Porter/Duff Over (premultiplied alpha) with subpixel support. glEnable(GL_FRAMEBUFFER_SRGB); glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC1_COLOR); #else

Linear colour stuff currently sits behind IMGUI_IMPL_FREETYPE but ideally you want linear colour even outside of freetype support, which is also what I was trying to do over here https://github.com/ocornut/imgui/pull/2943 and runs into similar issues (though the opposite) as you do by optionally converting imgui internally to linear colour. I don't particularly know what the best approach is here though, but I've apparently got enough free time to kill writing hundreds of words about colour spaces that I'm happy to try and figure it out

20k avatar Jan 17 '20 22:01 20k

Hi there. I might have a solution regarding the CustomRect issue.

CustomRect support without breaking ImGui batching implies being able to distinguish between standard and subpixel fragments. Storing subpixel texels vertically flipped allows to do so by using the sign of the partial derivative in the fragment shader. It has the benefit of being relatively efficient (DDY, SGE, MAD, MAD, MUL) and of not implying any changes in ImGui data structures.

I've just pushed the fix.

Screenshot of example_sdl_opengl3_freetype using AddCustomRectFontGlyph(): imgui_freetype_subpixel_custom_rects

loicmolinari avatar Jan 20 '20 00:01 loicmolinari