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

ImFont::DisplayOffset and Ascent/Descent issues #1619

Closed
ocornut opened this issue Feb 15, 2018 · 4 comments
Closed

ImFont::DisplayOffset and Ascent/Descent issues #1619

ocornut opened this issue Feb 15, 2018 · 4 comments

Comments

@ocornut
Copy link
Owner

ocornut commented Feb 15, 2018

Does anyone here uses the ImFont::DisplayOffset field?

It's been arbitrarily set to default to ImVec2(0, 1) since July 2015.
This has been done mostly because it looks righter with ProggyClean, and on some fonts with the way we rounded Ascent/Descent it made fonts more centered, but it was incorrect in other cases.

I have created a font_offset branch that does 2 things:
https://github.com/ocornut/imgui/compare/font_offset?expand=1

1/ set DisplayOffset.y to 0 by default in ImFont. AddFontDefault() changes it to 1 for ProggyClean.
2/ in the font builder, I am rounding Ascent and Descent values to they match what freetype does (it always rounds them).

DisplayOffset 0, without rounded Ascent/Descent:
You can see that the STB render looks too high, and this is where the previous DisplayOffset=1 value helped.
stb_ascent_1_before

DisplayOffset 0, with rounded Ascent/Descent:
stb_ascent_2_after

How this going to affect people:

-People who only used default font: won't be affected.

  • People who didn't touch DisplayOffset.y : hopefully fonts are better centered now. Any regression?
  • People who override DisplayOffset.y = 0 won't be affected.
  • People who did DisplayOffset.y -= 1 will now have their font one pixel too high. Those are people who were caring about their use of fonts, so they will notice and fix easily?

(I think one tangential issue we have with this is that FramePadding.y is symmetrical, we don't have a way to specify a different padding value for bottom and top. Ideally would be useful. Practically would probably break things and add confusion?)

@goncalo
Copy link

goncalo commented Feb 15, 2018

I have been using ImFontConfig.GlyphOffset.y = -1 in custom fonts which I guess was compensating DisplayOffset.y = 1 to 0 (never noticed DisplayOffset wasn't zero).

As other fonts can be merged into the default ImFont, wouldn't it be better to use GlyphOffset.y = 1 in ProggyClean instead of DisplayOffset to offset just ProggyClean glyphs?

@ocornut
Copy link
Owner Author

ocornut commented Feb 15, 2018

@goncalo: which font file and size/settings were you using? default stb rasterizer? So i can double check the effect of my ascent/descent rounding.

As other fonts can be merged into the default ImFont, wouldn't it be better to use GlyphOffset.y = 1 in ProggyClean instead of DisplayOffset to offset just ProggyClean glyphs?

That’s exactly what this patch is doing.

@DomGries
Copy link
Contributor

DomGries commented Feb 16, 2018

I also only used ImFontConfig.GlyphOffset.y to fix vertical font alignment when merging multiple fonts so this seems like a good change from my point of view.

EDIT: After trying the patch I can confirm that the text is now centered consistently vertically (even when scaling font size and ImFontConfig.GlyphOffset.y with screen size).

@ocornut ocornut modified the milestones: v1.49, v1.60 Feb 25, 2018
ocornut added a commit that referenced this issue Mar 8, 2018
@ocornut
Copy link
Owner Author

ocornut commented Mar 8, 2018

This is now merged. If you were adding or subtracting to ImFont::DisplayOffset.y you may be affected. Hopefully it'll be easier to spot if that's the case.

@ocornut ocornut closed this as completed Mar 8, 2018
ocornut added a commit that referenced this issue Mar 9, 2018
ocornut added a commit that referenced this issue Sep 17, 2020
…1619)

+ Fonts: AddFontDefault() adjust its vertical offset based on floor(size/13) instead of always +1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants