From e82b49d2d468f9d26dbd4f7ad500016ebf4e2943 Mon Sep 17 00:00:00 2001 From: ocornut Date: Mon, 7 Aug 2023 12:38:24 +0200 Subject: [PATCH] MultiSelect: (Breaking) Use ImGuiSelectionUserData (= ImS64) instead of void* for selection user data. Less confusing for most users, less casting. --- imgui.h | 38 +++++++++++++++++++++++--------------- imgui_demo.cpp | 24 ++++++++++++------------ imgui_internal.h | 6 +++--- imgui_widgets.cpp | 30 +++++++++++++++--------------- 4 files changed, 53 insertions(+), 45 deletions(-) diff --git a/imgui.h b/imgui.h index 811bf940e..4d6ecaf0b 100644 --- a/imgui.h +++ b/imgui.h @@ -265,8 +265,9 @@ typedef ImWchar32 ImWchar; typedef ImWchar16 ImWchar; #endif -// Multi-Selection item index or identifier when using SetNextItemSelectionUserData()/BeginMultiSelect() -// (Most users are likely to use this store an item INDEX but this may be used to store a POINTER as well.) +// Multi-Selection item index or identifier when using BeginMultiSelect() +// - Used by SetNextItemSelectionUserData() + and inside ImGuiMultiSelectIO structure. +// - Most users are likely to use this store an item INDEX but this may be used to store a POINTER as well. Read comments near ImGuiMultiSelectIO for details. typedef ImS64 ImGuiSelectionUserData; // Callback and functions types @@ -670,7 +671,7 @@ namespace ImGui // Multi-selection system for Selectable() and TreeNode() functions. // - This enables standard multi-selection/range-selection idioms (CTRL+Mouse/Keyboard, SHIFT+Mouse/Keyboard, etc.) in a way that also allow a clipper to be used. - // - Read comments near ImGuiMultiSelectIO for details. + // - ImGuiSelectionUserData is often used to store your item index. Read comments near ImGuiMultiSelectIO for details. IMGUI_API ImGuiMultiSelectIO* BeginMultiSelect(ImGuiMultiSelectFlags flags); IMGUI_API ImGuiMultiSelectIO* EndMultiSelect(); IMGUI_API void SetNextItemSelectionUserData(ImGuiSelectionUserData selection_user_data); @@ -2754,18 +2755,25 @@ enum ImGuiMultiSelectFlags_ // For small selection set (<100 items), you might want to not bother with using the clipper, as the cost you should // be negligible (as least on Dear ImGui side). // If you are not sure, always start without clipping and you can work your way to the optimized version afterwards. -// - The void* RangeSrcItem/RangeDstItem value represent a selectable object. They are the value you pass to SetNextItemSelectionUserData(). -// Most likely you will want to store an index here. -// Storing an integer index is the easiest thing to do, as SetRange requests will give you two end points and you will need to interpolate -// between them to honor range selection. But the code never assume that sortable integers are used (you may store pointers to your object, -// and then from the pointer have your own way of iterating from RangeSrcItem to RangeDstItem). +// About ImGuiSelectionUserData: +// - This is your application-defined identifier in a selection set: +// - For each item is submitted by your calls to SetNextItemSelectionUserData(). +// - In return we store them into RangeSrcItem/RangeDstItem and other fields ImGuiMultiSelectIO. +// - Most applications will store an object INDEX, hence the chosen name and type. +// Storing an integer index is the easiest thing to do, as SetRange requests will give you two end points +// and you will need to interpolate between them to honor range selection. +// - However it is perfectly possible to store a POINTER inside this value! The multi-selection system never assume +// that you identify items by indices, and never attempt to interpolate between two ImGuiSelectionUserData values. +// - As most users will want to cast this to integer, for convenience and to reduce confusion we use ImS64 instead +// of void*, being syntactically easier to downcast. But feel free to reinterpret_cast a pointer into this. +// - If you need to wrap this API for another language/framework, feel free to expose this as 'int' if simpler. // 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 is part of the items clipped before the first submitted/visible item. // 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, this is easy to compute: if (clipper.DisplayStart > data->RangeSrcItem) { data->RangeSrcPassedBy = true; } -// - If you are using pointers, you may need additional processing in each clipper step to tell if current DisplayStart comes after RangeSrcItem.. +// - 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 in each clipper step to tell if current DisplayStart comes after RangeSrcItem.. // - (4) Submit your items with SetNextItemSelectionUserData() + Selectable()/TreeNode() calls. (optionally call IsItemToggledSelection() if you need that info immediately for displaying your item, before EndMultiSelect()) // 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. @@ -2780,19 +2788,19 @@ struct ImGuiMultiSelectIO bool RequestClear; // ms:w, app:r / / ms:w, app:r // 1. Request app/user to clear selection. bool RequestSelectAll; // ms:w, app:r / / ms:w, app:r // 2. Request app/user to select all. bool RequestSetRange; // / / ms:w, app:r // 3. Request app/user to select/unselect [RangeSrcItem..RangeDstItem] items, based on RangeSelected. In practice, only EndMultiSelect() request this, app code can read after BeginMultiSelect() and it will always be false. - void* 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). + 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 - void* RangeSrcItem; // ms:w / app:r / ms:w, app:r // Begin: Last known SetNextItemSelectionUserData() value for RangeSrcItem. End: parameter from RequestSetRange request. - void* RangeDstItem; // / / ms:w, app:r // End: parameter from RequestSetRange request. + ImGuiSelectionUserData RangeSrcItem; // ms:w / app:r / ms:w, app:r // Begin: Last known SetNextItemSelectionUserData() value for RangeSrcItem. End: parameter from RequestSetRange request. + ImGuiSelectionUserData RangeDstItem; // / / ms:w, app:r // End: parameter from RequestSetRange request. ImS8 RangeDirection; // / / ms:w, app:r // End: parameter from RequestSetRange request. +1 if RangeSrcItem came before RangeDstItem, -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 RangeSelected; // / / 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 clipper) Need to be set by app/user if RangeSrcItem was part of the clipped set before submitting the visible items. 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 NavIdSelected; // ms:w, app:r / / // (If using deletion) Last known selection state for NavId (if part of submitted items). - void* NavIdItem; // ms:w, app:r / / // (If using deletion) Last known SetNextItemSelectionUserData() value 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). ImGuiMultiSelectIO() { Clear(); } - void Clear() { memset(this, 0, sizeof(*this)); RequestFocusItem = NavIdItem = RangeSrcItem = RangeDstItem = (void*)-1; } + void Clear() { memset(this, 0, sizeof(*this)); RequestFocusItem = NavIdItem = RangeSrcItem = RangeDstItem = (ImGuiSelectionUserData)-1; } }; //----------------------------------------------------------------------------- diff --git a/imgui_demo.cpp b/imgui_demo.cpp index bd4618e32..af8604182 100644 --- a/imgui_demo.cpp +++ b/imgui_demo.cpp @@ -2802,7 +2802,7 @@ struct ExampleSelection { if (ms_io->RequestClear) { Clear(); } if (ms_io->RequestSelectAll) { SelectAll(items_count); } - if (ms_io->RequestSetRange) { SetRange((int)(intptr_t)ms_io->RangeSrcItem, (int)(intptr_t)ms_io->RangeDstItem, ms_io->RangeSelected ? 1 : 0); } + if (ms_io->RequestSetRange) { SetRange((int)ms_io->RangeSrcItem, (int)ms_io->RangeDstItem, ms_io->RangeSelected ? 1 : 0); } } void DebugTooltip() @@ -2827,19 +2827,19 @@ struct ExampleSelection QueueDeletion = false; // If current item is not selected. - if (ms_io->NavIdSelected == false) // Here 'NavIdSelected' should be == to 'GetSelected(ms_io->NavIdData)' + if (ms_io->NavIdSelected == false) // Here 'NavIdSelected' should be == to 'GetSelected(ms_io->NavIdData)' { - ms_io->RangeSrcReset = true; // Request to recover RangeSrc from NavId next frame. Would be ok to reset even without the !NavIdSelected test but it would take an extra frame to recover RangeSrc when deleting a selected item. - return (int)(intptr_t)ms_io->NavIdItem; // Request to land on same item after deletion. + ms_io->RangeSrcReset = true; // Request to recover RangeSrc from NavId next frame. Would be ok to reset even without the !NavIdSelected test but it would take an extra frame to recover RangeSrc when deleting a selected item. + return (int)ms_io->NavIdItem; // Request to land on same item after deletion. } // If current item is selected: land on first unselected item after RangeSrc. - for (int n = (int)(intptr_t)ms_io->RangeSrcItem + 1; n < items.Size; n++) + for (int n = (int)ms_io->RangeSrcItem + 1; n < items.Size; n++) if (!GetSelected(n)) return n; // If current item is selected: otherwise return last unselected item. - for (int n = IM_MIN((int)(intptr_t)ms_io->RangeSrcItem, items.Size) - 1; n >= 0; n--) + for (int n = IM_MIN((int)ms_io->RangeSrcItem, items.Size) - 1; n >= 0; n--) if (!GetSelected(n)) return n; @@ -2860,7 +2860,7 @@ struct ExampleSelection IM_UNUSED(ms_io); ImVector new_items; new_items.reserve(items.Size - SelectionSize); - int next_focus_idx_in_old_selection = (int)(intptr_t)ms_io->RequestFocusItem; + int next_focus_idx_in_old_selection = (int)ms_io->RequestFocusItem; int next_focus_idx_in_new_selection = -1; for (int n = 0; n < items.Size; n++) { @@ -3016,8 +3016,8 @@ static void ShowDemoWindowMultiSelect() // FIXME-MULTISELECT: If pressing Delete + another key we have ambiguous behavior. const bool want_delete = (selection.GetSize() > 0) && ImGui::IsWindowFocused() && ImGui::IsKeyPressed(ImGuiKey_Delete); if (want_delete) - ms_io->RequestFocusItem = (void*)(intptr_t)selection.ApplyDeletionPreLoop(ms_io, items); - const int next_focus_item_idx = (int)(intptr_t)ms_io->RequestFocusItem; + ms_io->RequestFocusItem = selection.ApplyDeletionPreLoop(ms_io, items); + const int next_focus_item_idx = (int)ms_io->RequestFocusItem; for (int n = 0; n < items.Size; n++) { @@ -3138,7 +3138,7 @@ static void ShowDemoWindowMultiSelect() const bool want_delete = selection.QueueDeletion || ((selection.GetSize() > 0) && ImGui::IsWindowFocused() && ImGui::IsKeyPressed(ImGuiKey_Delete)); if (want_delete) selection.ApplyDeletionPreLoop(ms_io, items); - const int next_focus_item_idx = (int)(intptr_t)ms_io->RequestFocusItem; + const int next_focus_item_idx = (int)ms_io->RequestFocusItem; if (show_in_table) { @@ -3162,7 +3162,7 @@ static void ShowDemoWindowMultiSelect() { // 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 && clipper.DisplayStart > (int)(intptr_t)ms_io->RangeSrcItem) + if (use_clipper && !ms_io->RangeSrcPassedBy && clipper.DisplayStart > ms_io->RangeSrcItem) ms_io->RangeSrcPassedBy = true; const int item_begin = use_clipper ? clipper.DisplayStart : 0; @@ -3254,7 +3254,7 @@ static void ShowDemoWindowMultiSelect() // 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 && items.Size > (int)(intptr_t)ms_io->RangeSrcItem) + if (use_clipper && !ms_io->RangeSrcPassedBy && items.Size > ms_io->RangeSrcItem) ms_io->RangeSrcPassedBy = true; if (show_in_table) diff --git a/imgui_internal.h b/imgui_internal.h index b076368e5..ce80d94be 100644 --- a/imgui_internal.h +++ b/imgui_internal.h @@ -1741,11 +1741,11 @@ struct IMGUI_API ImGuiMultiSelectState int LastFrameActive; // Last used frame-count, for GC. ImS8 RangeSelected; // -1 (don't have) or true/false ImS8 NavIdSelected; // -1 (don't have) or true/false - void* RangeSrcItem; // - void* NavIdItem; // SetNextItemSelectionUserData() value for NavId (if part of submitted items) + ImGuiSelectionUserData RangeSrcItem; // + ImGuiSelectionUserData NavIdItem; // SetNextItemSelectionUserData() value for NavId (if part of submitted items) ImGuiMultiSelectState() { Init(0); } - void Init(ImGuiID id) { Window = NULL; ID = id; LastFrameActive = 0; RangeSelected = NavIdSelected = -1; RangeSrcItem = NavIdItem = (void*)-1; } + void Init(ImGuiID id) { Window = NULL; ID = id; LastFrameActive = 0; RangeSelected = NavIdSelected = -1; RangeSrcItem = NavIdItem = ImGuiSelectionUserData_Invalid; } }; #endif // #ifdef IMGUI_HAS_MULTI_SELECT diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index 9dacb010d..e62a8e100 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -7128,7 +7128,7 @@ static void DebugLogMultiSelectRequests(const char* function, const ImGuiMultiSe ImGuiContext& g = *GImGui; 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->RangeSrcItem, data->RangeDstItem, data->RangeSelected, data->RangeDirection); + if (data->RequestSetRange) IMGUI_DEBUG_LOG_SELECTION("[selection] %s: RequestSetRange %p..%p = %d (dir %+d)\n", function, (void*)data->RangeSrcItem, (void*)data->RangeDstItem, data->RangeSelected, data->RangeDirection); } // Return ImGuiMultiSelectIO structure. Lifetime: valid until corresponding call to EndMultiSelect(). @@ -7174,7 +7174,7 @@ ImGuiMultiSelectIO* ImGui::BeginMultiSelect(ImGuiMultiSelectFlags flags) if (ms->KeyMods & ImGuiMod_Shift) ms->IsSetRange = true; if (ms->IsSetRange) - IM_ASSERT(storage->RangeSrcItem != (void*)-1); // Not ready -> could clear? + IM_ASSERT(storage->RangeSrcItem != ImGuiSelectionUserData_Invalid); // Not ready -> could clear? if ((ms->KeyMods & (ImGuiMod_Ctrl | ImGuiMod_Shift)) == 0) ms->BeginIO.RequestClear = true; } @@ -7207,15 +7207,15 @@ ImGuiMultiSelectIO* ImGui::EndMultiSelect() if (ms->IsFocused) { - if (ms->BeginIO.RangeSrcReset || (ms->BeginIO.RangeSrcPassedBy == false && ms->BeginIO.RangeSrcItem != (void*)-1)) + if (ms->BeginIO.RangeSrcReset || (ms->BeginIO.RangeSrcPassedBy == false && ms->BeginIO.RangeSrcItem != ImGuiSelectionUserData_Invalid)) { IMGUI_DEBUG_LOG_SELECTION("[selection] EndMultiSelect: Reset RangeSrcItem.\n"); // Will set be to NavId. - ms->Storage->RangeSrcItem = (void*)-1; + ms->Storage->RangeSrcItem = ImGuiSelectionUserData_Invalid; } - if (ms->NavIdPassedBy == false && ms->Storage->NavIdItem != (void*)-1) + if (ms->NavIdPassedBy == false && ms->Storage->NavIdItem != ImGuiSelectionUserData_Invalid) { IMGUI_DEBUG_LOG_SELECTION("[selection] EndMultiSelect: Reset NavIdItem.\n"); - ms->Storage->NavIdItem = (void*)-1; + ms->Storage->NavIdItem = ImGuiSelectionUserData_Invalid; ms->Storage->NavIdSelected = -1; } } @@ -7255,7 +7255,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 == (void*)selection_user_data) + if (ms->BeginIO.RangeSrcItem == selection_user_data) ms->BeginIO.RangeSrcPassedBy = true; } else @@ -7273,7 +7273,7 @@ void ImGui::MultiSelectItemHeader(ImGuiID id, bool* p_selected) ImGuiMultiSelectState* storage = ms->Storage; IM_ASSERT(g.NextItemData.FocusScopeId == g.CurrentFocusScopeId && "Forgot to call SetNextItemSelectionUserData() prior to item, required in BeginMultiSelect()/EndMultiSelect() scope"); - void* item_data = (void*)g.NextItemData.SelectionUserData; + ImGuiSelectionUserData item_data = g.NextItemData.SelectionUserData; // Apply Clear/SelectAll requests requested by BeginMultiSelect(). // This is only useful if the user hasn't processed them already, and this only works if the user isn't using the clipper. @@ -7293,7 +7293,7 @@ void ImGui::MultiSelectItemHeader(ImGuiID id, bool* p_selected) if (is_range_dst) { ms->RangeDstPassedBy = true; - if (storage->RangeSrcItem == (void*)-1) // If we don't have RangeSrc, assign RangeSrc = RangeDst + if (storage->RangeSrcItem == ImGuiSelectionUserData_Invalid) // If we don't have RangeSrc, assign RangeSrc = RangeDst { storage->RangeSrcItem = item_data; storage->RangeSelected = selected ? 1 : 0; @@ -7302,7 +7302,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) { - IM_ASSERT(storage->RangeSrcItem != (void*)-1 && storage->RangeSelected != -1); + IM_ASSERT(storage->RangeSrcItem != ImGuiSelectionUserData_Invalid && storage->RangeSelected != -1); selected = (storage->RangeSelected != 0); } else if ((ms->KeyMods & ImGuiMod_Ctrl) == 0) @@ -7330,13 +7330,13 @@ void ImGui::MultiSelectItemFooter(ImGuiID id, bool* p_selected, bool* p_pressed) if (!ms->IsFocused && !hovered) return; - void* item_data = (void*)g.NextItemData.SelectionUserData; + ImGuiSelectionUserData item_data = g.NextItemData.SelectionUserData; const bool is_multiselect = (ms->Flags & ImGuiMultiSelectFlags_SingleSelect) == 0; bool is_ctrl = (ms->KeyMods & ImGuiMod_Ctrl) != 0; bool is_shift = (ms->KeyMods & ImGuiMod_Shift) != 0; - if (g.NavId == id && storage->RangeSrcItem == (void*)-1) + if (g.NavId == id && storage->RangeSrcItem == ImGuiSelectionUserData_Invalid) { storage->RangeSrcItem = item_data; storage->RangeSelected = selected; // Will be updated at the end of this function anyway. @@ -7398,7 +7398,7 @@ void ImGui::MultiSelectItemFooter(ImGuiID id, bool* p_selected, bool* p_pressed) // Shift+Arrow always select // Ctrl+Shift+Arrow copy source selection state (alrady stored by BeginMultiSelect() in RangeSelected) //IM_ASSERT(storage->HasRangeSrc && storage->HasRangeValue); - ms->EndIO.RangeSrcItem = (storage->RangeSrcItem != (void*)-1) ? storage->RangeSrcItem : item_data; + ms->EndIO.RangeSrcItem = (storage->RangeSrcItem != ImGuiSelectionUserData_Invalid) ? storage->RangeSrcItem : item_data; ms->EndIO.RangeSelected = (is_ctrl && storage->RangeSelected != -1) ? (storage->RangeSelected != 0) : true; ms->EndIO.RangeDirection = ms->BeginIO.RangeSrcPassedBy ? +1 : -1; } @@ -7459,8 +7459,8 @@ void ImGui::DebugNodeMultiSelectState(ImGuiMultiSelectState* storage) if (!is_active) { PopStyleColor(); } if (!open) return; - Text("RangeSrcItem = %p, RangeSelected = %d", storage->RangeSrcItem, storage->RangeSelected); - Text("NavIdData = %p, NavIdSelected = %d", storage->NavIdItem, storage->NavIdSelected); + Text("RangeSrcItem = %p, RangeSelected = %d", (void*)storage->RangeSrcItem, storage->RangeSelected); + Text("NavIdData = %p, NavIdSelected = %d", (void*)storage->NavIdItem, storage->NavIdSelected); TreePop(); #else IM_UNUSED(storage);