MultiSelect: ImGuiSelectionBasicStorage: optimized for smaller insert…

…ion amounts in larger sets + fix caling batch select with same value.

imgui.cpp

@@ -2551,6 +2551,7 @@ ImGuiStoragePair* ImLowerBound(ImGuiStoragePair* in_begin, ImGuiStoragePair* in_
     return in_p;
 }
 
+IM_MSVC_RUNTIME_CHECKS_OFF
 static int IMGUI_CDECL PairComparerByID(const void* lhs, const void* rhs)
 {
     // We can't just do a subtraction because qsort uses signed integers and subtracting our ID doesn't play well with that.
@@ -2560,7 +2561,6 @@ static int IMGUI_CDECL PairComparerByID(const void* lhs, const void* rhs)
 }
 
 // For quicker full rebuild of a storage (instead of an incremental one), you may add all your contents and then sort once.
-IM_MSVC_RUNTIME_CHECKS_OFF
 void ImGuiStorage::BuildSortByKey()
 {
     ImQsort(Data.Data, (size_t)Data.Size, sizeof(ImGuiStoragePair), PairComparerByID);

imgui.h

@@ -2752,8 +2752,8 @@ enum ImGuiMultiSelectFlags_
     ImGuiMultiSelectFlags_NoAutoSelect          = 1 << 3,   // Disable selecting items when navigating (useful for e.g. supporting range-select in a list of checkboxes)
     ImGuiMultiSelectFlags_NoAutoClear           = 1 << 4,   // Disable clearing selection when navigating or selecting another one (generally used with ImGuiMultiSelectFlags_NoAutoSelect. useful for e.g. supporting range-select in a list of checkboxes)
     ImGuiMultiSelectFlags_NoAutoClearOnReselect = 1 << 5,   // Disable clearing selection when clicking/selecting an already selected item
-    ImGuiMultiSelectFlags_BoxSelect1d           = 1 << 6,   // Enable box-selection with same width and same x pos items (e.g. only full row Selectable()). Small optimization.
-    ImGuiMultiSelectFlags_BoxSelect2d           = 1 << 7,   // Enable box-selection with varying width or varying x pos items support (e.g. different width labels, or 2D layout/grid). This alters clipping logic so that e.g. horizontal movements will update selection of normally clipped items. Box-selection works better with little bit of spacing between items hit-box in order to be able to aim at empty space.
+    ImGuiMultiSelectFlags_BoxSelect1d           = 1 << 6,   // Enable box-selection with same width and same x pos items (e.g. only full row Selectable()). Box-selection works better with little bit of spacing between items hit-box in order to be able to aim at empty space.
+    ImGuiMultiSelectFlags_BoxSelect2d           = 1 << 7,   // Enable box-selection with varying width or varying x pos items support (e.g. different width labels, or 2D layout/grid). This is slower: alters clipping logic so that e.g. horizontal movements will update selection of normally clipped items.
     ImGuiMultiSelectFlags_BoxSelectNoScroll     = 1 << 8,   // Disable scrolling when box-selecting near edges of scope.
     ImGuiMultiSelectFlags_ClearOnEscape         = 1 << 9,   // Clear selection when pressing Escape while scope is focused.
     ImGuiMultiSelectFlags_ClearOnClickVoid      = 1 << 10,  // Clear selection when clicking on empty location within scope.

imgui_widgets.cpp

@@ -7875,7 +7875,7 @@ static void ImGuiSelectionBasicStorage_BatchSetItemSelected(ImGuiSelectionBasicS
     ImGuiStorage* storage = &selection->_Storage;
     ImGuiStoragePair* it = ImLowerBound(storage->Data.Data, storage->Data.Data + size_before_amends, id);
     const bool is_contained = (it != storage->Data.Data + size_before_amends) && (it->key == id);
-    if (selected == is_contained && it->val_i != 0)
+    if (selected == (is_contained && it->val_i != 0))
         return;
     if (selected && !is_contained)
         storage->Data.push_back(ImGuiStoragePair(id, selection_order)); // Push unsorted at end of vector, will be sorted in SelectionMultiAmendsFinish()
@@ -7915,10 +7915,15 @@ void ImGuiSelectionBasicStorage::ApplyRequests(ImGuiMultiSelectIO* ms_io)
     IM_ASSERT(ms_io->ItemsCount != -1 && "Missing value for items_count in BeginMultiSelect() call!");
     IM_ASSERT(AdapterIndexToStorageId != NULL);
 
-    // This is optimized/specialized to cope nicely with very large selections (e.g. 1 million items)
+    // This is optimized/specialized to cope with very large selections (e.g. 100k+ items)
     // - A simpler version could call SetItemSelected() directly instead of ImGuiSelectionBasicStorage_BatchSetItemSelected() + ImGuiSelectionBasicStorage_BatchFinish().
     // - Optimized select can append unsorted, then sort in a second pass. Optimized unselect can clear in-place then compact in a second pass.
-    // - (A more optimal version wouldn't even use ImGuiStorage but directly a ImVector<ImGuiID> to reduce bandwidth, but this is a reasonable trade off to reuse code)
+    // - A more optimal version wouldn't even use ImGuiStorage but directly a ImVector<ImGuiID> to reduce bandwidth, but this is a reasonable trade off to reuse code.
+    // - There are many ways this could be better optimized. The worse case scenario being: using BoxSelect2d in a grid, box-select scrolling down while wiggling
+    //   left and right: it affects coarse clipping + can emit multiple SetRange with 1 item each.)
+    // FIXME-OPT: For each block of consecutive SetRange request:
+    // - add all requests to a sorted list, store ID, selected, offset in ImGuiStorage.
+    // - rewrite sorted storage a single time.
     for (ImGuiSelectionRequest& req : ms_io->Requests)
     {
         if (req.Type == ImGuiSelectionRequestType_SetAll)
@@ -7935,13 +7940,27 @@ void ImGuiSelectionBasicStorage::ApplyRequests(ImGuiMultiSelectIO* ms_io)
         }
         else if (req.Type == ImGuiSelectionRequestType_SetRange)
         {
-            // Use req.RangeDirection to set order field so that shift+clicking from 1 to 5 is different than shift+clicking from 5 to 1
-            const int size_before_amends = _Storage.Data.Size;
-            int selection_order = _SelectionOrder + ((req.RangeDirection < 0) ? (int)req.RangeLastItem - (int)req.RangeFirstItem : 0);
-            for (int idx = (int)req.RangeFirstItem; idx <= (int)req.RangeLastItem; idx++, selection_order += req.RangeDirection)
-                ImGuiSelectionBasicStorage_BatchSetItemSelected(this, GetStorageIdFromIndex(idx), req.Selected, size_before_amends, selection_order);
-            _SelectionOrder += (int)req.RangeLastItem - (int)req.RangeFirstItem + 1;
-            ImGuiSelectionBasicStorage_BatchFinish(this, req.Selected, size_before_amends);
+            const int selection_changes = (int)req.RangeLastItem - (int)req.RangeFirstItem + 1;
+            //ImGuiContext& g = *GImGui; IMGUI_DEBUG_LOG_SELECTION("Req %d/%d: set %d to %d\n", ms_io->Requests.index_from_ptr(&req), ms_io->Requests.Size, selection_changes, req.Selected);
+            if (selection_changes == 1 || (selection_changes < Size / 100))
+            {
+                // Multiple sorted insertion + copy likely to be faster.
+                // Technically we could do a single copy with a little more work (sort sequential SetRange requests)
+                for (int idx = (int)req.RangeFirstItem; idx <= (int)req.RangeLastItem; idx++)
+                    SetItemSelected(GetStorageIdFromIndex(idx), req.Selected);
+            }
+            else
+            {
+                // Append insertion + single sort likely be faster.
+                // Use req.RangeDirection to set order field so that shift+clicking from 1 to 5 is different than shift+clicking from 5 to 1
+                const int size_before_amends = _Storage.Data.Size;
+                int selection_order = _SelectionOrder + ((req.RangeDirection < 0) ? selection_changes - 1 : 0);
+                for (int idx = (int)req.RangeFirstItem; idx <= (int)req.RangeLastItem; idx++, selection_order += req.RangeDirection)
+                    ImGuiSelectionBasicStorage_BatchSetItemSelected(this, GetStorageIdFromIndex(idx), req.Selected, size_before_amends, selection_order);
+                if (req.Selected)
+                    _SelectionOrder += selection_changes;
+                ImGuiSelectionBasicStorage_BatchFinish(this, req.Selected, size_before_amends);
+            }
         }
     }
 }