ProgressBar with fraction == NaN leads to gigantic allocation #7451

New issue

Closed

opened 2024-03-28 14:55:00 +00:00 by alektron · 0 comments

alektron commented

2024-03-28 14:55:00 +00:00

(Migrated from github.com)

Version/Branch of Dear ImGui:

Version 1.90.5 (19046), Branch: docking (master/docking/etc.)

Back-ends:

Backend independent

Compiler, OS:

Visual Studio 2022 MSVC C++ 20

Full config/build information:

Dear ImGui 1.90.5 WIP (19046)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=199711
define: _WIN32
define: _WIN64
define: _MSC_VER=1938
define: _MSVC_LANG=202002
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
--------------------------------
io.BackendPlatformName: imgui_impl_win32
io.BackendRendererName: imgui_impl_opengl3
io.ConfigFlags: 0x00000443
 NavEnableKeyboard
 NavEnableGamepad
 DockingEnable
 ViewportsEnable
io.ConfigViewportsNoDecoration
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x00001C0E
 HasMouseCursors
 HasSetMousePos
 PlatformHasViewports
 HasMouseHoveredViewport
 RendererHasVtxOffset
 RendererHasViewports
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64
io.DisplaySize: 1264.00,761.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.FrameRounding: (See explanation)

Details:

Reproduction:

volatile float zero = 0.0f;
ImGui::ProgressBar(0 / zero);

Explanation:
Passing NaN to a ProgressBar widget with FrameRounding != 0 leads to a giant allocation in PathArcTo.
In my specific case this happens due to us calculating the fraction for the progress bar as Count / (float)MaxCount, which happens to be able to become a 0 / (float)0.
Now, shame on us for dividing through 0 but it seems to me like that would be a pretty common expression to use for a progress bar and therefore a mistake that is easily made.

Now ProgressBar calls ImSaturate internally on fraction which e.g. properly prevents issues when fraction equals infinity.
However NaN seems to stay NaN.
This NaN gets then passed down to RenderRectFilledRangeH.
IMPORTANT: If FrameRounding is set to 0, RenderRectFilledRangeH will exit out early and while the values calculated from NaN are still wrong, it is not critical for AddRectFilled. It may look wrong but doesn't crash.
With frame rounding enabled however, a variation of Path...To will get used to draw the rounded rectangle.
And PathArcTo for example will allocate. I did not look exactly into how the NaN influences the number of path points. I also had different results in our application as opposed to a fresh imgui clone example.
However from that point it is easy to see that things go wrong and lead to a giant allocation in _Path.reserve.

The fix is not exactly difficult (just a check in ProgressBar) but it raises the quesiton if maybe PathArcTo itself should be more robust against this kind of input. That may be up to discussion and probably better decided by someone else.

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

No response

### Version/Branch of Dear ImGui: Version 1.90.5 (19046), Branch: docking (master/docking/etc.) ### Back-ends: Backend independent ### Compiler, OS: Visual Studio 2022 MSVC C++ 20 ### Full config/build information: ``` Dear ImGui 1.90.5 WIP (19046) -------------------------------- sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20 define: __cplusplus=199711 define: _WIN32 define: _WIN64 define: _MSC_VER=1938 define: _MSVC_LANG=202002 define: IMGUI_HAS_VIEWPORT define: IMGUI_HAS_DOCK -------------------------------- io.BackendPlatformName: imgui_impl_win32 io.BackendRendererName: imgui_impl_opengl3 io.ConfigFlags: 0x00000443 NavEnableKeyboard NavEnableGamepad DockingEnable ViewportsEnable io.ConfigViewportsNoDecoration io.ConfigInputTextCursorBlink io.ConfigWindowsResizeFromEdges io.ConfigMemoryCompactTimer = 60.0 io.BackendFlags: 0x00001C0E HasMouseCursors HasSetMousePos PlatformHasViewports HasMouseHoveredViewport RendererHasVtxOffset RendererHasViewports -------------------------------- io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64 io.DisplaySize: 1264.00,761.00 io.DisplayFramebufferScale: 1.00,1.00 -------------------------------- style.FrameRounding: (See explanation) ``` ### Details: Reproduction: ```cpp volatile float zero = 0.0f; ImGui::ProgressBar(0 / zero); ``` Explanation: Passing NaN to a ProgressBar widget with FrameRounding != 0 leads to a giant allocation in PathArcTo. In my specific case this happens due to us calculating the fraction for the progress bar as `Count / (float)MaxCount`, which happens to be able to become a `0 / (float)0`. Now, shame on us for dividing through 0 but it seems to me like that would be a pretty common expression to use for a progress bar and therefore a mistake that is easily made. Now `ProgressBar` calls ImSaturate internally on `fraction` which e.g. properly prevents issues when fraction equals infinity. However NaN seems to stay NaN. This NaN gets then passed down to `RenderRectFilledRangeH`. IMPORTANT: If FrameRounding is set to 0, `RenderRectFilledRangeH` will exit out early and while the values calculated from NaN are still wrong, it is not critical for `AddRectFilled`. It may look wrong but doesn't crash. With frame rounding enabled however, a variation of Path...To will get used to draw the rounded rectangle. And PathArcTo for example will allocate. I did not look exactly into how the NaN influences the number of path points. I also had different results in our application as opposed to a fresh imgui clone example. However from that point it is easy to see that things go wrong and lead to a giant allocation in `_Path.reserve`. The fix is not exactly difficult (just a check in ProgressBar) but it raises the quesiton if maybe `PathArcTo` itself should be more robust against this kind of input. That may be up to discussion and probably better decided by someone else. ### Screenshots/Video: _No response_ ### Minimal, Complete and Verifiable Example code: _No response_