DrawCmd: Added ForbidMerging boolean

[lut0pia]

Hello,

We've been using ImGui at work for some time and while extending our usage recently I came upon an issue in ImGui::RenderDimmedBackgroundBehindWindow where the assert making sure that the draw commands were not merged fired. The circumstances seemed to meet when attempting to draw the window selection widget while moving another window into a new viewport/window with a gamepad (I'm sure some other specifics were involved but I don't know what they are). I saw the FIXME so I tried finding a way to avoid the issue without changing everything. This is working for us and I thought that it could work for others too, but I haven't found much discussion around that issue (possibly our setup really is that rare?).

For more context we're using this Unreal Engine plugin with a few minor changes to it (but AFAIK the issue is present in vanilla too).

One thing I don't know is if there's other places to check for that newly created boolean.

Hopefully this can be useful, thanks for this great piece of software!

[ocornut]

Hello Lou, thanks for your PR.

I realize the FIXME suggested that, but I would prefer to study/understand and fix the root issue better and I would need a repro.

Can you easily repro it and could you provide more information?
Which version are you precisely on? (IMGUI_VERSION_NUM value)

FYI the original code could be expanded with an earlier assert:

draw_list->PushClipRect(viewport_rect.Min - ImVec2(1, 1), viewport_rect.Max + ImVec2(1, 1), false); // FIXME: Need to stricty ensure ImDrawCmd are not merged (ElemCount==6 checks below will verify that)
ImDrawCmd cmd = draw_list->CmdBuffer.back();
IM_ASSERT(cmd.ElemCount == 0);
draw_list->AddRectFilled(viewport_rect.Min, viewport_rect.Max, col);
cmd = draw_list->CmdBuffer.back();
IM_ASSERT(cmd.ElemCount == 6);

Supposedly if the assert triggers it is because PushClipRect() did not create a new command.
If that happen you could technically move execution cursor back to the PushClipRect() call and try to understand why.

[lut0pia]

Hello Omar,

I totally understand that this is closer to a workaround than a fix. I understand that ideally the rendering of the backgrounds would not involve juggling with the draw commands like this, but it definitely seemed out of my depth to rework the system to that extent.

The version we're using is 19180. Also I realize I forgot to mention we're using the docking branch.

I've been doing tests with:

draw_list->PushClipRect(viewport_rect.Min - ImVec2(1, 1), viewport_rect.Max + ImVec2(1, 1), false); // FIXME: Need to stricty ensure ImDrawCmd are not merged (ElemCount==6 checks below will verify that)
IM_ASSERT(draw_list->CmdBuffer.back().ElemCount == 0);
draw_list->AddRectFilled(viewport_rect.Min, viewport_rect.Max, col);
ImDrawCmd cmd = draw_list->CmdBuffer.back();
IM_ASSERT(cmd.ElemCount == 6);

And it is the first assert (== 0) that fires.

I wasn't able to repro in the default ImGui samples, but I was able to repro using vanilla UE and the vanilla ImGui plugin, so I created an issue there.

I was able to see what was happening inside PushClipRect and my understanding is that inside _OnChangedClipRect, it turned out that the previous header was the same as the current one, which resulted in the merge of cmds. The thing I can't really understand is why it happens in our case and not in vanilla ImGui, it could be that it just depends on whatever it was drawing before it, right?

Thanks again for your time!