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

Unrounded font size / Avoid floating point rounding error #3309

Closed

Conversation

johannes-athlos
Copy link

@johannes-athlos johannes-athlos commented Jun 18, 2020

Platform: Win32, DX11
Branch: docking

I had a problem that popup modals were sometimes drifting upwards on their own, exactly 1 pixel per frame. This also happened in the "Delete?" sample modal in the demo window.

I added a breakpoint to window->Pos and saw that inside ClampWindowRect, Pos.y went from 472.0 -> 471.999969, and later in the code this got floored to 471.0.

Avoiding unnecessary (a+b)-b calculation fixes the problem for me.

If size_for_clamping is not an integer, the result of

    (window->Pos + size_for_clamping) - size_for_clamping

can be (window->Pos - eps).  Later Pos is floored to an integer, and
this can cause an unintended -1.

This bug made popup modals drift upwards on their own in some
situations, including in the ImGui demo window.
@rokups
Copy link
Contributor

rokups commented Jun 19, 2020 via email

ocornut added a commit that referenced this pull request Jun 19, 2020
As a side-effect, some rounding error may be neutralized however this isn't the intent. (#3309)
@ocornut
Copy link
Owner

ocornut commented Jun 19, 2020

Hello @johannes-athlos,
I have pushed a simplification of this function which does include your change.
However, neutralizing a floating point error is not the aim of this commit (I pushed it to simplify the code mostly). If this happens here this will happens in various other situations.

Could you figure out a repro for the issue you describe (before merging my/your commit) or try to clarify what causes it? As mentioned, font size need to be rounded.

@johannes-athlos
Copy link
Author

Your commit also fixes the issue.

I'm using the default font and styling, except that I had set io.FontGlobalScale = 1.2f. When the problem happened, window->Size.y was 209.600006. Should I increase font size manually to some integer instead?

@ocornut ocornut changed the title Avoid floating point rounding error Unrounded font size / Avoid floating point rounding error Jun 19, 2020
@ocornut
Copy link
Owner

ocornut commented Jun 19, 2020

Right.. I don't have a good answer to that problem presently.
Rounding the return value of CalcFontSize() would make some sense but leads to other issues.
I guess it sorts of means that io.FontGlobalScale is not well supported or a little bit ill-defined (it always was, it's a remnant from v1.0). Would suggest building fonts at the desired size instead, until we clarify/solve this in a better way.

@johannes-athlos
Copy link
Author

Thanks for the answer. I'll leave FontGlobalScale to 1.0 for now. Closing this PR.

ocornut added a commit that referenced this pull request Sep 11, 2023
…unded to lowest integer. (#3164, #3309, #6800)

Note that using io.FontGlobalScale or SetWindowFontScale(), with are legacy-ish, partially supported features, can still lead to unrounded sizes and same issues.
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.

None yet

3 participants