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

feat: implement subpixel rendering #2092

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

Conversation

paxcut
Copy link
Contributor

@paxcut paxcut commented Jan 29, 2025

Proof of concept for implementing subpixel processing in ImGui. This is work in progress, and it is bound to have problems.

What it does:

  1. Uses freetype own subpixel processing implementation to build a 32-bit color atlas for the default font only (no icons, no unifont) . 2) Avoids pixel perfect font conversion when possible. 3) Self contained, no ImGui source code changes.
  2. Results in much improved legibility of fonts rendered on low dpi LCD screens that use horizontal RGB pixel layouts (no BRG or OLED or CRT if they even exist anymore)

What it doesn't:

  1. Fancy class based interface. The code is barely the minimum needed to show it can work. 2) Dual source color blending. That needs to be implemented in shader code, so it needs to change ImGui source code although minimally. This will result in some characters appearing dimmer than others. Easily fixed with small fragment and vertex shaders. 3) subpixel positioning. If characters are very thin they will look colored, or they can be moved to improve legibility. 4) deal with detection of fringe cases including rare pixel layouts, non LCD screens, Mac-OS not handling subpixel rendering and any other deviation from the standard LCD. 5) tries to be efficient in speed or memory use. Font Atlases will be 4 times the size they were before, but there are no noticeable delays in font loading in the examples I have tried.

Any comments and code improvements are welcome.

paxcut and others added 12 commits January 29, 2025 06:59
Proof of concept for implementing subpixel processing in ImGui. This is work in progress, and it is bound to have problems.

What it does:
1) Uses freetype own subpixel processing implementation to build a 32-bit color atlas for the default font only (no icons, no unifont) .
2) Avoids pixel perfect font conversion when possible.
3) Self contained, no ImGui source code changes.
4) Results in much improved legibility of fonts rendered on low dpi LCD screens that use horizontal RGB pixel layouts (no BRG or OLED or CRT if they even exist anymore)

What it doesn't:
1) Fancy class based interface. The code is barely the minimum needed to show it can work.
2) Dual source color blending. That needs to be implemented in shader code, so it needs to change ImGui source code although minimally. This will result in some characters appearing dimmer than others. Easily fixed with small fragment and vertex shaders.
3) subpixel positioning. If characters are very thin they will look colored, or they can be moved to improve legibility.
4) deal with detection of fringe cases including rare pixel layouts, non LCD screens, Mac-OS not handling subpixel rendering and any other deviation from the standard LCD.
5) tries to be efficient in speed or memory use. Font Atlases will be 4 times the size they were before, but there are no noticeable delays in font loading in the examples I have tried.

Any comments and code improvements are welcome.
Icons become unaligned because the default font's descent is not calculated until the atlas is built. Atlas building must be delayed until custom rects are defined and besides, building an atlas just to obtain the descent is something that ImGui font implementation makes necessary.

All font and glyph metrics are accessible when they are loaded by specifying some desired size using the freetype api. It takes a little bit of extra code, but it is the most efficient way to obtain metrics.
The dual double loop done inside the final rendering, one to remove the padding and the other to move the data to the atlas rect have been merged into one double loop that loads the atlas data directly from the bitmap saving time and memory.
The dual glyph rasterization, one during rect creation and the other during Atlas final data write have also been made more efficient. All the bitmaps are saved into a map from rect id to bitmap.
In the debug version on my slow computer fonts load in about 4 seconds. In comparison, a nightly takes about 2 seconds.
…ntation.

Most, if not all, the text tendering in imgui is done through RenderText so callbacks are inserted before and after all text related draw calls to turn on and off special settings that control subpixel rendering. Dual source  blending is required in this case and depending on the theme color we use the srgb render buffer or not. A parameter with Gamma correction is passed using an unused matrix element of the projection matrix (2d vs 3d) to the shaders which can use it to determine if subpixel rendering code should be executed. To compensate for the nonlinear color blending that has been the de facto standard a gamma correction is applied to the color of the text.

On the freetype side a bitmap with three times the sampling frequency in the x direction is first produced. the bitmap is further processed using the standard 5 tap lcd filter and the resulting glyphs are loaded into custom rects of the atlas.

The resulting rendered text is clearly sharper than plain anti-aliased and there are no color fringes that can be detected unless considerable zoom is used.

The pr will remain as a draft until further testing of various types of fonts and sizes can be performed, but it should compile and run fine under the master code base.
…now. Fedora needs to have cmake updated so that should be resolved once the scripts are updated to use a new version.
… source blending. Msvc complains because I used a negative sign on an unsigned number.
…using distortions in the text layout.

The reason for the error was using different rasterizations of the glyph to calculate the bitmap (x3 wide) vs the one to get metrics (x1 wide) the fix was to use the tripled x resolution bitmap also to get the metrics, but dividing x metrics by 3.
}

void UseFontShaders(const ImDrawList *parent_list, const ImDrawCmd *cmd) {
useFontShaders = !useFontShaders;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd use two callbacks probably, one that enables the font shader and one that disables it instead of having one that toggles it. That way, when reading the code, you know exactly when the shader is active and when it isn't

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the change and committed. Please review it again at own convenience.

@@ -256,4 +283,152 @@ namespace hex::fonts {
std::list<std::vector<u8>> m_fontData;
};


class Bitmap {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably move these classes into their own files since it's quite a bit of code and not immediately related to the font atlas

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can move them but the only reason these are needed is to construct the font atlas custom rects. freetype bitmaps are returned in special buffers and cannot be stored separately from the rest of the bitmap information related to the glyph. The bitmaps contain only the information needed to write them in the next stage after building the atlas with the empty rects, but we need to process all the rects for all the glyphs in all the fonts that go in this atlas first.

The same is true for the RGB type. it is only a helper to add an alpha value to the rgb that is generated by the subpixel filtering stage. Both are only needed to create the atlas. I am not sure where else they would go.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved code to their own files and deleted old and unused code.

//void blend( Bitmap &bitmap, ImU32 x, ImU32 y);
};

class RGBA {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to match the style of the rest of ImHex here. For example remove extra new lines and spaces, use camelCase for the functions, etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sorry about that. Some parts of the code started out as part of Imgui source code so it was written to match imgui style. I changed it back to use imhex style as suggested.


#include <font_atlas.hpp>

namespace hex::fonts {

bool BuildSubPixelAtlas(FontAtlas *fontAtlas) {
FT_Library ft = nullptr;
if (FT_Init_FreeType(&ft) != 0) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't freetype initialized by ImGui already?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the name of this function can be confusing. What it does is create an instance of the library that is independent from any other. A different instance is created in the previous function just to calculate the ascent and descent of the fonts without having to build and atlas. The ft library instance is destroyed when we exit the function by calling a Done function which frees all the memory from the faces, glyphs drivers and anything else.

fontName = io.Fonts->ConfigData[i].Name;

std::ranges::transform(fontName.begin(), fontName.end(), fontName.begin(), [](unsigned char c) { return std::tolower(c); });
if (fontName.find("proggy") != std::string::npos || fontName.find("unifont") != std::string::npos) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there maybe a better way to detect if we should skip fonts instead of just matching for names? e.g what happens if you'd load a pixel perfect TTF font?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no real way to characterize a font as being pixel perfect that I know of other than inspecting it. Proggy clean and unifont are not bitmap fonts. They are scalable and described using contours like any other true type font. If you want to use some pixel perfect font and you know what its native size is, you just wouldn't use subpixel rendering and use none for antialising. Those two need to be listed because they are part of the standard 4 fonts we load for each choice of font.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay in that case I think it would be better that we specify if they need anti-aliasing or not when registering them, e.g when calling registerFont have it be an argument there. That way it's more easily expandable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with that is that at this point we are only using the font configuration data stored in the atlas and we don't have access to the font map returned by getFonts(). I load the names of the fonts on the previous function call just so I can filter the fonts that cannot be resized and don't need subpixel rendering.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still working on this. I need to find some way to pass the information about which internal fonts cannot be resized so they wont be processed for subpixel rendering. maybe we can use a name like unresizableFont to all the fonts that need to be skipped instead of their explicit names since the names are erased as soon as the texture is registered anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I implemented a system where fonts now include a field that specifies if they are scalable which can be optionally set when registering the font. Fonts that are not scalable cannot be anti-aliased or sub-pixel rendered as that requires scaling the font. Currently only one font (unifont) being registered uses this property.

@@ -79,6 +79,112 @@ namespace hex::fonts {
return true;
}

ImU32 Bitmap::crossCorrelate(ImU8 *input, ImU32 inLen, ImU32 skipY) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also doesn't really belong here. Put that in its own file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a lot of code that was leftover from earlier trials of different things that needed cleaning up. I have removed most of the unused stuff and will be committing those changes when we resolve the issue with the disabled text in the settings of the fonts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code was moved/removed. Please check it again when you have the time.

paxcut and others added 3 commits March 11, 2025 08:59
. Added on and off callbacks for font shader control.
. Moved new classes and functions to their own file. Under imhexlib in helpers with the name of freetype so that it can be expanded if needed in the future.
. Removed old and unused code.

Additionally, the following changes are also included:
. Fixed problem with fonts becoming transparent and leaking the background colors. The equation  being used to blend alphas was using the wrong parameter of GL_SRC_ALPHA which would then multiply by itself creating additional transparencies. Using GL_ONE instead prevents the error from occurring.
. Tweaked the Gamma values to obtain better contrast between active and disabled items.
. Made some of the descriptions on the settings short enough so that they fit with default window size making sure te description was complete.
. Fixed logic inconsistency when turning antialiasing on in font configuration.
. Added drop down option in font settings to select among three types of antialiasing.
. Added on and off callbacks for font shader control.
. Moved new classes and functions to their own file.  Under imhexlib in helpers with the name of freetype so that it can be expanded if needed in the future.
. Removed old and unused code.
. Instead of using the name to identify which fonts won't be processed by the subpixel renderer each font now contains a field that can be used to check if the font is scalable. Non-scalable fonts cannot be anti-aliased or sub-pixelated. When the font is added to the atlas the property is checked and if the font cannot be scaled then it assigns to it a name which is the same for all non-scalable fonts which then is checked during the sub-pixel processing. For example when fonts are registered the unifont is labeled as non-scalable and is therefore automatically excluded.

Additionally, the following changes are also included:
. Fixed problem with fonts becoming transparent and leaking the background colors. The equation  being used to blend alphas was using the wrong parameter of GL_SRC_ALPHA which would then multiply by itself creating additional transparencies. Using GL_ONE instead prevents the error from occurring.
. Tweaked the Gamma values to obtain better contrast between active and disabled items.
. Made some of the descriptions on the settings short enough so that they fit with default window size making sure te description was complete.
. Fixed logic inconsistency when turning antialiasing on in font configuration.
. Added drop down option in font settings to select among three types of antialiasing. The entries now grey-out correctly when they can't be used, but only on the dark theme.
. Added error messages indicating that fonts failed to load instead of failing silently.
. Glyphs that use subpixel rendering were not able to use their alpha channel as transparency because that role belongs to the subpixels and the alpha channel does not take part in the (dual source) blending. This shortcoming can be addressed by multiplying the texture colors (I call them colors for clarity but they are alpha coverages) by the alpha value of the color fragment inside the fragment shader. THis works well for most colors under color themes except for the light theme. The issue is that in the Light theme the color that stands out the most is [0,0,0](black) so multiplication by alpha leaves it unchanged. In this case instead of a multiplication the shader simply assigns the value of the alpha channel to the fragment color.
@paxcut paxcut marked this pull request as ready for review March 13, 2025 14:31
@paxcut paxcut requested a review from WerWolv March 13, 2025 14:32
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

Successfully merging this pull request may close these issues.

2 participants