Backends: Allegro: Brough back al_draw_indexed_prim.

[Espyo]

Context:

Some time back, Allegro's DirectX implementation of al_draw_indexed_prim broke. This originated commit 185b4dd in Dear ImGui. Some time later though, version 5.2.5 (February 2019) of Allegro ended up fixing this problem.

My changes:

I brought back the indexed primitive logic from that Dear ImGui commit, and updated it to today's code. Not only is this more optimized since it makes use of indexing, but it also removes an annoying compilation warning ("variable ‘indices’ set but not used [-Wunused-but-set-variable]") considering indices is no longer unused. One thing to note: al_draw_indexed_prim doesn't seem to like it when the num_vtx argument is 0, which Dear ImGui sometimes does. I figured if'ing that case and skipping was the best solution.

I admit I'm not 100% familiar with how that code works, but I'm confident enough to submit this PR. Basically, only the latest commit matters. But if this PR ends up being an unwanted change for some reason, then I instead propose using the first commit of the branch, which just removes the unused bit of code with the indices variable. Functionally, it's the same as before my PR, but with fewer compilation warnings. (That commit was made some hours ago before I realized Allegro had fixed the problem, and that I should probably try my hand at updating the backend. Oops.)

Testing:

I've tried running the Dear ImGui demo with my PR's branch and found absolutely no problems. Tested on my Lubuntu machine, tested DirectX on a dinky Windows 7 virtual machine, and tested OpenGL and DirectX on a Windows 10 64-bit machine.

• Linux compiler: g++ (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0
• Windows VM compiler: Visual Studio Community 2019.
• Operating System: Lubuntu 22.04.1 LTS 64-bit

[ocornut]

Hello,
Thanks for the PR!
This would need to do a compile time #if version check to support older and newer versions, and the change needs to be in line with local coding style.

cheers,
Omar

[ocornut]

Why did you add the if (pcmd->ElemCount > 0) test? It seems unnecessary?

[ocornut]

I have merged your commit 50aeeff + fix/tweaks to support both versions a5f3596 + other misc tweaks
Thank you!

[Espyo]

Regarding the check if (pcmd->ElemCount > 0), I was getting some instances in my program where Dear ImGui was sending 0 as the value. I tried again on the demo window and couldn't reproduce it, but I was able to create a MRE that does reproduce it.

Using examples/example_allegro5/main.cpp as a basis, I've replaced the three windows between ImGui::NewFrame(); and ImGui::Render(); with simply:

        ImGui::OpenPopup("some dialog");
        if(ImGui::BeginPopupModal("some dialog", &is_open)) {
            node_is_open = ImGui::TreeNode("asda");
            if(node_is_open) {
                ImGui::TreePop();
            }
            ImGui::EndPopup();
        }

So right now, with Dear ImGui >= a5f3596 and Allegro >= 5.2.5, the program will crash whenever the user creates a modal with a TreeNode inside, because Dear ImGui is sending Allegro a pcmd->ElemCount of 0 for some unknown reason.

Would it make sense to continue investigating this pcmd->ElemCount == 0 debacle in this PR's thread, or should I open an issue for this crash?

[ocornut]

Very interesting! I’ll investigate that, from my pov it seems like a bug in core dear imgui but it is interesting that no other backends got affected by that. Will fix one way or another soon :)

[ocornut]

Pushed a fix 9825f7f.
Turns out Metal validation layer would be similarly affected. (same issue as #4857)
Thanks a lot!

[Espyo]

Awesome! I've tested a bunch and I can't get any crash at all. Looks like everything is fine now. Thanks!