From 075f4ac66127cc20a72e65720645922b5817ccf1 Mon Sep 17 00:00:00 2001 From: ocornut Date: Wed, 26 Jan 2022 15:53:18 +0100 Subject: [PATCH] Don't merge ImDrawCmd which have had their IdxOffset changed to not be sequential. Fixed CTRL+Tab into an empty window causing artefacts on the highlight rectangle due to bad reordering on ImDrawCmd. This is bit of a weird edge case adding weight to ImDrawCmd merging, if we could rework the mess in RenderDimmedBackgroundBehindWindow() we may be able to undo some of that. --- docs/CHANGELOG.txt | 1 + imgui.cpp | 5 ++++- imgui_draw.cpp | 15 +++++++++------ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index cbd324b51..37391478a 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -107,6 +107,7 @@ Other Changes: handled by core automatically for all kind of inputs. (#4858, #2787, #1992, #3383, #2525, #1320) - New IO functions for keyboard/gamepad: AddKeyEvent(), AddKeyAnalogEvent(), AddKeyModsEvent(). - New IO functions for mouse: AddMousePosEvent(), AddMouseButtonEvent(), AddMouseWheelEvent(). +- Fixed CTRL+Tab into an empty window causing artefacts on the highlight rectangle due to bad reordering on ImDrawCmd. - Fixed a situation where CTRL+Tab or Modal can occasionally lead to the creation of ImDrawCmd with zero triangles, which would makes the draw operation of some backends assert (e.g. Metal with debugging). (#4857) - Popups: Fixed a regression crash when a new window is created after a modal on the same frame. (#4920) [@rokups] diff --git a/imgui.cpp b/imgui.cpp index 77ce8cadd..87a3beadc 100644 --- a/imgui.cpp +++ b/imgui.cpp @@ -4678,6 +4678,7 @@ static void ImGui::RenderDimmedBackgroundBehindWindow(ImGuiWindow* window, ImU32 { // We've already called AddWindowToDrawData() which called DrawList->ChannelsMerge() on DockNodeHost windows, // and draw list have been trimmed already, hence the explicit recreation of a draw command if missing. + // FIXME: This is creating complication, might be simpler if we could inject a drawlist in drawdata at a given position and not attempt to manipulate ImDrawCmd order. ImDrawList* draw_list = window->RootWindow->DrawList; if (draw_list->CmdBuffer.Size == 0) draw_list->AddDrawCmd(); @@ -4688,7 +4689,7 @@ static void ImGui::RenderDimmedBackgroundBehindWindow(ImGuiWindow* window, ImU32 draw_list->CmdBuffer.pop_back(); draw_list->CmdBuffer.push_front(cmd); draw_list->PopClipRect(); - draw_list->_PopUnusedDrawCmd(); // Since are past the calls to AddDrawListToDrawData() we don't have a _PopUnusedDrawCmd() running on commands. + draw_list->AddDrawCmd(); // We need to create a command as CmdBuffer.back().IdxOffset won't be correct if we append to same command. } } @@ -4713,6 +4714,8 @@ static void ImGui::RenderDimmedBackgrounds() { ImGuiContext& g = *GImGui; ImGuiWindow* modal_window = GetTopMostAndVisiblePopupModal(); + if (g.DimBgRatio <= 0.0f && g.NavWindowingHighlightAlpha <= 0.0f) + return; const bool dim_bg_for_modal = (modal_window != NULL); const bool dim_bg_for_window_list = (g.NavWindowingTargetAnim != NULL && g.NavWindowingTargetAnim->Active); if (!dim_bg_for_modal && !dim_bg_for_window_list) diff --git a/imgui_draw.cpp b/imgui_draw.cpp index 423f8cd27..d431c61ae 100644 --- a/imgui_draw.cpp +++ b/imgui_draw.cpp @@ -488,9 +488,10 @@ void ImDrawList::AddCallback(ImDrawCallback callback, void* callback_data) } // Compare ClipRect, TextureId and VtxOffset with a single memcmp() -#define ImDrawCmd_HeaderSize (IM_OFFSETOF(ImDrawCmd, VtxOffset) + sizeof(unsigned int)) -#define ImDrawCmd_HeaderCompare(CMD_LHS, CMD_RHS) (memcmp(CMD_LHS, CMD_RHS, ImDrawCmd_HeaderSize)) // Compare ClipRect, TextureId, VtxOffset -#define ImDrawCmd_HeaderCopy(CMD_DST, CMD_SRC) (memcpy(CMD_DST, CMD_SRC, ImDrawCmd_HeaderSize)) // Copy ClipRect, TextureId, VtxOffset +#define ImDrawCmd_HeaderSize (IM_OFFSETOF(ImDrawCmd, VtxOffset) + sizeof(unsigned int)) +#define ImDrawCmd_HeaderCompare(CMD_LHS, CMD_RHS) (memcmp(CMD_LHS, CMD_RHS, ImDrawCmd_HeaderSize)) // Compare ClipRect, TextureId, VtxOffset +#define ImDrawCmd_HeaderCopy(CMD_DST, CMD_SRC) (memcpy(CMD_DST, CMD_SRC, ImDrawCmd_HeaderSize)) // Copy ClipRect, TextureId, VtxOffset +#define ImDrawCmd_AreSequentialIdxOffset(CMD_0, CMD_1) (CMD_0->IdxOffset + CMD_0->ElemCount == CMD_1->IdxOffset) // Try to merge two last draw commands void ImDrawList::_TryMergeDrawCmds() @@ -498,7 +499,7 @@ void ImDrawList::_TryMergeDrawCmds() IM_ASSERT_PARANOID(CmdBuffer.Size > 0); ImDrawCmd* curr_cmd = &CmdBuffer.Data[CmdBuffer.Size - 1]; ImDrawCmd* prev_cmd = curr_cmd - 1; - if (ImDrawCmd_HeaderCompare(curr_cmd, prev_cmd) == 0 && curr_cmd->UserCallback == NULL && prev_cmd->UserCallback == NULL) + if (ImDrawCmd_HeaderCompare(curr_cmd, prev_cmd) == 0 && ImDrawCmd_AreSequentialIdxOffset(prev_cmd, curr_cmd) && curr_cmd->UserCallback == NULL && prev_cmd->UserCallback == NULL) { prev_cmd->ElemCount += curr_cmd->ElemCount; CmdBuffer.pop_back(); @@ -521,7 +522,7 @@ void ImDrawList::_OnChangedClipRect() // Try to merge with previous command if it matches, else use current command ImDrawCmd* prev_cmd = curr_cmd - 1; - if (curr_cmd->ElemCount == 0 && CmdBuffer.Size > 1 && ImDrawCmd_HeaderCompare(&_CmdHeader, prev_cmd) == 0 && prev_cmd->UserCallback == NULL) + if (curr_cmd->ElemCount == 0 && CmdBuffer.Size > 1 && ImDrawCmd_HeaderCompare(&_CmdHeader, prev_cmd) == 0 && ImDrawCmd_AreSequentialIdxOffset(prev_cmd, curr_cmd) && prev_cmd->UserCallback == NULL) { CmdBuffer.pop_back(); return; @@ -544,7 +545,7 @@ void ImDrawList::_OnChangedTextureID() // Try to merge with previous command if it matches, else use current command ImDrawCmd* prev_cmd = curr_cmd - 1; - if (curr_cmd->ElemCount == 0 && CmdBuffer.Size > 1 && ImDrawCmd_HeaderCompare(&_CmdHeader, prev_cmd) == 0 && prev_cmd->UserCallback == NULL) + if (curr_cmd->ElemCount == 0 && CmdBuffer.Size > 1 && ImDrawCmd_HeaderCompare(&_CmdHeader, prev_cmd) == 0 && ImDrawCmd_AreSequentialIdxOffset(prev_cmd, curr_cmd) && prev_cmd->UserCallback == NULL) { CmdBuffer.pop_back(); return; @@ -1738,6 +1739,8 @@ void ImDrawListSplitter::Merge(ImDrawList* draw_list) if (ch._CmdBuffer.Size > 0 && last_cmd != NULL) { + // Do not include ImDrawCmd_AreSequentialIdxOffset() in the compare as we rebuild IdxOffset values ourselves. + // Manipulating IdxOffset (e.g. by reordering draw commands like done by RenderDimmedBackgroundBehindWindow()) is not supported within a splitter. ImDrawCmd* next_cmd = &ch._CmdBuffer[0]; if (ImDrawCmd_HeaderCompare(last_cmd, next_cmd) == 0 && last_cmd->UserCallback == NULL && next_cmd->UserCallback == NULL) {