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

ImGui::RenderTextEllipsis needs latin glyph #4269

Closed
ghost opened this issue Jun 25, 2021 · 1 comment
Closed

ImGui::RenderTextEllipsis needs latin glyph #4269

ghost opened this issue Jun 25, 2021 · 1 comment

Comments

@ghost
Copy link

ghost commented Jun 25, 2021

Version/Branch of Dear ImGui:

Version: 1.83WIP
Branch: Unknown (Dear ImGui version provided by https://github.com/eliasdaler/imgui-sfml)

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_dx10.cpp + imgui_impl_win32.cpp
Compiler: MSVC 192930038
Operating System: Windows 10 Family VERSION(20H2) BUILD(19042.1052) EXPERIENCE(120.2212.2020.0)

My Issue:

Currently working with a chinese font, I don't need to push any latin glyph ranges when adding the font to the atlas.
But a latin character is used in the Dear ImGui code when calculating characters width for tables headers.

The Code

In ImGui::RenderTextEllipsis, EllipsisChar returns 65535

imgui/imgui.cpp

Line 2785 in 30224fd

ImWchar ellipsis_char = font->EllipsisChar;

which triggers the next if state to replace the character used

imgui/imgui.cpp

Line 2787 in 30224fd

if (ellipsis_char == (ImWchar)-1)

by the dot character

imgui/imgui.cpp

Line 2789 in 30224fd

ellipsis_char = (ImWchar)'.';

The Callstack

TLN.exe!ImGui::RenderTextEllipsis(ImDrawList * draw_list, const ImVec2 & pos_min, const ImVec2 & pos_max, float clip_max_x, float ellipsis_max_x, const char * text, const char * text_end_full, const ImVec2 * text_size_if_known) Ligne 2784 C++
TLN.exe!ImGui::TableHeader(const char * label) Ligne 2946 C++
TLN.exe!ImGui::TableHeadersRow() Ligne 2804 C++
TLN.exe!CManager_Window::Wnd_Tasks() Ligne 533 C++
TLN.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Ligne 53 C++ [external code]

Conclusion

Without the dot character (0x002E) pushed into the glyph ranges used by the font, it crashes.
Use the unknown character (0xFFFD) instead of the dot character (0x002E) seems to be a good alternative :
L2789 ellipsis_char = (ImWchar)0xFFFD;

@ocornut ocornut changed the title ImGui::RenderTextEllipsis char width calculation needs latin glyph ImGui::RenderTextEllipsis needs latin glyph Jun 25, 2021
ocornut added a commit that referenced this issue Jun 25, 2021
… U+FF0E dot character to construct an ellipsis if U+002E '.' is not available. (#4269)
ocornut added a commit that referenced this issue Jun 25, 2021
@ocornut
Copy link
Owner

ocornut commented Jun 25, 2021

I confirmed that this can crash if:

  • the font doesn't have an ellipsis char merged (U+2026 or U+0085)
  • AND the font doesn't have a replacement . character (U+002E)
  • AND the font doesn't have a fallback char merged (default to ? U+003F)

There are two issues here:

  • the ellipsis should find other versions of the . character. This will be desirable for Asian-only font. Used U+FF0E for now.
  • we need a valid fallback char (regardless of ellipsis rendering). I think the code crash because of a lack of fallback. Even without an ellipsis if you rendering a missing character it would crash.

Both issues should be fixed now.
Pushed two changes: 38165f4 + 9440974

image

@ocornut ocornut closed this as completed Jun 25, 2021
ocornut added a commit that referenced this issue Jan 11, 2023
…more data. (#2775, #4269)

Ideally we're bake a single glyph but it's currently difficult to setup in our font building process.
vanitas55 added a commit to vanitas55/imgui that referenced this issue Jan 16, 2023
kjblanchard pushed a commit to kjblanchard/imgui that referenced this issue May 5, 2023
…more data. (ocornut#2775, ocornut#4269)

Ideally we're bake a single glyph but it's currently difficult to setup in our font building process.
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

1 participant