Non-integer sized fonts (Fringe issue: ScrollToBringRectIntoView() does not work properly when ScaleAllSizes() was called with non-multiple of 0.5) #3164

Open
opened 2020-04-25 18:05:14 +00:00 by JPGygax68 · 8 comments
JPGygax68 commented 2020-04-25 18:05:14 +00:00 (Migrated from github.com)

Version and branch:

Version: 1.77 WIP (17601)
Branch: Tables

Compiler, platform, backends:*
Visual C++ 2019, Windows 10, SDL2 + OpenGL 3

My Issue/Question:

This will probably never be a problem in itself, but it might still be useful to find out what causes it.

When you call ScaleAllSizes() with a number that is not a multiple of 0.5, ScrollToBringRectIntoView() scrolls by an incorrect (insufficient) amount.

Values 1, 2, and 1.5 are all ok, but 1.75 is not.

This may only be true when working with tables, I didn't check that.

I tried to debug, but was unable to pinpoint the cause. It would have to be a combination of multiplication by 2 and using floor(), but I couldn't put my finger on it.

Screenshots/Video

In this screenshot, the button has been clicked and row 350 should be fully in view, but the scrolling only made it to ~ 343:

grafik

Example


// In the setup code

float dpi_scaling = 1.75;
int font_size = 14;
ImGui::GetStyle().ScaleAllSizes(dpi_scaling);
ImGui::GetIO().Fonts->AddFontFromFileTTF("C:\\windows\\fonts\Arial.ttf", dpi_scaling * font_size, NULL, nullptr);


// Inside a window:

void ScrollRectIntoViewIncorrect()
{
    static bool list_created = false;
    static std::vector<std::string> list;

    if (!list_created) {
        for (auto i = 0U; i < 400; i ++) list.push_back(std::to_string(i));
        list_created = true;
    }

    bool scroll = false;
    if (ImGui::Button("Scroll to row 350")) {
        scroll = true;
    }

    auto flags = ImGuiTableFlags_ScrollFreezeTopRow |
        ImGuiTableFlags_ScrollY;

    if (ImGui::BeginTable("###List", 1, flags))
    {
        ImGui::TableSetupColumn("Number");
        ImGui::TableAutoHeaders();

        for (auto row_n = 0; row_n < list.size(); row_n++) {
            ImGui::TableNextRow();
            ImGui::TextUnformatted(list[row_n].c_str());
            ImGui::TableNextCell();
            if (scroll && row_n == 350) {
                auto window = ImGui::GetCurrentWindow();
                ImGui::ScrollToBringRectIntoView(window, window->DC.LastItemRect);
                scroll = false;
            }
        }

        ImGui::EndTable();
    }
}
**Version and branch:** Version: 1.77 WIP (17601) Branch: Tables **Compiler, platform, backends:*** Visual C++ 2019, Windows 10, SDL2 + OpenGL 3 **My Issue/Question:** This will probably never be a problem in itself, but it might still be useful to find out what causes it. When you call `ScaleAllSizes()` with a number that is not a multiple of 0.5, `ScrollToBringRectIntoView()` scrolls by an incorrect (insufficient) amount. Values 1, 2, and 1.5 are all ok, but 1.75 is not. This may only be true when working with tables, I didn't check that. I tried to debug, but was unable to pinpoint the cause. It would have to be a combination of multiplication by 2 and using floor(), but I couldn't put my finger on it. **Screenshots/Video** In this screenshot, the button has been clicked and row 350 should be fully in view, but the scrolling only made it to ~ 343: ![grafik](https://user-images.githubusercontent.com/795272/80287843-00d17e80-8734-11ea-8a6d-976c628c9375.png) **Example** ```C++ // In the setup code float dpi_scaling = 1.75; int font_size = 14; ImGui::GetStyle().ScaleAllSizes(dpi_scaling); ImGui::GetIO().Fonts->AddFontFromFileTTF("C:\\windows\\fonts\Arial.ttf", dpi_scaling * font_size, NULL, nullptr); // Inside a window: void ScrollRectIntoViewIncorrect() { static bool list_created = false; static std::vector<std::string> list; if (!list_created) { for (auto i = 0U; i < 400; i ++) list.push_back(std::to_string(i)); list_created = true; } bool scroll = false; if (ImGui::Button("Scroll to row 350")) { scroll = true; } auto flags = ImGuiTableFlags_ScrollFreezeTopRow | ImGuiTableFlags_ScrollY; if (ImGui::BeginTable("###List", 1, flags)) { ImGui::TableSetupColumn("Number"); ImGui::TableAutoHeaders(); for (auto row_n = 0; row_n < list.size(); row_n++) { ImGui::TableNextRow(); ImGui::TextUnformatted(list[row_n].c_str()); ImGui::TableNextCell(); if (scroll && row_n == 350) { auto window = ImGui::GetCurrentWindow(); ImGui::ScrollToBringRectIntoView(window, window->DC.LastItemRect); scroll = false; } } ImGui::EndTable(); } } ```
ocornut commented 2020-04-26 21:09:42 +00:00 (Migrated from github.com)

Hello,

I have copy and pasted your example and cannot repro the bug so I assume there's something else causing it. What's your font size? Did you actually repro with the code your copied? No list clipper?

ScaleAllSizes() rounds all values so I don't think it would have a side effect?

However if you were to scale your font by that same factor and have them not be integer-sized you could run into problems presently.

Hello, I have copy and pasted your example and cannot repro the bug so I assume there's something else causing it. What's your font size? Did you actually repro with the code your copied? No list clipper? ScaleAllSizes() rounds all values so I don't think it would have a side effect? However if you were to scale your font by that same factor and have them not be integer-sized you could run into problems presently.
JPGygax68 commented 2020-04-27 06:36:17 +00:00 (Migrated from github.com)

Hi Omar, yes that is the code I used. But you're right about the font: I'm scaling that with the same factor. If the font size is used in any of the calculations and is being rounded, that would account for the effect nicely.
In my case, the font is Arial, size 1.75 * 14 = 24.5

EDIT: confirming that rounding the font size removes the problem.

Hi Omar, yes that is the code I used. But you're right about the font: I'm scaling that with the same factor. If the font size is used in any of the calculations and is being rounded, that would account for the effect nicely. In my case, the font is Arial, size 1.75 * 14 = 24.5 EDIT: confirming that rounding the font size removes the problem.
ocornut commented 2020-04-27 09:28:09 +00:00 (Migrated from github.com)

EDIT: confirming that rounding the font size removes the problem.

I'll add extra comments and perhaps rounding on the side of the font system.

> EDIT: confirming that rounding the font size removes the problem. I'll add extra comments and perhaps rounding on the side of the font system.
JPGygax68 commented 2020-04-27 09:29:18 +00:00 (Migrated from github.com)

It occurred to me that my use case is perhaps not as unusual as I thought.

I'm calculating the DPI scaling factor based on the size of the display, with the following formula:

dpi_scaling = (float)screen_width / 1920;

The idea is to have the font size be independent of the screen's resolution, using a factor of 1 for Full HD and 2 for 4K. But there are plenty of other resolutions out there, so non-standard factors can happen very easily.

P.S. I should probably use the vertical resolution instead of the horizontal one, to account for ultra-wide "flight-sim" type displays.

It occurred to me that my use case is perhaps not as unusual as I thought. I'm calculating the DPI scaling factor based on the size of the display, with the following formula: ```c++ dpi_scaling = (float)screen_width / 1920; ``` The idea is to have the font size be independent of the screen's resolution, using a factor of 1 for Full HD and 2 for 4K. But there are plenty of other resolutions out there, so non-standard factors can happen very easily. P.S. I should probably use the vertical resolution instead of the horizontal one, to account for ultra-wide "flight-sim" type displays.
JPGygax68 commented 2020-04-27 09:42:13 +00:00 (Migrated from github.com)

I have updated my code sample to include font size calculation and loading.

I have updated my code sample to include font size calculation and loading.
rokups commented 2020-04-27 10:20:23 +00:00 (Migrated from github.com)

The idea is to have the font size be independent of the screen's resolution, using a factor of 1 for Full HD and 2 for 4K. But there are plenty of other resolutions out there, so non-standard factors can happen very easily.

Rounding font size is still fine, unless your base font size is 1. Your formula yields 1.(3) scaling factor on 1440p screens, which upscales 16px font to 21.(3) which we round to 21px. With rounding you lose or gain at most 0.5px of font size which is hardly perceivable anyway.

> The idea is to have the font size be independent of the screen's resolution, using a factor of 1 for Full HD and 2 for 4K. But there are plenty of other resolutions out there, so non-standard factors can happen very easily. Rounding font size is still fine, unless your base font size is 1. Your formula yields 1.(3) scaling factor on 1440p screens, which upscales 16px font to `21.(3)` which we round to 21px. With rounding you lose or gain at most 0.5px of font size which is hardly perceivable anyway.
JPGygax68 commented 2020-04-27 10:31:09 +00:00 (Migrated from github.com)

@rokups True, and I have no issue with having to round the font size. I'm not quite sure it is required by FreeType though, since it would have to do grid-fitting and anti-aliasing anyway.
But if Dear ImGui does require the font size to be rounded, then I'd be grateful if it does so automatically, or provides some kind of check.

@rokups True, and I have no issue with having to round the font size. I'm not quite sure it is required by FreeType though, since it would have to do grid-fitting and anti-aliasing anyway. But if Dear ImGui does require the font size to be rounded, then I'd be grateful if it does so automatically, or provides some kind of check.
ocornut commented 2023-09-11 08:06:09 +00:00 (Migrated from github.com)

I added enforcing of font size rounding in bd63a9f

Note that using io.FontGlobalScale or SetWindowFontScale(), with are legacy-ish/partially supported features but sitll somehow convenient in some situation, can still lead to unrounded sizes and same issues.

I added enforcing of font size rounding in bd63a9f Note that using io.FontGlobalScale or SetWindowFontScale(), with are legacy-ish/partially supported features but sitll somehow convenient in some situation, can still lead to unrounded sizes and same issues.
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ocornut/imgui#3164
No description provided.