Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for ImDrawListSplitter::Merge - corrupted data in buffer (64k+ vertices) with 16-bits indices #3129 #3163

Closed
wants to merge 6 commits into from
1 change: 1 addition & 0 deletions imgui.h
Original file line number Diff line number Diff line change
Expand Up @@ -2068,6 +2068,7 @@ struct ImDrawList
inline void PrimVtx(const ImVec2& pos, const ImVec2& uv, ImU32 col) { PrimWriteIdx((ImDrawIdx)_VtxCurrentIdx); PrimWriteVtx(pos, uv, col); }
IMGUI_API void UpdateClipRect();
IMGUI_API void UpdateTextureID();
IMGUI_API void UpdateStaleDrawCmd();
};

// All draw data to render a Dear ImGui frame
Expand Down
45 changes: 41 additions & 4 deletions imgui_draw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ void ImDrawList::UpdateClipRect()

// Try to merge with previous command if it matches, else use current command
ImDrawCmd* prev_cmd = CmdBuffer.Size > 1 ? curr_cmd - 1 : NULL;
if (curr_cmd->ElemCount == 0 && prev_cmd && memcmp(&prev_cmd->ClipRect, &curr_clip_rect, sizeof(ImVec4)) == 0 && prev_cmd->TextureId == GetCurrentTextureId() && prev_cmd->UserCallback == NULL)
if (curr_cmd->ElemCount == 0 && prev_cmd && memcmp(&prev_cmd->ClipRect, &curr_clip_rect, sizeof(ImVec4)) == 0 && prev_cmd->VtxOffset == _VtxCurrentOffset && prev_cmd->TextureId == GetCurrentTextureId() && prev_cmd->UserCallback == NULL)
CmdBuffer.pop_back();
else
curr_cmd->ClipRect = curr_clip_rect;
Expand All @@ -480,12 +480,49 @@ void ImDrawList::UpdateTextureID()

// Try to merge with previous command if it matches, else use current command
ImDrawCmd* prev_cmd = CmdBuffer.Size > 1 ? curr_cmd - 1 : NULL;
if (curr_cmd->ElemCount == 0 && prev_cmd && prev_cmd->TextureId == curr_texture_id && memcmp(&prev_cmd->ClipRect, &GetCurrentClipRect(), sizeof(ImVec4)) == 0 && prev_cmd->UserCallback == NULL)
if (curr_cmd->ElemCount == 0 && prev_cmd && prev_cmd->TextureId == curr_texture_id && memcmp(&prev_cmd->ClipRect, &GetCurrentClipRect(), sizeof(ImVec4)) == 0 && prev_cmd->VtxOffset == _VtxCurrentOffset && prev_cmd->UserCallback == NULL)
CmdBuffer.pop_back();
else
curr_cmd->TextureId = curr_texture_id;
}

// Use this when you're about to draw to pre-cached ImDrawCmd.
// It act like UpdateClipRect, UpdateTextureID and (not implemented UpdateVtxOffset) combined.
void ImDrawList::UpdateStaleDrawCmd()
{
// If current command is used with different settings we need to add a new command
ImDrawCmd* curr_cmd = CmdBuffer.Size ? &CmdBuffer.back() : NULL;
const ImVec4 curr_clip_rect = GetCurrentClipRect();
const ImTextureID curr_texture_id = GetCurrentTextureId();
if (!curr_cmd || (curr_cmd->ElemCount != 0 && ((curr_cmd->VtxOffset != _VtxCurrentOffset) || (curr_cmd->TextureId != curr_texture_id) || (memcmp(&curr_cmd->ClipRect, &curr_clip_rect, sizeof(ImVec4)) != 0))) || curr_cmd->UserCallback != NULL)
{
AddDrawCmd();
return;
}

// Try to merge with previous command if it matches, else use current command
ImDrawCmd* prev_cmd = CmdBuffer.Size > 1 ? curr_cmd - 1 : NULL;
const bool can_try_merge = (curr_cmd->ElemCount == 0 && prev_cmd && prev_cmd->UserCallback != NULL);
const bool has_same_texture_id = can_try_merge && (prev_cmd->TextureId == curr_texture_id);
const bool has_same_clip_rect = can_try_merge && (memcmp(&prev_cmd->ClipRect, &curr_clip_rect, sizeof(ImVec4)) == 0);
const bool has_same_vtx_offset = can_try_merge && (prev_cmd->VtxOffset == _VtxCurrentOffset);

if (has_same_texture_id && has_same_clip_rect && has_same_vtx_offset)
{
CmdBuffer.pop_back();
return;
}

if (!has_same_texture_id)
curr_cmd->TextureId = curr_texture_id;

if (!has_same_clip_rect)
curr_cmd->ClipRect = curr_clip_rect;

if (!has_same_vtx_offset)
curr_cmd->VtxOffset = _VtxCurrentOffset;
}

#undef GetCurrentClipRect
#undef GetCurrentTextureId

Expand Down Expand Up @@ -1403,8 +1440,7 @@ void ImDrawListSplitter::Merge(ImDrawList* draw_list)
if (int sz = ch._IdxBuffer.Size) { memcpy(idx_write, ch._IdxBuffer.Data, sz * sizeof(ImDrawIdx)); idx_write += sz; }
}
draw_list->_IdxWritePtr = idx_write;
draw_list->UpdateClipRect(); // We call this instead of AddDrawCmd(), so that empty channels won't produce an extra draw call.
draw_list->UpdateTextureID();
draw_list->UpdateStaleDrawCmd(); // We call this instead of AddDrawCmd(), so that empty channels won't produce an extra draw call.
_Count = 1;
}

Expand All @@ -1420,6 +1456,7 @@ void ImDrawListSplitter::SetCurrentChannel(ImDrawList* draw_list, int idx)
memcpy(&draw_list->CmdBuffer, &_Channels.Data[idx]._CmdBuffer, sizeof(draw_list->CmdBuffer));
memcpy(&draw_list->IdxBuffer, &_Channels.Data[idx]._IdxBuffer, sizeof(draw_list->IdxBuffer));
draw_list->_IdxWritePtr = draw_list->IdxBuffer.Data + draw_list->IdxBuffer.Size;
draw_list->UpdateStaleDrawCmd();
}

//-----------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion imgui_widgets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7553,7 +7553,7 @@ void ImGui::PushColumnsBackground()
int cmd_size = window->DrawList->CmdBuffer.Size;
PushClipRect(columns->HostClipRect.Min, columns->HostClipRect.Max, false);
IM_UNUSED(cmd_size);
IM_ASSERT(cmd_size == window->DrawList->CmdBuffer.Size); // Being in channel 0 this should not have created an ImDrawCmd
IM_ASSERT(cmd_size >= window->DrawList->CmdBuffer.Size); // Being in channel 0 this should not have created an ImDrawCmd
}

void ImGui::PopColumnsBackground()
Expand Down