Triangle is only half as thick as other shapes with the same thickness value

[geekynils]

Version/Branch of Dear ImGui:

Dear ImGui 1.83 WIP (18204)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201402
define: __APPLE__
define: __GNUC__=4
define: __clang_version__=12.0.0 (clang-1200.0.32.29)
--------------------------------
io.BackendPlatformName: imgui_impl_osx
io.BackendRendererName: imgui_impl_metal
io.ConfigFlags: 0x00000000
io.ConfigMacOSXBehaviors
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000A
 HasMouseCursors
 RendererHasVtxOffset
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64
io.DisplaySize: 1200.00,720.00
io.DisplayFramebufferScale: 2.00,2.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_osx and imgui_impl_metal
Compiler: Apple clang version 12.0.0 (clang-1200.0.32.29)
Operating System: macOS Big Sur 11.2.3 (20D91

My Issue/Question:

Thanks for creating IMGUI, I am having fun with it! However I think I discovered a small bug:

See screenshot below, the thickness of triangles is only around half of other shapes. Interestingly this also happens when I draw the triangle with AddPolyline(...) (a polyline with 3 points in clockwise order and closed set to true) and it's not a problem with the example screenshot on the main github page. So it might be related to Metal rendering on the Mac.

Screenshots/Video

Standalone, minimal, complete and verifiable example: (see #2261)

Just run the examples on a Mac with the metal backend.

[ocornut]

Hello,

It's not specific to Metal, I can see the same in DirectX11 and OpenGL and that tesselation is entirely done in imgui_draw.cpp anyway and unlikely to be related to rendering backend.

We have known issues with sharp angles, e.g. #3366 #2964 #2183 and I'd like to merge #2964 (maybe I can dig into that now...) so I would treat this issue as duplicate, but.... I admit I got very surprised to learn that 1.67wip (screenshot you linked to) got this triangle right....

I dug and I realized that 9ad3419 which was intended as a simple "dumb optimization" actually broke this. Investigating this now...

[ocornut]

Had a breakthrough "doh!" moment, thanks to the 1.67 WIP screenshot you linked to.....

• My January 2019 commit 9ad3419 was broken, the macro IM_NORMALIZE2F_OVER_EPSILON_CLAMP introduced a sqrtf() call which should not have been there.
• In April 2019 @rmitton spotted the issue and figured out a fix Improved algorithm for mitre joints on thick lines #2518. His fix removed the misusage of sqrt() but also used different thresholds/logic which effectively wasn't on par with code before the breakage.
• Variants of this problems were discussed plenty of times, and somehow we got blindsided by the fact that the correct solution (Render thick lines with correct thickness and beveled corners #2964) may require to rework tessellation, without realizing that our "incorrect" solution had a much better variant before I broke it and it got half-restored. My additional mistake is I should have ran a git blame on the macros changed by Improved algorithm for mitre joints on thick lines #2518 but at the time no one realized the code got broken accidentally three months earlier.

Restoring that code to use same logic as before the breakage:

Original code had a scaling limit which ihmo is best increased. By increasing the limit, thickness is better preserved for sharp angles on thick polylines, with the side-effect that the geometry ends up reaching farer than one would expect by e.g. the polygon bounding box + half thickness. Depending on what the polyline is supposed to intend it might be a problem or not at all.

As an experiment I changed this limit to 500.0f (pictured above) and I think it is a better tradeoff this way so I'm enabling this. A bevel (#2964) would be required to guarantee not extruding out, and I suspect we'll eventually introduce two runtime thresholds: threshold for bevel, threshold for limiting normal extrusion when not beveling.

The following issues #4053, #3366, #2964, #2868, #2518, #2183 were related and I'm going to add commentary on them now.

Issue #2183

This was the first submitted issue. Further confusion coming from the fact it was posted in Nov 2018 before I made the Jan 2019 breakage, then most of the discussions happened AFTER that. The original issue related to that threshold which today I've increased (see second gif above). While not technically fixed, I believe today's revert of old code + increase of the threshold is a fairly good mitigation of the issue. Full conclusion would come from either providing option of beveling (#2964) either providing the option of having that normal extruding the geometry as far as it needs (which is only a matter of increasing that 500.0f scaling limit).

Issue #2518

This was the fix by @rmitton attempting to fix my error.
If you compare the code BEFORE my breakage and the code AFTER the fix, it went from:

// before breakage in jan 2019
{ float d2 = VX*VX + VY*VY; if (d2 > 0.000001f)  { float inv_len = 1.0f / d2); if (inv_len > 100.0f) inv_len = 100.0f; VX *= inv_len; VY *= inv_len; } }

to

// after "fix" in apr 2019
{ float d2 = VX*VX + VY*VY; if (d2 < 0.5f) d2 = 0.5f; float inv_lensq = 1.0f / d2; VX *= inv_lensq; VY *= inv_lensq; }

The change of threshold from 0.000001f to 0.5f had the most side-effect and would end up breaking thickness in many situations.

Pushed fix fdda8b8 now!

[ocornut]

While reviewing the different issues I ended up reverting the scaling limit extent back. While both have merit, based on #3366 for now I think it is more important to preserve the property of not straying far off intended bounds. So effectively now we are closer to a full revert of logic from pre-Jan 2019, and I've identified that we might want one or two configurable thresholds later.

(
Testbed for future reference:

//ImGui::Text(ImGui::GetIO().KeyShift ? "After fix" : "Before fix");
ImDrawList* draw_list = ImGui::GetWindowDrawList();

{
    // Demo
    ImGui::Text("#2868");
    const ImVec2 p = ImGui::GetCursorScreenPos();
    float x = p.x, y = p.y;
    float sz = 40.0f;
    float th = 5.0f;
    draw_list->AddRect(ImVec2(x, y), ImVec2(x + sz, y + sz), IM_COL32(255, 255, 0, 255), 0.0f, 0, th); x += sz + 10.0f;  // Triangle
    draw_list->AddTriangle(ImVec2(x + sz * 0.5f, y), ImVec2(x + sz, y + sz - 0.5f), ImVec2(x, y + sz - 0.5f), IM_COL32(255,255,0,255), th); x += sz + 10.0f;  // Triangle
    draw_list->AddTriangle(ImVec2(x + sz * 0.3f, y), ImVec2(x, y + sz - 0.5f), ImVec2(x + sz * 0.6f, y + sz - 0.5f), IM_COL32(255, 255, 0, 255), th);
    ImGui::Dummy(ImVec2(sz, sz));
}
{
    // #2868
    float th = 2.0f;
    ImGui::Text("#2868");
    const ImVec2 p = ImGui::GetCursorScreenPos();
    draw_list->AddTriangle(p, ImVec2(p.x + 100, p.y + 0), ImVec2(p.x + 0, p.y + 100), IM_COL32(0, 255, 0, 255), th);
    ImGui::Dummy(ImVec2(100, 100));
}
{
    // #3366
    float th = 2.0f;
    ImGui::Text("#3366");
    const ImVec2 p = ImGui::GetCursorScreenPos();
    float x = p.x, y = p.y;
    if (1) {
        // The version I want to work
        ImVector<ImVec2> path;
        path.push_back(ImVec2(x + 0, y + 50));
        path.push_back(ImVec2(x + 98, y + 50));
        path.push_back(ImVec2(x + 99, y + 60));
        path.push_back(ImVec2(x + 100, y + 00));
        path.push_back(ImVec2(x + 110, y + 50));
        path.push_back(ImVec2(x + 120, y + 50));
        path.push_back(ImVec2(x + 200, y + 50));
        draw_list->AddLine(ImVec2(x, y + 60.0f), ImVec2(x + 200, y + 60.0f), IM_COL32(255, 255, 0, 255));
        draw_list->AddPolyline(&path[0], path.size(), IM_COL32(0xff, 0x5b, 0xfc, 0xff), false, 2.0f);
        ImGui::Dummy(ImVec2(200, 80));
    }
}
{
    // #2183
    ImGui::Text("#2183");
    const ImVec2 p = ImGui::GetCursorScreenPos();
    ImVec2 points[] = { ImVec2(p.x + 100, p.y + 70), ImVec2(p.x + 140, p.y + 750), ImVec2(p.x + 50, p.y + 0) };
    ImGui::GetWindowDrawList()->AddPolyline(points, 3, 0xFF00FFFF, true, 3.0f);
    ImGui::Dummy(ImVec2(0, 750.0f));
}

)

[geekynils]

That was super-quick, thanks!