From 8fe6b3195282b6247f53f75c0cfb5907448b05ec Mon Sep 17 00:00:00 2001 From: ocornut Date: Wed, 23 Aug 2023 20:45:02 +0200 Subject: [PATCH] MultiSelect: (Breaking) Removed RangeSrcPassedBy in favor of favoring user to call IncludeByIndex(RangeSrcItem) which is easier/simpler to honor. Especially as recent changes made it required to also update RangeSrcPassedBy after last clipper Step. Should now be simpler. --- imgui.h | 14 ++++---------- imgui_demo.cpp | 21 +++++---------------- imgui_internal.h | 3 ++- imgui_widgets.cpp | 10 +++++----- 4 files changed, 16 insertions(+), 32 deletions(-) diff --git a/imgui.h b/imgui.h index ea47d0fd8..efc7c6888 100644 --- a/imgui.h +++ b/imgui.h @@ -2774,13 +2774,8 @@ enum ImGuiMultiSelectFlags_ // Usage flow: // BEGIN - (1) Call BeginMultiSelect() and retrieve the ImGuiMultiSelectIO* result. // - (2) [If using clipper] Honor Clear/SelectAll/SetRange requests by updating your selection data. Same code as Step 6. -// LOOP - (3) [If using clipper] Set RangeSrcPassedBy=true if the RangeSrcItem item was already passed by. -// This is because for range-selection we need to know if we are currently "inside" or "outside" the range. -// - If you are using integer indices in ImGuiSelectionUserData, this is easy to compute: -// if (clipper.DisplayStart > data->RangeSrcItem) { data->RangeSrcPassedBy = true; } -// - If you are using pointers in ImGuiSelectionUserData, you may need additional processing, e.g. find the index of RangeSrcItem before applying the above operation. -// - This also needs to be done at the end of the clipper loop, otherwise we can't tell if the item still exist. -// - (4) Submit your items with SetNextItemSelectionUserData() + Selectable()/TreeNode() calls. +// - (3) [If using clipper] You need to make sure RangeSrcItem is always submitted. Calculate its index and pass to clipper.IncludeIndex(). If already using indices in ImGuiSelectionUserData, it is as simple as clipper.IncludeIndex((int)ms_io->RangeSrcItem); +// LOOP - (4) Submit your items with SetNextItemSelectionUserData() + Selectable()/TreeNode() calls. // END - (5) Call EndMultiSelect() and retrieve the ImGuiMultiSelectIO* result. // - (6) Honor Clear/SelectAll/SetRange requests by updating your selection data. Same code as Step 2. // If you submit all items (no clipper), Step 2 and 3 and will be handled by Selectable()/TreeNode on a per-item basis. @@ -2799,12 +2794,11 @@ struct ImGuiMultiSelectIO bool RequestSetRange; // / / ms:w, app:r // 3. Request app/user to select/unselect [RangeFirstItem..RangeLastItem] items, based on RangeSelected. Only EndMultiSelect() request this, app code can read after BeginMultiSelect() and it will always be false. ImGuiSelectionUserData RequestFocusItem; // app:w / app:r / app:r // (If using deletion) 4. Request user to focus item. This is actually only manipulated in user-space, but we provide storage to facilitate implementing a deletion idiom (see demo). // STATE/ARGUMENTS -------------------------// BEGIN / LOOP / END - ImGuiSelectionUserData RangeSrcItem; // ms:w / app:r / // (If using clipper) Begin: Source item (generally the first selected item when multi-selecting, which is used as a reference point). Read during loop in order for user code to set RangeSrcPassedBy. + ImGuiSelectionUserData RangeSrcItem; // ms:w app:r / / // (If using clipper) Begin: Source item (generally the first selected item when multi-selecting, which is used as a reference point) must never be cliped! ImGuiSelectionUserData RangeFirstItem; // / / ms:w, app:r // End: parameter for RequestSetRange request (this is generally == RangeSrcItem when shift selecting from top to bottom) ImGuiSelectionUserData RangeLastItem; // / / ms:w, app:r // End: parameter for RequestSetRange request (this is generally == RangeSrcItem when shift selecting from bottom to top) bool RangeSelected; // / / ms:w, app:r // End: parameter for RequestSetRange request. true = Select Range, false = Unselect Range. - bool RangeSrcPassedBy; // / ms:rw app:w / ms:r // (If using clipper) Need to be set by app/user if RangeSrcItem was passed by. Ignore if not clipping. - bool RangeSrcReset; // app:w / app:w / ms:r // (If using deletion) Set before EndMultiSelect() to reset ResetSrcItem (e.g. if deleted selection). + bool RangeSrcReset; // app:w / / ms:r // (If using deletion) Set before EndMultiSelect() to reset ResetSrcItem (e.g. if deleted selection). bool NavIdSelected; // ms:w, app:r / / // (If using deletion) Last known selection state for NavId (if part of submitted items). ImGuiSelectionUserData NavIdItem; // ms:w, app:r / / // (If using deletion) Last known SetNextItemSelectionUserData() value for NavId (if part of submitted items). diff --git a/imgui_demo.cpp b/imgui_demo.cpp index 3cf95e646..5b889a271 100644 --- a/imgui_demo.cpp +++ b/imgui_demo.cpp @@ -2978,10 +2978,10 @@ static void ShowDemoWindowMultiSelect() ImGuiListClipper clipper; clipper.Begin(ITEMS_COUNT); + if (ms_io->RangeSrcItem > 0) + clipper.IncludeItemByIndex((int)ms_io->RangeSrcItem); // Ensure RangeSrc item is not clipped. while (clipper.Step()) { - if (!ms_io->RangeSrcPassedBy && clipper.DisplayStart > ms_io->RangeSrcItem) - ms_io->RangeSrcPassedBy = true; for (int n = clipper.DisplayStart; n < clipper.DisplayEnd; n++) { char label[64]; @@ -2991,8 +2991,6 @@ static void ShowDemoWindowMultiSelect() ImGui::Selectable(label, item_is_selected); } } - if (!ms_io->RangeSrcPassedBy && ITEMS_COUNT > ms_io->RangeSrcItem) - ms_io->RangeSrcPassedBy = true; ms_io = ImGui::EndMultiSelect(); selection.ApplyRequests(ms_io, ITEMS_COUNT); @@ -3185,16 +3183,13 @@ static void ShowDemoWindowMultiSelect() { clipper.Begin(items.Size); if (next_focus_item_idx != -1) - clipper.IncludeItemByIndex(next_focus_item_idx); // Ensure item to focus is not clipped + clipper.IncludeItemByIndex(next_focus_item_idx); // Ensure focused item is not clipped + if (ms_io->RangeSrcItem > 0) + clipper.IncludeItemByIndex((int)ms_io->RangeSrcItem); // Ensure RangeSrc item is not clipped. } while (!use_clipper || clipper.Step()) { - // IF clipping is used: you need to set 'RangeSrcPassedBy = true' if RangeSrc was passed over. - // If you submit all items this is unnecessary as this is one by SetNextItemSelectionUserData() - if (use_clipper && !ms_io->RangeSrcPassedBy && clipper.DisplayStart > ms_io->RangeSrcItem) - ms_io->RangeSrcPassedBy = true; - const int item_begin = use_clipper ? clipper.DisplayStart : 0; const int item_end = use_clipper ? clipper.DisplayEnd : items.Size; for (int n = item_begin; n < item_end; n++) @@ -3281,12 +3276,6 @@ static void ShowDemoWindowMultiSelect() break; } - // If clipping is used: you need to set 'RangeSrcPassedBy = true' if RangeSrc was passed over. - // If you submit all items this is unnecessary as this is one by SetNextItemSelectionUserData() - // Here we essentially notify before EndMultiSelect() that RangeSrc is still present in our data set. - if (use_clipper && !ms_io->RangeSrcPassedBy && items.Size > ms_io->RangeSrcItem) - ms_io->RangeSrcPassedBy = true; - if (show_in_table) { ImGui::EndTable(); diff --git a/imgui_internal.h b/imgui_internal.h index 3bf729e3b..2e2999a92 100644 --- a/imgui_internal.h +++ b/imgui_internal.h @@ -1725,7 +1725,8 @@ struct IMGUI_API ImGuiMultiSelectTempData 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 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 NavIdPassedBy; - bool RangeDstPassedBy; // Set by the the item that matches NavJustMovedToId when IsSetRange is set. + bool RangeSrcPassedBy; // Set by the item that matches RangeSrcItem. + bool RangeDstPassedBy; // Set by the item that matches NavJustMovedToId when IsSetRange is set. //ImRect Rect; // Extent of selection scope between BeginMultiSelect() / EndMultiSelect(), used by ImGuiMultiSelectFlags_ClearOnClickRectVoid. ImGuiMultiSelectTempData() { Clear(); } diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index 178f0991f..1930fd790 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -7213,7 +7213,7 @@ ImGuiMultiSelectIO* ImGui::EndMultiSelect() if (ms->IsFocused) { - if (ms->BeginIO.RangeSrcReset || (ms->BeginIO.RangeSrcPassedBy == false && ms->BeginIO.RangeSrcItem != ImGuiSelectionUserData_Invalid)) + if (ms->BeginIO.RangeSrcReset || (ms->RangeSrcPassedBy == false && ms->BeginIO.RangeSrcItem != ImGuiSelectionUserData_Invalid)) { IMGUI_DEBUG_LOG_SELECTION("[selection] EndMultiSelect: Reset RangeSrcItem.\n"); // Will set be to NavId. ms->Storage->RangeSrcItem = ImGuiSelectionUserData_Invalid; @@ -7262,7 +7262,7 @@ void ImGui::SetNextItemSelectionUserData(ImGuiSelectionUserData selection_user_d // Auto updating RangeSrcPassedBy for cases were clipper is not used (done before ItemAdd() clipping) g.NextItemData.ItemFlags |= ImGuiItemFlags_HasSelectionUserData | ImGuiItemFlags_IsMultiSelect; if (ms->BeginIO.RangeSrcItem == selection_user_data) - ms->BeginIO.RangeSrcPassedBy = true; + ms->RangeSrcPassedBy = true; } else { @@ -7291,7 +7291,7 @@ void ImGui::MultiSelectItemHeader(ImGuiID id, bool* p_selected) 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. + // For this to work, we need someone to set 'RangeSrcPassedBy = true' at some point (either clipper either SetNextItemSelectionUserData() function) if (ms->IsSetRange) { IM_ASSERT(id != 0 && (ms->KeyMods & ImGuiMod_Shift) != 0); @@ -7306,7 +7306,7 @@ void ImGui::MultiSelectItemHeader(ImGuiID id, bool* p_selected) } } const bool is_range_src = storage->RangeSrcItem == item_data; - if (is_range_src || is_range_dst || ms->BeginIO.RangeSrcPassedBy != ms->RangeDstPassedBy) + if (is_range_src || is_range_dst || ms->RangeSrcPassedBy != ms->RangeDstPassedBy) { IM_ASSERT(storage->RangeSrcItem != ImGuiSelectionUserData_Invalid && storage->RangeSelected != -1); selected = (storage->RangeSelected != 0); @@ -7406,7 +7406,7 @@ void ImGui::MultiSelectItemFooter(ImGuiID id, bool* p_selected, bool* p_pressed) //IM_ASSERT(storage->HasRangeSrc && storage->HasRangeValue); ms->EndIO.RangeSrcItem = (storage->RangeSrcItem != ImGuiSelectionUserData_Invalid) ? storage->RangeSrcItem : item_data; ms->EndIO.RangeSelected = (is_ctrl && storage->RangeSelected != -1) ? (storage->RangeSelected != 0) : true; - range_direction = ms->BeginIO.RangeSrcPassedBy ? +1 : -1; + range_direction = ms->RangeSrcPassedBy ? +1 : -1; } else {