ocornut / imgui

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

Text wrapping: the current wrapping is not ideal for punctuations in some cases #8139

Open achabense opened 2 weeks ago

achabense commented 2 weeks ago

Version/Branch of Dear ImGui:

Version 1.91.2

Back-ends:

imgui_impl_sdl2.cpp + imgui_impl_sdlrenderer2.cpp

Compiler, OS:

Windows 10 + MSVC 2022

Full config/build information:

No response

Details:

As the screenshot shows, sometimes wrapped-text will split the punctuation to the beginning of next line, even if if can fit well into the first line.

(The problem will not show up if there is no " before the period .. )

Screenshots/Video:

image

(If it's s = "This is called qux.") image

Minimal, Complete and Verifiable Example code:

static void issue() {
    if (ImGui::Begin("Issue", 0, ImGuiWindowFlags_NoSavedSettings)) {
        std::string s = "This is called \"qux\".";
        ImGui::PushTextWrapPos(ImGui::CalcTextSize(s.c_str()).x + 8);
        ImGui::TextUnformatted(s.c_str());
        ImGui::Spacing();
        s += " That"; // "qux". That
        ImGui::TextUnformatted(s.c_str()); // Though the '.' can fit in the first line, it's now at the second line.
        ImGui::PopTextWrapPos();
    }
    ImGui::End();
}
ocornut commented 2 weeks ago

Thank you for the repro. I confirm this is an issue and the logic in CalcWordWrapPositionA() is behaving incorrectly here.

Slightly tweaked repro:

if (ImGui::Begin("Issue #8139", NULL, ImGuiWindowFlags_NoSavedSettings))
{
    const char* s1 = "abcde!.";
    const char* s2 = "abcde!. That";
    float local_off_x = ImGui::GetCursorPos().x;
    ImGui::PushTextWrapPos(ImGui::CalcTextSize(s1).x + local_off_x);
    ImGui::TextUnformatted(s1);
    ImGui::Spacing();
    ImGui::TextUnformatted(s2); // Though the '.' can fit in the first line, it's now at the second line.
    ImGui::PopTextWrapPos();
}
ImGui::End();
ocornut commented 2 weeks ago

I had a quick look and it didn't seem like a trivial fix. The logic in CalcWordWrapPositionA() is a little bit weird and may need to be redesigned. Awkwardly we also don't have much testing for word-wrapping so I suppose we should be adding some.

I added a (broken) test for this so we have it ready for when there is a fix: https://github.com/ocornut/imgui_test_engine/commit/c16b5cc4b5b8d35fd3297bd497c4c3f23c13d81c But I don't know when I would be able to look at this myself.