Merge branch 'grid-col-row-sel'

Fix returning duplicates from Get{Row,Col}Selection(); add more tests.

See https://github.com/wxWidgets/wxWidgets/pull/1874

include/wx/generic/grid.h

@@ -864,6 +864,8 @@ private:
     int m_rightCol;
 };
 
+typedef wxVector<wxGridBlockCoords> 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<wxGridBlockCoords>::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;

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); }

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.
 

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)

src/generic/gridsel.cpp

@@ -40,7 +40,7 @@ int CompareInts(int n1, int n2)
 
 }
 
-WX_DEFINE_SORTED_ARRAY(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 )
     {
@@ -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);
             }
         }
     }
@@ -769,7 +770,7 @@ wxArrayInt wxGridSelection::GetColSelection() const
     if ( m_selectionMode == wxGrid::wxGridSelectRows )
         return wxArrayInt();
 
-    wxIntSortedArray uniqueRows(&CompareInts);
+    wxIntSortedArray uniqueCols;
     const size_t count = m_selection.size();
     for ( size_t n = 0; n < count; ++n )
     {
@@ -779,16 +780,17 @@ wxArrayInt wxGridSelection::GetColSelection() const
         {
             for ( int c = block.GetLeftCol(); c <= block.GetRightCol(); ++c )
             {
-                uniqueRows.Add(c);
+                if ( uniqueCols.Index(c) == wxNOT_FOUND )
+                    uniqueCols.Add(c);
             }
         }
     }
 
     wxArrayInt result;
-    result.reserve(uniqueRows.size());
+    result.reserve(uniqueCols.size());
-    for( size_t i = 0; i < uniqueRows.size(); ++i )
+    for( size_t i = 0; i < uniqueCols.size(); ++i )
     {
-        result.push_back(uniqueRows[i]);
+        result.push_back(uniqueCols[i]);
     }
     return result;
 }

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<RowRange> 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);
@@ -901,6 +925,47 @@ 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->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);
 
 
@@ -909,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);
 }