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

FreeType subpixel rendering support #2986

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

loicmolinari
Copy link
Contributor

@loicmolinari loicmolinari commented Jan 17, 2020

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

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.
@loicmolinari
Copy link
Contributor Author

loicmolinari commented Jan 17, 2020

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.

@ocornut
Copy link
Owner

ocornut commented Jan 17, 2020

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?

@loicmolinari
Copy link
Contributor Author

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.

@ocornut
Copy link
Owner

ocornut commented Jan 17, 2020

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.

@20k
Copy link

20k commented Jan 17, 2020

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

@loicmolinari
Copy link
Contributor Author

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

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

Successfully merging this pull request may close these issues.

3 participants