From a39f9e766147a9d018814894314ec6a64d011479 Mon Sep 17 00:00:00 2001 From: ocornut Date: Thu, 1 Jun 2023 17:49:33 +0200 Subject: [PATCH] MultiSelect: Internals rename of IO fields to avoid ambiguity with io/rw concepts + memset constructors, tweaks. debug --- imgui.h | 32 +++++++--------- imgui_demo.cpp | 2 +- imgui_internal.h | 13 ++++--- imgui_widgets.cpp | 96 +++++++++++++++++++++++------------------------ 4 files changed, 69 insertions(+), 74 deletions(-) diff --git a/imgui.h b/imgui.h index b18a770ba..7d0582a28 100644 --- a/imgui.h +++ b/imgui.h @@ -2781,26 +2781,20 @@ enum ImGuiMultiSelectFlags_ // If you submit all items (no clipper), Step 2 and 3 and will be handled by Selectable() on a per-item basis. struct ImGuiMultiSelectIO { - // Output (return by BeginMultiSelect()/EndMultiSelect() - // - Always process requests in their structure order. - bool RequestClear; // Begin, End // 1. Request user to clear selection - bool RequestSelectAll; // Begin, End // 2. Request user to select all - bool RequestSetRange; // End // 3. Request user to set or clear selection in the [RangeSrc..RangeDst] range - bool RangeValue; // End // End: parameter from RequestSetRange request. true = Select Range, false = Unselect Range. - void* RangeSrc; // Begin, End // End: parameter from RequestSetRange request + you need to save this value so you can pass it again next frame. / Begin: this is the value you passed to BeginMultiSelect() - void* RangeDst; // End // End: parameter from RequestSetRange request. - int RangeDirection; // End // End: parameter from RequestSetRange request. +1 if RangeSrc came before RangeDst, -1 otherwise. Available as an indicator in case you cannot infer order from the void* values. If your void* values are storing indices you will never need this. + // - Always process requests in this order: Clear, SelectAll, SetRange. + // - Below: who reads/writes each fields? 'r'=read, 'w'=write, 'ms'=multi-select code, 'app'=application/user code. + // // BEGIN / LOOP / END + bool RequestClear; // ms:w, app:r / / ms:w, app:r // 1. Request user to clear selection (processed by app code) + bool RequestSelectAll; // ms:w, app:r / / ms:w, app:r // 2. Request user to select all (processed by app code) + bool RequestSetRange; // / / ms:w, app:r // 3. Request user to alter selection in the [RangeSrc..RangeDst] range using RangeValue. In practice, only EndMultiSelect() request this, app code can read after BeginMultiSelect() and it will always be false. + void* RangeSrc; // ms:w / app:r / ms:w, app:r // Begin: Last known RangeSrc value. End: parameter from RequestSetRange request. + void* RangeDst; // / / ms:w, app:r // End: parameter from RequestSetRange request. + ImS8 RangeDirection; // / / ms:w, app:r // End: parameter from RequestSetRange request. +1 if RangeSrc came before RangeDst, -1 otherwise. Available as an indicator in case you cannot infer order from the void* values. If your void* values are storing indices you will never need this. + bool RangeValue; // / / ms:w, app:r // End: parameter from RequestSetRange request. true = Select Range, false = Unselect Range. + bool RangeSrcPassedBy; // / ms:rw app:w / ms:r // (If using a clipper) Need to be set by user if RangeSrc was part of the clipped set before submitting the visible items. Ignore if not clipping. - // Input (written by user between BeginMultiSelect()/EndMultiSelect() - bool RangeSrcPassedBy; // Loop // (If using a clipper) Need to be set by user if RangeSrc was part of the clipped set before submitting the visible items. Ignore if not clipping. - - ImGuiMultiSelectIO() { Clear(); } - void Clear() - { - RequestClear = RequestSelectAll = RequestSetRange = RangeSrcPassedBy = RangeValue = false; - RangeSrc = RangeDst = NULL; - RangeDirection = 0; - } + ImGuiMultiSelectIO() { Clear(); } + void Clear() { memset(this, 0, sizeof(*this)); } }; //----------------------------------------------------------------------------- diff --git a/imgui_demo.cpp b/imgui_demo.cpp index 8117437da..a07cc6ebf 100644 --- a/imgui_demo.cpp +++ b/imgui_demo.cpp @@ -2779,7 +2779,7 @@ struct ExampleSelection { // Data ImGuiStorage Storage; // Selection set - int SelectionSize; // Number of selected items (== number of 1 in the Storage, maintained by this class) + int SelectionSize; // Number of selected items (== number of 1 in the Storage, maintained by this class) // FIXME-RANGESELECT: Imply more difficult to track with intrusive selection schemes? int RangeRef; // Reference/pivot item (generally last clicked item) // Functions diff --git a/imgui_internal.h b/imgui_internal.h index 4770a2fd6..42f822c38 100644 --- a/imgui_internal.h +++ b/imgui_internal.h @@ -1713,21 +1713,22 @@ struct ImGuiOldColumns #ifdef IMGUI_HAS_MULTI_SELECT +// Temporary storage for multi-select struct IMGUI_API ImGuiMultiSelectTempData { ImGuiID FocusScopeId; // Copied from g.CurrentFocusScopeId (unless another selection scope was pushed manually) ImGuiMultiSelectFlags Flags; ImGuiKeyChord KeyMods; ImGuiWindow* Window; - ImGuiMultiSelectIO In; // The In requests are set and returned by BeginMultiSelect() - ImGuiMultiSelectIO Out; // The Out requests are finalized and returned by EndMultiSelect() + ImGuiMultiSelectIO BeginIO; // Requests are set and returned by BeginMultiSelect(), written to by user during the loop. + ImGuiMultiSelectIO EndIO; // Requests are set during the loop and returned by EndMultiSelect(). bool IsFocused; // Set if currently focusing the selection scope (any item of the selection). May be used if you have custom shortcut associated to selection. - bool InRangeDstPassedBy; // (Internal) set by the the item that match NavJustMovedToId when InRequestRangeSetNav is set. - bool InRequestSetRangeNav; // (Internal) set by BeginMultiSelect() when using Shift+Navigation. Because scrolling may be affected we can't afford a frame of lag with Shift+Navigation. + bool IsSetRange; // Set by BeginMultiSelect() when using Shift+Navigation. Because scrolling may be affected we can't afford a frame of lag with Shift+Navigation. + bool SetRangeDstPassedBy; // Set by the the item that matches NavJustMovedToId when IsSetRange is set. //ImRect Rect; // Extent of selection scope between BeginMultiSelect() / EndMultiSelect(), used by ImGuiMultiSelectFlags_ClearOnClickRectVoid. - ImGuiMultiSelectTempData() { Clear(); } - void Clear() { FocusScopeId = 0; Flags = ImGuiMultiSelectFlags_None; KeyMods = ImGuiMod_None; Window = NULL; In.Clear(); Out.Clear(); InRangeDstPassedBy = InRequestSetRangeNav = false; } + ImGuiMultiSelectTempData() { Clear(); } + void Clear() { memset(this, 0, sizeof(*this)); } }; #endif // #ifdef IMGUI_HAS_MULTI_SELECT diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index de53de54d..d9008e340 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -7120,21 +7120,22 @@ void ImGui::DebugNodeTypingSelectState(ImGuiTypingSelectState* data) // - SetNextItemSelectionUserData() // - MultiSelectItemHeader() [Internal] // - MultiSelectItemFooter() [Internal] +// - DebugNodeMultiSelectState() [Internal] //------------------------------------------------------------------------- -static void DebugLogMultiSelectRequests(const ImGuiMultiSelectIO* data) +static void DebugLogMultiSelectRequests(const char* function, const ImGuiMultiSelectIO* data) { ImGuiContext& g = *GImGui; - if (data->RequestClear) IMGUI_DEBUG_LOG_SELECTION("EndMultiSelect: RequestClear\n"); - if (data->RequestSelectAll) IMGUI_DEBUG_LOG_SELECTION("EndMultiSelect: RequestSelectAll\n"); - if (data->RequestSetRange) IMGUI_DEBUG_LOG_SELECTION("EndMultiSelect: RequestSetRange %p..%p = %d (dir %+d)\n", data->RangeSrc, data->RangeDst, data->RangeValue, data->RangeDirection); + if (data->RequestClear) IMGUI_DEBUG_LOG_SELECTION("[selection] %s: RequestClear\n", function); + if (data->RequestSelectAll) IMGUI_DEBUG_LOG_SELECTION("[selection] %s: RequestSelectAll\n", function); + if (data->RequestSetRange) IMGUI_DEBUG_LOG_SELECTION("[selection] %s: RequestSetRange %p..%p = %d (dir %+d)\n", function, data->RangeSrc, data->RangeDst, data->RangeValue, data->RangeDirection); } ImGuiMultiSelectIO* ImGui::BeginMultiSelect(ImGuiMultiSelectFlags flags, void* range_ref, bool range_ref_is_selected) { ImGuiContext& g = *GImGui; ImGuiWindow* window = g.CurrentWindow; - ImGuiMultiSelectTempData* ms = &g.MultiSelectTempData[0];; + ImGuiMultiSelectTempData* ms = &g.MultiSelectTempData[0]; IM_ASSERT(g.CurrentMultiSelect == NULL); // No recursion allowed yet (we could allow it if we deem it useful) g.CurrentMultiSelect = ms; @@ -7151,36 +7152,35 @@ ImGuiMultiSelectIO* ImGui::BeginMultiSelect(ImGuiMultiSelectFlags flags, void* r if ((flags & ImGuiMultiSelectFlags_NoMultiSelect) == 0) { - ms->In.RangeSrc = ms->Out.RangeSrc = range_ref; - ms->In.RangeValue = ms->Out.RangeValue = range_ref_is_selected; + ms->BeginIO.RangeSrc = ms->EndIO.RangeSrc = range_ref; + ms->BeginIO.RangeValue = ms->EndIO.RangeValue = range_ref_is_selected; } // Auto clear when using Navigation to move within the selection (we compare SelectScopeId so it possible to use multiple lists inside a same window) - // FIXME: Polling key mods after the fact (frame following the move request) is incorrect, but latching it would requires non-trivial change in MultiSelectItemFooter() if (g.NavJustMovedToId != 0 && g.NavJustMovedToFocusScopeId == ms->FocusScopeId && g.NavJustMovedToHasSelectionData) { if (ms->KeyMods & ImGuiMod_Shift) - ms->InRequestSetRangeNav = true; + ms->IsSetRange = true; if ((ms->KeyMods & (ImGuiMod_Ctrl | ImGuiMod_Shift)) == 0) - ms->In.RequestClear = true; + ms->BeginIO.RequestClear = true; } // Shortcut: Select all (CTRL+A) if (ms->IsFocused && !(flags & ImGuiMultiSelectFlags_NoMultiSelect) && !(flags & ImGuiMultiSelectFlags_NoSelectAll)) if (Shortcut(ImGuiMod_Ctrl | ImGuiKey_A)) - ms->In.RequestSelectAll = true; + ms->BeginIO.RequestSelectAll = true; // Shortcut: Clear selection (Escape) // FIXME-MULTISELECT: Only hog shortcut if selection is not null, meaning we need "has selection or "selection size" data here. // Otherwise may be done by caller but it means Shortcut() needs to be exposed. if (ms->IsFocused && (flags & ImGuiMultiSelectFlags_ClearOnEscape)) if (Shortcut(ImGuiKey_Escape)) - ms->In.RequestClear = true; + ms->BeginIO.RequestClear = true; if (g.DebugLogFlags & ImGuiDebugLogFlags_EventSelection) - DebugLogMultiSelectRequests(&ms->In); + DebugLogMultiSelectRequests("BeginMultiSelect", &ms->BeginIO); - return &ms->In; + return &ms->BeginIO; } ImGuiMultiSelectIO* ImGui::EndMultiSelect() @@ -7196,13 +7196,13 @@ ImGuiMultiSelectIO* ImGui::EndMultiSelect() if (IsWindowHovered() && g.HoveredId == 0) if (IsMouseReleased(0) && IsMouseDragPastThreshold(0) == false && g.IO.KeyMods == ImGuiMod_None) { - ms->Out.RequestClear = true; - ms->Out.RequestSelectAll = ms->Out.RequestSetRange = false; + ms->EndIO.RequestClear = true; + ms->EndIO.RequestSelectAll = ms->EndIO.RequestSetRange = false; } // Unwind if (ms->Flags & ImGuiMultiSelectFlags_NoUnselect) - ms->Out.RangeValue = true; + ms->EndIO.RangeValue = true; ms->FocusScopeId = 0; ms->Window = NULL; ms->Flags = ImGuiMultiSelectFlags_None; @@ -7210,9 +7210,9 @@ ImGuiMultiSelectIO* ImGui::EndMultiSelect() g.CurrentMultiSelect = NULL; if (g.DebugLogFlags & ImGuiDebugLogFlags_EventSelection) - DebugLogMultiSelectRequests(&ms->Out); + DebugLogMultiSelectRequests("EndMultiSelect", &ms->EndIO); - return &ms->Out; + return &ms->EndIO; } void ImGui::SetNextItemSelectionUserData(ImGuiSelectionUserData selection_user_data) @@ -7229,8 +7229,8 @@ void ImGui::SetNextItemSelectionUserData(ImGuiSelectionUserData selection_user_d // Auto updating RangeSrcPassedBy for cases were clipper is not used (done before ItemAdd() clipping) if (ImGuiMultiSelectTempData* ms = g.CurrentMultiSelect) - if (ms->In.RangeSrc == (void*)selection_user_data) - ms->In.RangeSrcPassedBy = true; + if (ms->BeginIO.RangeSrc == (void*)selection_user_data) + ms->BeginIO.RangeSrcPassedBy = true; } void ImGui::MultiSelectItemHeader(ImGuiID id, bool* p_selected) @@ -7248,23 +7248,23 @@ void ImGui::MultiSelectItemHeader(ImGuiID id, bool* p_selected) // This is only useful if the user hasn't processed them already, and this only works if the user isn't using the clipper. // If you are using a clipper (aka not submitting every element of the list) you need to process the Clear/SelectAll request after calling BeginMultiSelect() bool selected = *p_selected; - if (ms->In.RequestClear) + if (ms->BeginIO.RequestClear) selected = false; - else if (ms->In.RequestSelectAll) + else if (ms->BeginIO.RequestSelectAll) selected = true; // When using SHIFT+Nav: because it can incur scrolling we cannot afford a frame of lag with the selection highlight (otherwise scrolling would happen before selection) // For this to work, IF the user is clipping items, they need to set RangeSrcPassedBy = true to notify the system. - if (ms->InRequestSetRangeNav) + if (ms->IsSetRange) { IM_ASSERT(id != 0); IM_ASSERT((ms->KeyMods & ImGuiMod_Shift) != 0); - const bool is_range_src = (ms->In.RangeSrc == item_data); - const bool is_range_dst = !ms->InRangeDstPassedBy && g.NavJustMovedToId == id; // Assume that g.NavJustMovedToId is not clipped. + const bool is_range_src = (ms->BeginIO.RangeSrc == item_data); + const bool is_range_dst = !ms->SetRangeDstPassedBy && g.NavJustMovedToId == id; // Assume that g.NavJustMovedToId is not clipped. if (is_range_dst) - ms->InRangeDstPassedBy = true; - if (is_range_src || is_range_dst || ms->In.RangeSrcPassedBy != ms->InRangeDstPassedBy) - selected = ms->In.RangeValue; + ms->SetRangeDstPassedBy = true; + if (is_range_src || is_range_dst || ms->BeginIO.RangeSrcPassedBy != ms->SetRangeDstPassedBy) + selected = ms->BeginIO.RangeValue; else if ((ms->KeyMods & ImGuiMod_Ctrl) == 0) selected = false; } @@ -7331,53 +7331,53 @@ void ImGui::MultiSelectItemFooter(ImGuiID id, bool* p_selected, bool* p_pressed) if (is_shift && is_multiselect) { // Shift+Arrow always select, Ctrl+Shift+Arrow copy source selection state. - ms->Out.RequestSetRange = true; - ms->Out.RangeDst = item_data; + ms->EndIO.RequestSetRange = true; + ms->EndIO.RangeDst = item_data; if (!is_ctrl) - ms->Out.RangeValue = true; - ms->Out.RangeDirection = ms->In.RangeSrcPassedBy ? +1 : -1; + ms->EndIO.RangeValue = true; + ms->EndIO.RangeDirection = ms->BeginIO.RangeSrcPassedBy ? +1 : -1; } else { // Ctrl inverts selection, otherwise always select selected = (is_ctrl && (ms->Flags & ImGuiMultiSelectFlags_NoUnselect) == 0) ? !selected : true; - ms->Out.RangeSrc = ms->Out.RangeDst = item_data; - ms->Out.RangeValue = selected; + ms->EndIO.RangeSrc = ms->EndIO.RangeDst = item_data; + ms->EndIO.RangeValue = selected; } if (input_source == ImGuiInputSource_Mouse || g.NavActivateId == id) { // Mouse click without CTRL clears the selection, unless the clicked item is already selected if (is_multiselect && !is_ctrl) - ms->Out.RequestClear = true; - if (is_multiselect && !is_shift && ms->Out.RequestClear) + ms->EndIO.RequestClear = true; + if (is_multiselect && !is_shift && ms->EndIO.RequestClear) { // For toggle selection unless there is a Clear request, we can handle it completely locally without sending a RangeSet request. - IM_ASSERT(ms->Out.RangeSrc == ms->Out.RangeDst); // Setup by else block above - ms->Out.RequestSetRange = true; - ms->Out.RangeValue = selected; - ms->Out.RangeDirection = +1; + IM_ASSERT(ms->EndIO.RangeSrc == ms->EndIO.RangeDst); // Setup by else block above + ms->EndIO.RequestSetRange = true; + ms->EndIO.RangeValue = selected; + ms->EndIO.RangeDirection = +1; } if (!is_multiselect) { // Clear selection, set single item range - IM_ASSERT(ms->Out.RangeSrc == item_data && ms->Out.RangeDst == item_data); // Setup by block above - ms->Out.RequestClear = true; - ms->Out.RequestSetRange = true; + IM_ASSERT(ms->EndIO.RangeSrc == item_data && ms->EndIO.RangeDst == item_data); // Setup by block above + ms->EndIO.RequestClear = true; + ms->EndIO.RequestSetRange = true; } } else if (input_source == ImGuiInputSource_Keyboard || input_source == ImGuiInputSource_Gamepad) { if (is_multiselect && is_shift && !is_ctrl) - ms->Out.RequestClear = true; + ms->EndIO.RequestClear = true; else if (!is_multiselect) - ms->Out.RequestClear = true; + ms->EndIO.RequestClear = true; } } // Update/store the selection state of the Source item (used by CTRL+SHIFT, when Source is unselected we perform a range unselect) - if (ms->Out.RangeSrc == item_data && is_ctrl && is_shift && is_multiselect && !(ms->Flags & ImGuiMultiSelectFlags_NoUnselect)) - ms->Out.RangeValue = selected; + if (ms->EndIO.RangeSrc == item_data && is_ctrl && is_shift && is_multiselect && !(ms->Flags & ImGuiMultiSelectFlags_NoUnselect)) + ms->EndIO.RangeValue = selected; *p_selected = selected; *p_pressed = pressed;