From f268c02c199f8979fbd195eb2ab4cf3761e02a65 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 26 May 2020 16:31:34 +0200 Subject: [PATCH 1/5] Use "uniqueCols" for an array containing column indices No real changes, just don't use misleading "Rows" in the name of a variable containing column indices. --- src/generic/gridsel.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/generic/gridsel.cpp b/src/generic/gridsel.cpp index 83b39e1fb3..fb478b339d 100644 --- a/src/generic/gridsel.cpp +++ b/src/generic/gridsel.cpp @@ -769,7 +769,7 @@ wxArrayInt wxGridSelection::GetColSelection() const if ( m_selectionMode == wxGrid::wxGridSelectRows ) return wxArrayInt(); - wxIntSortedArray uniqueRows(&CompareInts); + wxIntSortedArray uniqueCols(&CompareInts); const size_t count = m_selection.size(); for ( size_t n = 0; n < count; ++n ) { @@ -779,16 +779,16 @@ wxArrayInt wxGridSelection::GetColSelection() const { for ( int c = block.GetLeftCol(); c <= block.GetRightCol(); ++c ) { - uniqueRows.Add(c); + uniqueCols.Add(c); } } } wxArrayInt result; - result.reserve(uniqueRows.size()); - for( size_t i = 0; i < uniqueRows.size(); ++i ) + result.reserve(uniqueCols.size()); + for( size_t i = 0; i < uniqueCols.size(); ++i ) { - result.push_back(uniqueRows[i]); + result.push_back(uniqueCols[i]); } return result; } From 92b6a55fd6c63c1e57c0680502568e2e3481a056 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 26 May 2020 16:32:05 +0200 Subject: [PATCH 2/5] Fix returning duplicates from Get{Row,Col}Selection() The "unique" rows/columns arrays used in the implementation of these functions were not unique at all, as we happily added duplicates of the existing items into them. Fix this by checking that a row/column is not already present before adding it. Add a (previously failing) unit test checking that this works correctly with overlapping selected blocks. --- src/generic/gridsel.cpp | 6 ++++-- tests/controls/gridtest.cpp | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/generic/gridsel.cpp b/src/generic/gridsel.cpp index fb478b339d..55de239323 100644 --- a/src/generic/gridsel.cpp +++ b/src/generic/gridsel.cpp @@ -749,7 +749,8 @@ wxArrayInt wxGridSelection::GetRowSelection() const { for ( int r = block.GetTopRow(); r <= block.GetBottomRow(); ++r ) { - uniqueRows.Add(r); + if ( uniqueRows.Index(r) == wxNOT_FOUND ) + uniqueRows.Add(r); } } } @@ -779,7 +780,8 @@ wxArrayInt wxGridSelection::GetColSelection() const { for ( int c = block.GetLeftCol(); c <= block.GetRightCol(); ++c ) { - uniqueCols.Add(c); + if ( uniqueCols.Index(c) == wxNOT_FOUND ) + uniqueCols.Add(c); } } } diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 3649985b36..34d4a925c5 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -901,6 +901,12 @@ TEST_CASE_METHOD(GridTestCase, "Grid::SelectionMode", "[grid]") CHECK(selectedRows.Count() == 1); CHECK(selectedRows[0] == 3); + // Check that overlapping selection blocks are handled correctly. + m_grid->ClearSelection(); + m_grid->SelectBlock(0, 0, 4, 1); + m_grid->SelectBlock(2, 0, 6, 1, true /* add to selection */); + CHECK( m_grid->GetSelectedRows().size() == 7 ); + CHECK(m_grid->GetSelectionMode() == wxGrid::wxGridSelectRows); From 340a86b3de8003a27a3e8eac8c80b3318f3c27da Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 26 May 2020 16:34:02 +0200 Subject: [PATCH 3/5] Use more efficient storage for wxIntSortedArray As this array contains ints, store them in wxVector instead of taking twice as much memory (in 64 bit builds) by storing them in wxVector. --- src/generic/gridsel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/generic/gridsel.cpp b/src/generic/gridsel.cpp index 55de239323..836ac1c7aa 100644 --- a/src/generic/gridsel.cpp +++ b/src/generic/gridsel.cpp @@ -40,7 +40,7 @@ int CompareInts(int n1, int n2) } -WX_DEFINE_SORTED_ARRAY(int, wxIntSortedArray); +WX_DEFINE_SORTED_ARRAY_INT(int, wxIntSortedArray); wxGridSelection::wxGridSelection( wxGrid * grid, From 445ccb23cc2cd4eecadfcfbcac79de083abeacae Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 26 May 2020 16:37:26 +0200 Subject: [PATCH 4/5] Simplify using wxIntSortedArray in wxGridSelection code There is no need to specify the comparison function when defining the variables of this type when we can just specify it once when defining the array type itself. No real changes. --- src/generic/gridsel.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/generic/gridsel.cpp b/src/generic/gridsel.cpp index 836ac1c7aa..eb5b9b3139 100644 --- a/src/generic/gridsel.cpp +++ b/src/generic/gridsel.cpp @@ -40,7 +40,7 @@ int CompareInts(int n1, int n2) } -WX_DEFINE_SORTED_ARRAY_INT(int, wxIntSortedArray); +WX_DEFINE_SORTED_ARRAY_CMP_INT(int, CompareInts, wxIntSortedArray); wxGridSelection::wxGridSelection( wxGrid * grid, @@ -739,7 +739,7 @@ wxArrayInt wxGridSelection::GetRowSelection() const if ( m_selectionMode == wxGrid::wxGridSelectColumns ) return wxArrayInt(); - wxIntSortedArray uniqueRows(&CompareInts); + wxIntSortedArray uniqueRows; const size_t count = m_selection.size(); for ( size_t n = 0; n < count; ++n ) { @@ -770,7 +770,7 @@ wxArrayInt wxGridSelection::GetColSelection() const if ( m_selectionMode == wxGrid::wxGridSelectRows ) return wxArrayInt(); - wxIntSortedArray uniqueCols(&CompareInts); + wxIntSortedArray uniqueCols; const size_t count = m_selection.size(); for ( size_t n = 0; n < count; ++n ) { From 3307000baad50a4ef79bae54bacf7eb34ee5fc7f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 27 May 2020 03:19:34 +0200 Subject: [PATCH 5/5] Add wxGrid::GetSelectedRowBlocks() and GetSelectedColBlocks() These functions are much simpler to use in the application code using wxGrid in row- or column-only selection mode than GetSelectedBlocks() itself because they take care of deduplicating, ordering and squashing together the adjacent ranges, so that the application can use their results directly, unlike with GetSelectedBlocks(). --- include/wx/generic/grid.h | 10 ++- include/wx/generic/private/grid.h | 22 +++++ interface/wx/grid.h | 40 +++++++++ src/generic/grid.cpp | 135 ++++++++++++++++++++++++++++++ tests/controls/gridtest.cpp | 65 ++++++++++++++ 5 files changed, 271 insertions(+), 1 deletion(-) diff --git a/include/wx/generic/grid.h b/include/wx/generic/grid.h index cdfd95fdae..5bc3c9dfff 100644 --- a/include/wx/generic/grid.h +++ b/include/wx/generic/grid.h @@ -864,6 +864,8 @@ private: int m_rightCol; }; +typedef wxVector wxGridBlockCoordsVector; + // ---------------------------------------------------------------------------- // wxGridBlockDiffResult: The helper struct uses as a result type for difference // functions of wxGridBlockCoords class. @@ -882,7 +884,7 @@ struct wxGridBlockDiffResult class wxGridBlocks { - typedef wxVector::const_iterator iterator_impl; + typedef wxGridBlockCoordsVector::const_iterator iterator_impl; public: class iterator @@ -1997,7 +1999,13 @@ public: bool IsInSelection( const wxGridCellCoords& coords ) const { return IsInSelection( coords.GetRow(), coords.GetCol() ); } + // Efficient methods returning the selected blocks (there are few of those). wxGridBlocks GetSelectedBlocks() const; + wxGridBlockCoordsVector GetSelectedRowBlocks() const; + wxGridBlockCoordsVector GetSelectedColBlocks() const; + + // Less efficient (but maybe more convenient methods) returning all + // selected cells, rows or columns -- there can be many and many of those. wxGridCellCoordsArray GetSelectedCells() const; wxGridCellCoordsArray GetSelectionBlockTopLeft() const; wxGridCellCoordsArray GetSelectionBlockBottomRight() const; diff --git a/include/wx/generic/private/grid.h b/include/wx/generic/private/grid.h index 1161b79337..709cf0c167 100644 --- a/include/wx/generic/private/grid.h +++ b/include/wx/generic/private/grid.h @@ -530,6 +530,12 @@ public: virtual int Select(const wxRect& r) const = 0; virtual int& Select(wxRect& r) const = 0; + // Return or set left/top or right/bottom component of a block. + virtual int SelectFirst(const wxGridBlockCoords& block) const = 0; + virtual int SelectLast(const wxGridBlockCoords& block) const = 0; + virtual void SetFirst(wxGridBlockCoords& block, int line) const = 0; + virtual void SetLast(wxGridBlockCoords& block, int line) const = 0; + // Returns width or height of the rectangle virtual int& SelectSize(wxRect& r) const = 0; @@ -642,6 +648,14 @@ public: virtual int Select(const wxSize& sz) const wxOVERRIDE { return sz.x; } virtual int Select(const wxRect& r) const wxOVERRIDE { return r.x; } virtual int& Select(wxRect& r) const wxOVERRIDE { return r.x; } + virtual int SelectFirst(const wxGridBlockCoords& block) const wxOVERRIDE + { return block.GetTopRow(); } + virtual int SelectLast(const wxGridBlockCoords& block) const wxOVERRIDE + { return block.GetBottomRow(); } + virtual void SetFirst(wxGridBlockCoords& block, int line) const wxOVERRIDE + { block.SetTopRow(line); } + virtual void SetLast(wxGridBlockCoords& block, int line) const wxOVERRIDE + { block.SetBottomRow(line); } virtual int& SelectSize(wxRect& r) const wxOVERRIDE { return r.width; } virtual wxSize MakeSize(int first, int second) const wxOVERRIDE { return wxSize(first, second); } @@ -715,6 +729,14 @@ public: virtual int Select(const wxSize& sz) const wxOVERRIDE { return sz.y; } virtual int Select(const wxRect& r) const wxOVERRIDE { return r.y; } virtual int& Select(wxRect& r) const wxOVERRIDE { return r.y; } + virtual int SelectFirst(const wxGridBlockCoords& block) const wxOVERRIDE + { return block.GetLeftCol(); } + virtual int SelectLast(const wxGridBlockCoords& block) const wxOVERRIDE + { return block.GetRightCol(); } + virtual void SetFirst(wxGridBlockCoords& block, int line) const wxOVERRIDE + { block.SetLeftCol(line); } + virtual void SetLast(wxGridBlockCoords& block, int line) const wxOVERRIDE + { block.SetRightCol(line); } virtual int& SelectSize(wxRect& r) const wxOVERRIDE { return r.height; } virtual wxSize MakeSize(int first, int second) const wxOVERRIDE { return wxSize(second, first); } diff --git a/interface/wx/grid.h b/interface/wx/grid.h index 0ac6074571..33edf24f5f 100644 --- a/interface/wx/grid.h +++ b/interface/wx/grid.h @@ -4675,10 +4675,50 @@ public: } @endcode + Notice that the blocks returned by this method are not ordered in any + particular way and may overlap. For grids using rows or columns-only + selection modes, GetSelectedRowBlocks() or GetSelectedColBlocks() can + be more convenient, as they return ordered and non-overlapping blocks. + @since 3.1.4 */ wxGridBlocks GetSelectedBlocks() const; + /** + Returns an ordered range of non-overlapping selected rows. + + For the grids using wxGridSelectRows selection mode, returns the + possibly empty vector containing the coordinates of non-overlapping + selected row blocks in the natural order, i.e. from smallest to the + biggest row indices. + + To see the difference between this method and GetSelectedBlocks(), + consider the case when the user selects rows 2..4 in the grid and then + also selects (using Ctrl/Shift keys) the rows 1..3. Iterating over the + result of GetSelectedBlocks() would yield two blocks directly + corresponding to the users selection, while this method returns a + vector with a single element corresponding to the rows 1..4. + + This method returns empty vector for the other selection modes. + + @see GetSelectedBlocks(), GetSelectedColBlocks() + + @since 3.1.4 + */ + wxGridBlockCoordsVector GetSelectedRowBlocks() const; + + /** + Returns an ordered range of non-overlapping selected columns. + + This method is symmetric to GetSelectedRowBlocks(), but is useful only + in wxGridSelectColumns selection mode. + + @see GetSelectedBlocks() + + @since 3.1.4 + */ + wxGridBlockCoordsVector GetSelectedColBlocks() const; + /** Returns an array of individually selected cells. diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index ee8b4adc5e..f6ed381ba2 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -10453,6 +10453,141 @@ wxGridBlocks wxGrid::GetSelectedBlocks() const return wxGridBlocks(blocks.begin(), blocks.end()); } +static +wxGridBlockCoordsVector +DoGetRowOrColBlocks(wxGridBlocks blocks, const wxGridOperations& oper) +{ + wxGridBlockCoordsVector res; + + for ( wxGridBlocks::iterator it = blocks.begin(); it != blocks.end(); ++it ) + { + const int firstNew = oper.SelectFirst(*it); + const int lastNew = oper.SelectLast(*it); + + // Check if this block intersects any of the existing ones. + // + // We use simple linear search because we assume there are only a few + // blocks in all, and it's not worth complicating the code to use + // anything more advanced, but this definitely could be improved to use + // the fact that the vector is always sorted. + for ( size_t n = 0;; ) + { + if ( n == res.size() ) + { + // We didn't find any overlapping blocks, so add this one to + // the end. + res.push_back(*it); + break; + } + + wxGridBlockCoords& block = res[n]; + const int firstThis = oper.SelectFirst(block); + const int lastThis = oper.SelectLast(block); + + if ( lastNew < firstThis ) + { + // Not only it doesn't overlap this block, but it won't overlap + // any subsequent ones neither, so insert it here and stop. + res.insert(res.begin() + n, *it); + break; + } + + if ( lastThis < firstNew ) + { + // It doesn't overlap this one, but continue checking. + n++; + continue; + } + + // The blocks overlap, combine them by adjusting the bounds of the + // current block. + + // The first bound is simple as we know that firstNew must be + // strictly greater than the last coordinate of all the previous + // elements, otherwise we would have combined it with them earlier. + if ( firstNew < firstThis ) + oper.SetFirst(block, firstNew); + + // But for the last one, we need to find the last element it + // overlaps (which may be this block itself). We call its index n2 + // to avoid confusion with "last" used for the block component. + size_t n2 = n; + for ( ;; ) + { + const wxGridBlockCoords& block2 = res[n2]; + if ( lastNew < oper.SelectFirst(block2) ) + { + oper.SetLast(block, lastNew); + break; + } + + // Do it here as we'll need to remove the current block if it's + // the last overlapping one and we break just below. + n2++; + + if ( lastNew < oper.SelectLast(block2) ) + { + oper.SetLast(block, oper.SelectLast(block2)); + break; + } + + if ( n2 == res.size() ) + { + oper.SetLast(block, lastNew); + break; + } + } + + if ( n2 > n + 1 ) + res.erase(res.begin() + n + 1, res.begin() + n2); + + break; + } + } + + // This is another inefficiency: it would be also possible to do everything + // in one pass, combining the adjacent ranges in the loop above. But this + // is more complicated and doesn't seem to be worth it, for the arrays of + // small sizes that we work with here, so do an extra path combining the + // adjacent ranges. + for ( size_t n = 0;; ) + { + if ( n + 1 >= res.size() ) + break; + + if ( oper.SelectFirst(res[n + 1]) == oper.SelectLast(res[n]) + 1 ) + { + // The ranges touch, combine them. + oper.SetLast(res[n], oper.SelectLast(res[n + 1])); + + // And erase the subsumed range. + res.erase(res.begin() + n + 1, res.begin() + n + 2); + } + else // Just go to the next one. + { + n++; + } + } + + return res; +} + +wxGridBlockCoordsVector wxGrid::GetSelectedRowBlocks() const +{ + if ( !m_selection || m_selection->GetSelectionMode() != wxGridSelectRows ) + return wxGridBlockCoordsVector(); + + return DoGetRowOrColBlocks(GetSelectedBlocks(), wxGridRowOperations()); +} + +wxGridBlockCoordsVector wxGrid::GetSelectedColBlocks() const +{ + if ( !m_selection || m_selection->GetSelectionMode() != wxGridSelectColumns ) + return wxGridBlockCoordsVector(); + + return DoGetRowOrColBlocks(GetSelectedBlocks(), wxGridColumnOperations()); +} + wxGridCellCoordsArray wxGrid::GetSelectedCells() const { if (!m_selection) diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 34d4a925c5..9429a76ac8 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -85,6 +85,30 @@ protected: CHECK( ++it == selected.end() ); } + // Or specified ranges. + struct RowRange + { + RowRange(int top, int bottom) : top(top), bottom(bottom) { } + + int top, bottom; + }; + + typedef wxVector RowRanges; + + void CheckRowSelection(const RowRanges& ranges) + { + const wxGridBlockCoordsVector sel = m_grid->GetSelectedRowBlocks(); + REQUIRE( sel.size() == ranges.size() ); + + for ( size_t n = 0; n < sel.size(); ++n ) + { + INFO("n = " << n); + + const RowRange& r = ranges[n]; + CHECK( sel[n] == wxGridBlockCoords(r.top, 0, r.bottom, 1) ); + } + } + TestableGrid *m_grid; wxDECLARE_NO_COPY_CLASS(GridTestCase); @@ -907,6 +931,41 @@ TEST_CASE_METHOD(GridTestCase, "Grid::SelectionMode", "[grid]") m_grid->SelectBlock(2, 0, 6, 1, true /* add to selection */); CHECK( m_grid->GetSelectedRows().size() == 7 ); + CHECK( m_grid->GetSelectedColBlocks().empty() ); + + RowRanges rowRanges; + rowRanges.push_back(RowRange(0, 6)); + CheckRowSelection(rowRanges); + + m_grid->SelectBlock(6, 0, 8, 1); + m_grid->SelectBlock(1, 0, 4, 1, true /* add to selection */); + m_grid->SelectBlock(0, 0, 2, 1, true /* add to selection */); + CHECK( m_grid->GetSelectedRows().size() == 8 ); + + rowRanges.clear(); + rowRanges.push_back(RowRange(0, 4)); + rowRanges.push_back(RowRange(6, 8)); + CheckRowSelection(rowRanges); + + // Select all odd rows. + m_grid->ClearSelection(); + rowRanges.clear(); + for ( int i = 1; i < m_grid->GetNumberRows(); i += 2 ) + { + m_grid->SelectBlock(i, 0, i, 1, true); + rowRanges.push_back(RowRange(i, i)); + } + + CheckRowSelection(rowRanges); + + // Now select another block overlapping 2 of them and bordering 2 others. + m_grid->SelectBlock(2, 0, 6, 1, true); + + rowRanges.clear(); + rowRanges.push_back(RowRange(1, 7)); + rowRanges.push_back(RowRange(9, 9)); + CheckRowSelection(rowRanges); + CHECK(m_grid->GetSelectionMode() == wxGrid::wxGridSelectRows); @@ -915,10 +974,16 @@ TEST_CASE_METHOD(GridTestCase, "Grid::SelectionMode", "[grid]") m_grid->SetSelectionMode(wxGrid::wxGridSelectColumns); m_grid->SelectBlock(3, 1, 3, 1); + CHECK( m_grid->GetSelectedRowBlocks().empty() ); + wxArrayInt selectedCols = m_grid->GetSelectedCols(); CHECK(selectedCols.Count() == 1); CHECK(selectedCols[0] == 1); + wxGridBlockCoordsVector colBlocks = m_grid->GetSelectedColBlocks(); + CHECK( colBlocks.size() == 1 ); + CHECK( colBlocks.at(0) == wxGridBlockCoords(0, 1, 9, 1) ); + CHECK(m_grid->GetSelectionMode() == wxGrid::wxGridSelectColumns); }