search

ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
61.17k stars 10.31k forks source link

Merged Fonts vertical alignment #4127

Open rickyviking opened 3 years ago

rickyviking commented 3 years ago

This issue is related to https://github.com/ocornut/imgui/issues/2857

When merging an icon font (Google Material Icons) to the Roboto fonts, both loaded at the same size (20px), I'm seeing a vertical misalignment (4px), as shown by the following images:

  1. merged fonts without using vertical offset => misalignment
  2. merged fonts adding a vertical offset => OK, fonts aligned
  3. loading the Google Material fonts separately without merging, the fonts are aligned correctly.

The misalignment is introduced only when merging the 2 fonts, probably due to the different value of Descent (-1 and -5) computed for the 2 fonts independently - not sure how this value is computed.

Using the vertical offset as in point 2) above solves the issue, not sure if this is an issue or an expected behavior. Dear ImGui Version 1.81

  1. [merged font, no offset] imgui_icons_misaligned

  2. [merged font, 4px vertical offset] imgui_icons_offset

  3. [fonts loaded independently, no merge] imgui_icons_no_merge

ocornut commented 3 years ago

I think I made a mistake in 9da53bcecdc2273fef86f032ad55c36393c38ebd should have used current font's ascent value instead of dst_font->Ascent. I believe changing it to ascent would be a move in the good direction and may break lots of hardcoded GlyphOffset value.

ocornut commented 3 years ago

It's actually trickier than that. Current code ensure that all glyphs are sharing the same baseline, which is desirable e.g. when merging two text fonts (for western + asian language). I would argue it is probably similarly desirable when merging a text font + an icon font. image (This is a weird example but in reality it would make sense when combining asian + non-asian glyphs)

Using destination font ascent (current code) = same baseline

Using source font ascent:

The code before 9da53bcecdc2273fef86f032ad55c36393c38ebd attempted perform centering (perhaps incorrectly) and removed it in favor of explicit GlyphOffset values.

I want to say current behavior is actually better BUT creates a change depending on whether the icon font is merged or not, which is not ideal. Genuine question: your post state "I'm seeing a vertical misalignment" but what constitute a "misalignment" is entirely depending on your expectation. How do you use that icon font in the first place? How/why did you even notice the misalignment?

As we are aiming to facilitate use of mixed fonts at any size (as we expect to make resizing automatic), we may want to find a solution for this where user indication (if any) can be suitably scaled, but I don't yet know how to design that solution.

rickyviking commented 3 years ago

Genuine answer :) I wanted to add some icons besides text, for instance to show a informative sign in a tooltip: icon_baseline_02 or to indicate that a tree node is a folder in an asset browser: icon_baseline

Following the example to merge an icon font with the main text font, I expected that their baseline would be aligned, being the 2 fonts loaded at the same resolution (20px in my example). This is what the example shows as a result.
Say that I'd expect fonts (text or icon) merged with the same size would be aligned (as it happens to be when they're not merged). If that's not the case, I'd just add this info in the Fonts document.

ocornut commented 3 years ago

The point is that their baseline ARE aligned in your two screenshots.

rickyviking commented 3 years ago

eheh right, because I am manually adding a 4px vertical alignment - case 2) in my original post.

My code to obtain the screenshots above is:

   // load custom font
   std::string fontFile = "path/to/RobotoCondensed-Regular.ttf";
   ImFont* assetTreeFont = ImGui::GetIO().Fonts->AddFontFromFileTTF(fontFile.c_str(), 20);

   // merge google icons font
   ImFontConfig imFontConfig;
   imFontConfig.MergeMode = true;
   imFontConfig.GlyphOffset = { 0.f, 4.f };
   imFontConfig.GlyphMinAdvanceX = 20.0f; // Use if you want to make the icon monospaced

   ImVector<ImWchar> icon_ranges;
   ImFontGlyphRangesBuilder builder;

   builder.AddChar(0xe2c7); // ICON_MD_FOLDER
   // [...]   
   builder.BuildRanges(&icon_ranges);    // Build the final result (ordered ranges with all the unique characters submitted)

   fontFile = "path/to/MaterialIconsOutlined-Regular.otf";
   ImGui::GetIO().Fonts->AddFontFromFileTTF(fontFile.c_str(), 20.f, &imFontConfig, icon_ranges.Data);
ocornut commented 3 years ago

eheh right, because I am manually adding a 4px vertical alignment - case 2) in my original post.

Oops, right!

ocornut commented 3 years ago

I think replacing

const float font_off_x = cfg.GlyphOffset.x;
const float font_off_y = cfg.GlyphOffset.y + IM_ROUND(dst_font->Ascent);

with

const float merged_off_y = cfg.MergeMode ? (descent - dst_font->Descent) * (cfg.SizePixels / dst_font->FontSize) : 0.0f;
const float font_off_x = cfg.GlyphOffset.x;
const float font_off_y = cfg.GlyphOffset.y + IM_ROUND(dst_font->Ascent + merged_off_y);

Might be good. EDIT No it's not, doesn't work as well when merging two text fonts in other tests.

image

ocornut commented 3 years ago

The more I dig the more think there is no magic solution other than exposing a way to specify vertical alignment (for merged fonts) on a per font basis. This would be loosely equivalent to specifying the +4 here but in a way that would more naturally fit for scaling.

The equation above is merely a hack which happens to work for this font, but would fail at others. A good test case is to merge two copies for the same font but with different glyph sizes.

For now I would suggest keeping that GlyphOffset but this is a topic we will need to explore further in the future.

rickyviking commented 3 years ago

Didn't expect to create such a turmoil. I agree that leaving things as they are works just fine, I'd only suggest to mention the fact in the Fonts doc, as at the beginning I thought I was doing something wrong.