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

Floating point exception from slider in very small child window #5249

Closed
dbr opened this issue Apr 27, 2022 · 3 comments
Closed

Floating point exception from slider in very small child window #5249

dbr opened this issue Apr 27, 2022 · 3 comments

Comments

@dbr
Copy link

dbr commented Apr 27, 2022

Version/Branch of Dear ImGui:

Version: 1.86 via imgui-rs
Branch: docking

My Issue/Question:

We are using SetWindowFontScale to have a zoomable imnodes-style node graph. This all works well, except sometimes we get a floating point exception as follows:

#0  0x000055555d60405e in ImDrawList::_PathArcToFastEx (this=0x55555ebec2d8, center=..., radius=2.38418579e-07, a_min_sample=24, a_max_sample=36, a_step=0) at ./third-party/imgui-docking/imgui/imgui_draw.cpp:1076
#1  0x000055555d6044f3 in ImDrawList::PathArcToFast (this=0x55555ebec2d8, center=..., radius=2.38418579e-07, a_min_of_12=6, a_max_of_12=9) at ./third-party/imgui-docking/imgui/imgui_draw.cpp:1184
#2  0x000055555d6056a2 in ImDrawList::PathRect (this=0x55555ebec2d8, a=..., b=..., rounding=2.38418579e-07, flags=240) at ./third-party/imgui-docking/imgui/imgui_draw.cpp:1383
#3  0x000055555d605aa0 in ImDrawList::AddRectFilled (this=0x55555ebec2d8, p_min=..., p_max=..., col=4290559164, rounding=12, flags=0) at ./third-party/imgui-docking/imgui/imgui_draw.cpp:1423
#4  0x000055555d61316c in ImGui::ScrollbarEx (bb_frame=..., id=2833933471, axis=ImGuiAxis_Y, p_scroll_v=0x7ffffffe5308, size_avail_v=103, size_contents_v=119, flags=288) at ./third-party/imgui-docking/imgui/imgui_widgets.cpp:1002
#5  0x000055555d6128a8 in ImGui::Scrollbar (axis=ImGuiAxis_Y) at ./third-party/imgui-docking/imgui/imgui_widgets.cpp:906
#6  0x000055555d5a8411 in ImGui::RenderWindowDecorations (window=0x55555ec77d00, title_bar_rect=..., title_bar_is_highlight=true, handle_borders_and_resize_grips=true, resize_grip_count=2, resize_grip_col=0x7ffffffe55e0, resize_grip_draw_size=5)
    at ./third-party/imgui-docking/imgui/imgui.cpp:6119
#7  0x000055555d5abef5 in ImGui::Begin (name=0x55555e3e8b10 "View/C6DC8F45/8C64E6D8", p_open=0x0, flags=19663623) at ./third-party/imgui-docking/imgui/imgui.cpp:6876
#8  0x000055555d5a4bc4 in ImGui::BeginChildEx (name=0x0, id=2355422936, size_arg=..., border=false, flags=19663619) at ./third-party/imgui-docking/imgui/imgui.cpp:5410
#9  0x000055555d5a4d5d in ImGui::BeginChild (id=2355422936, size_arg=..., border=false, extra_flags=788992) at ./third-party/imgui-docking/imgui/imgui.cpp:5442
#10 0x000055555d640522 in igBeginChildID (id=2355422936, size=..., border=false, flags=788992) at ./third-party/imgui-docking/cimgui.cpp:141

The top of the backtrace refers to this code:
https://github.com/ocornut/imgui/blob/v1.86/imgui_draw.cpp#L1066-L1068
(well specifically these ones in the docking branch code)

Problem is most probably caused by radius being a tiny number (2.38418579e-07) leading to some zero-division

It's hard to narrow down the exact cause, as the crash only happens sometimes when randomly panning around in a large node graph - but I'm hoping the backtrace is enough to narrow down what causes this exception?

@ocornut
Copy link
Owner

ocornut commented Apr 27, 2022

We don't well support SetWindowFontScale() admittedly, it is kind-of an old artifact but I understand it has use.
In this case, it leads to non-integer sizes which we also don't correctly support (specific bug reports are welcome).

While a little of a workaround, I think in ScrollbarEx() it would be reasonable to change the if (bb_frame_width <= 0.0f || bb_frame_height <= 0.0f) line to if (bb_frame_width < 1.0f || bb_frame_height < 1.0f). Can you confirm if that fixes it for you?

On another side, I think we could probably to fix/clarify cases of using near-zero rounding values. That radius value will probably fail our const int radius_idx = (int)(radius + 0.999999f) kludge in _CalcCircleAutoSegmentCount(). I wonder if fallback CircleSegmentCounts[0] should be ~0 instead of 0 ?

@dbr
Copy link
Author

dbr commented Apr 27, 2022

While a little of a workaround, I think in ScrollbarEx() it would be reasonable to change the if (bb_frame_width <= 0.0f || bb_frame_height <= 0.0f) line to if (bb_frame_width < 1.0f || bb_frame_height < 1.0f). Can you confirm if that fixes it for you?

Tried making that change it didn't seem to help; in ScrollbarEx the bb_frame_width is ~6 and height is 10.0 - I guess the problem window isn't tiny, isntead it is just small enough that with the padding it ends up too small

#4  0x000055555d5ad11c in ImGui::ScrollbarEx (bb_frame=..., id=3031077347, axis=ImGuiAxis_Y, p_scroll_v=0x7ffffffe5308, size_avail_v=10, size_contents_v=21, flags=288) at ./third-party/imgui-docking/imgui/imgui_widgets.cpp:1002
(gdb) print bb_frame_width
$6 = 6.00000048
(gdb) print bb_frame_height 
$7 = 10
(gdb) print grab_rect.Min
$8 = {x = 5.99999952, y = 164}
(gdb) print grab_rect.Max
$9 = {x = 8, y = 169}
(gdb) print style.ScrollbarRounding
$10 = 12

Poking around further, it seems like in ImDrawList::PathRect the scrollbar rounding is the style-configured 12.0, and it gets scaled to the very-small float by this code:

rounding = ImMin(rounding, ImFabs(b.x - a.x) * ( ((flags & ImDrawFlags_RoundCornersTop)  == ImDrawFlags_RoundCornersTop)  || ((flags & ImDrawFlags_RoundCornersBottom) == ImDrawFlags_RoundCornersBottom) ? 0.5f : 1.0f ) - 1.0f)
rounding = ImMin(rounding, ImFabs(b.y - a.y) * ( ((flags & ImDrawFlags_RoundCornersLeft) == ImDrawFlags_RoundCornersLeft) || ((flags & ImDrawFlags_RoundCornersRight)  == ImDrawFlags_RoundCornersRight)  ? 0.5f : 1.0f ) - 1.0f)

The tiny number coming from - 1.0 part:

(gdb) print b.x
$36 = 8
(gdb) print a.x
$37 = 5.99999952
(gdb) print (b.x - a.x) * 0.5 - 1
$38 = 2.384185791015625e-07

If I make the following ch ange it does stop crashing, although the change feels.. pretty arbitrary:

 void ImDrawList::_PathArcToFastEx(const ImVec2& center, float radius, int a_min_sample, int a_max_sample, int a_step)
 {
-    if (radius <= 0.0f)
+    if (radius <= 0.5f)

@ocornut ocornut mentioned this issue May 9, 2022
ocornut added a commit that referenced this issue May 13, 2022
@ocornut
Copy link
Owner

ocornut commented May 13, 2022

We investigated this with @thedmd and concluded that < 0.5f was fine.
Pushed fixes 9e0517a
(We considering using < _FringeScale or < _FringeScale * 0.5f but will want later to eventually introduce a separate variable for that purpose, in order to facilitate scaling.)

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

2 participants