Improve responsiveness of a wxGrid having plenty of attributes

Speed up grid attribute lookups by using a hash map instead of array.

Closes #12764.

include/wx/generic/private/grid.h

@@ -71,8 +71,9 @@ struct wxGridCellWithAttr
     wxGridCellAttr  *attr;
 };
 
-WX_DECLARE_OBJARRAY_WITH_DECL(wxGridCellWithAttr, wxGridCellWithAttrArray,
+WX_DECLARE_HASH_MAP_WITH_DECL(wxLongLong_t, wxGridCellWithAttr*,
-                              class WXDLLIMPEXP_ADV);
+                              wxIntegerHash, wxIntegerEqual,
+                              wxGridCoordsToAttrMap, class WXDLLIMPEXP_CORE);
 
 
 // ----------------------------------------------------------------------------
@@ -468,16 +469,18 @@ private:
 class WXDLLIMPEXP_ADV wxGridCellAttrData
 {
 public:
+    ~wxGridCellAttrData();
+
     void SetAttr(wxGridCellAttr *attr, int row, int col);
     wxGridCellAttr *GetAttr(int row, int col) const;
     void UpdateAttrRows( size_t pos, int numRows );
     void UpdateAttrCols( size_t pos, int numCols );
 
 private:
-    // searches for the attr for given cell, returns wxNOT_FOUND if not found
+    // Tries to search for the attr for given cell.
-    int FindIndex(int row, int col) const;
+    wxGridCoordsToAttrMap::iterator FindIndex(int row, int col) const;
 
-    wxGridCellWithAttrArray m_attrs;
+    mutable wxGridCoordsToAttrMap m_attrs;
 };
 
 // this class stores attributes set for rows or columns

src/generic/grid.cpp

@@ -124,7 +124,6 @@ const int GRID_TEXT_MARGIN = 1;
 #include "wx/arrimpl.cpp"
 
 WX_DEFINE_OBJARRAY(wxGridCellCoordsArray)
-WX_DEFINE_OBJARRAY(wxGridCellWithAttrArray)
 
 // ----------------------------------------------------------------------------
 // events
@@ -762,18 +761,51 @@ wxGridCellEditor* wxGridCellAttr::GetEditor(const wxGrid* grid, int row, int col
 // wxGridCellAttrData
 // ----------------------------------------------------------------------------
 
+namespace
+{
+
+// Helper functions to convert grid coords to a key for the attr map, and
+// vice versa.
+
+wxGridCoordsToAttrMap::key_type CoordsToKey(int row, int col)
+{
+    // Treat both row and col as unsigned to not cause havoc with (unsupported)
+    // negative coords.
+    return (static_cast<wxULongLong_t>(row) << 32) + static_cast<wxUint32>(col);
+}
+
+void KeyToCoords(wxGridCoordsToAttrMap::key_type key, int *pRow, int *pCol)
+{
+    *pRow = key >> 32;
+    *pCol = key & wxUINT32_MAX;
+}
+
+} // anonymous namespace
+
+wxGridCellAttrData::~wxGridCellAttrData()
+{
+    for ( wxGridCoordsToAttrMap::iterator it = m_attrs.begin();
+          it != m_attrs.end();
+          ++it )
+    {
+        delete it->second;
+    }
+
+    m_attrs.clear();
+}
+
 void wxGridCellAttrData::SetAttr(wxGridCellAttr *attr, int row, int col)
 {
     // Note: contrary to wxGridRowOrColAttrData::SetAttr, we must not
     //       touch attribute's reference counting explicitly, since this
     //       is managed by class wxGridCellWithAttr
-    int n = FindIndex(row, col);
+    wxGridCoordsToAttrMap::iterator it = FindIndex(row, col);
-    if ( n == wxNOT_FOUND )
+    if ( it == m_attrs.end() )
     {
         if ( attr )
         {
             // add the attribute
-            m_attrs.Add(new wxGridCellWithAttr(row, col, attr));
+            m_attrs[CoordsToKey(row, col)] = new wxGridCellWithAttr(row, col, attr);
         }
         //else: nothing to do
     }
@@ -782,12 +814,13 @@ void wxGridCellAttrData::SetAttr(wxGridCellAttr *attr, int row, int col)
         if ( attr )
         {
             // change the attribute
-            m_attrs[(size_t)n].ChangeAttr(attr);
+            it->second->ChangeAttr(attr);
         }
         else
         {
             // remove this attribute
-            m_attrs.RemoveAt((size_t)n);
+            delete it->second;
+            m_attrs.erase(it);
         }
     }
 }
@@ -796,10 +829,10 @@ wxGridCellAttr *wxGridCellAttrData::GetAttr(int row, int col) const
 {
     wxGridCellAttr *attr = NULL;
 
-    int n = FindIndex(row, col);
+    wxGridCoordsToAttrMap::iterator it = FindIndex(row, col);
-    if ( n != wxNOT_FOUND )
+    if ( it != m_attrs.end() )
     {
-        attr = m_attrs[(size_t)n].attr;
+        attr = it->second->attr;
         attr->IncRef();
     }
 
@@ -809,7 +842,7 @@ wxGridCellAttr *wxGridCellAttrData::GetAttr(int row, int col) const
 namespace
 {
 
-void UpdateCellAttrRowsOrCols(wxGridCellWithAttrArray& attrs, int editPos,
+void UpdateCellAttrRowsOrCols(wxGridCoordsToAttrMap& attrs, int editPos,
                               int editRowCount, int editColCount)
 {
     wxASSERT( !editRowCount || !editColCount );
@@ -817,17 +850,33 @@ void UpdateCellAttrRowsOrCols(wxGridCellWithAttrArray& attrs, int editPos,
     const bool isEditingRows = (editRowCount != 0);
     const int editCount = (isEditingRows ? editRowCount : editColCount);
 
-    size_t count = attrs.GetCount();
+    // Copy updated attributes to a new map instead of attempting to edit attrs
-    for ( size_t n = 0; n < count; n++ )
+    // map in-place. This requires more memory but greatly simplifies the code
+    // and any attempt with in-place editing would likely also require a lot
+    // more attr lookups.
+    // The updated copy will contain an attrs map with, if applicable: adjusted
+    // coords and cell size, newly inserted attributes (for multicells), and
+    // without now deleted attributes.
+    wxGridCoordsToAttrMap newAttrs;
+
+    for ( wxGridCoordsToAttrMap::iterator it = attrs.begin();
+          it != attrs.end();
+          ++it )
     {
-        wxGridCellAttr* cellAttr = attrs[n].attr;
+        const wxGridCoordsToAttrMap::key_type oldCoords = it->first;
+        wxGridCellWithAttr* cellWithAttr = it->second;
+        wxGridCellAttr* cellAttr = cellWithAttr->attr;
+
         int cellRows, cellCols;
         cellAttr->GetSize(&cellRows, &cellCols);
 
-        wxGridCellCoords& coords = attrs[n].coords;
+        int cellRow, cellCol;
-        const wxCoord cellRow = coords.GetRow(),
+        KeyToCoords(oldCoords, &cellRow, &cellCol);
-                      cellCol = coords.GetCol(),
+
-                      cellPos = (isEditingRows ? cellRow : cellCol);
+        wxGridCellCoords& coords = cellWithAttr->coords;
+        wxASSERT(wxGridCellCoords(cellRow, cellCol) == coords);
+
+        const int cellPos = isEditingRows ? cellRow : cellCol;
 
         if ( cellPos < editPos )
         {
@@ -894,6 +943,9 @@ void UpdateCellAttrRowsOrCols(wxGridCellWithAttrArray& attrs, int editPos,
                 }
             }
 
+            // Set attribute at old/unmodified coords.
+            newAttrs[oldCoords] = cellWithAttr;
+
             continue;
         }
 
@@ -901,18 +953,20 @@ void UpdateCellAttrRowsOrCols(wxGridCellWithAttrArray& attrs, int editPos,
         {
             // This row/col is deleted and the cell doesn't exist any longer:
             // Remove the attribute.
-            attrs.RemoveAt(n);
+            delete cellWithAttr;
-            n--;
-            count--;
 
             continue;
         }
 
+        const wxGridCoordsToAttrMap::key_type newCoords
+            = CoordsToKey(cellRow + editRowCount, cellCol + editColCount);
+
         if ( GetCellSpan(cellRows, cellCols) != wxGrid::CellSpan_Inside )
         {
             // Rows/cols inserted or deleted (and this cell still exists):
             // Adjust cell coords.
             coords.Set(cellRow + editRowCount, cellCol + editColCount);
+            newAttrs[newCoords] = cellWithAttr;
 
             // Nothing more to do: cell is not an inside cell of a multicell.
             continue;
@@ -928,9 +982,7 @@ void UpdateCellAttrRowsOrCols(wxGridCellWithAttrArray& attrs, int editPos,
             // On a position that still exists after deletion but main cell
             // of multicell is within deletion range so the multicell is gone:
             // Remove the attribute.
-            attrs.RemoveAt(n);
+            delete cellWithAttr;
-            n--;
-            count--;
 
             continue;
         }
@@ -938,6 +990,7 @@ void UpdateCellAttrRowsOrCols(wxGridCellWithAttrArray& attrs, int editPos,
         // Rows/cols inserted or deleted (and this inside cell still exists):
         // Adjust (inside) cell coords.
         coords.Set(cellRow + editRowCount, cellCol + editColCount);
+        newAttrs[newCoords] = cellWithAttr;
 
         if ( mainPos >= editPos )
         {
@@ -962,15 +1015,17 @@ void UpdateCellAttrRowsOrCols(wxGridCellWithAttrArray& attrs, int editPos,
                 wxGridCellAttr* attr = new wxGridCellAttr;
                 attr->SetSize(cellRows - adjustRows, cellCols - adjustCols);
 
-                attrs.Add(new wxGridCellWithAttr(cellRow + adjustRows,
+                const int row = cellRow + adjustRows,
-                                                  cellCol + adjustCols,
+                          col = cellCol + adjustCols;
-                                                  attr));
+                newAttrs[CoordsToKey(row, col)] = new wxGridCellWithAttr(row, col, attr);
             }
         }
 
         // Let this inside cell's size point to the main cell of the multicell.
         cellAttr->SetSize(cellRows - editRowCount, cellCols - editColCount);
     }
+
+    attrs = newAttrs;
 }
 
 } // anonymous namespace
@@ -985,19 +1040,10 @@ void wxGridCellAttrData::UpdateAttrCols( size_t pos, int numCols )
     UpdateCellAttrRowsOrCols(m_attrs, static_cast<int>(pos), 0, numCols);
 }
 
-int wxGridCellAttrData::FindIndex(int row, int col) const
+wxGridCoordsToAttrMap::iterator
+wxGridCellAttrData::FindIndex(int row, int col) const
 {
-    size_t count = m_attrs.GetCount();
+    return m_attrs.find(CoordsToKey(row, col));
-    for ( size_t n = 0; n < count; n++ )
-    {
-        const wxGridCellCoords& coords = m_attrs[n].coords;
-        if ( (coords.GetRow() == row) && (coords.GetCol() == col) )
-        {
-            return n;
-        }
-    }
-
-    return wxNOT_FOUND;
 }
 
 // ----------------------------------------------------------------------------