From bc7e5ffc56af400cbd98ef8ec5a2c151d835dc67 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Fri, 22 Jan 2021 22:21:11 +0100 Subject: [PATCH 1/7] Add basic tests for wxGrid cell attribute presence and count Do some simple sanity checks with attributes, particularly overwriting a cell with NULL attribute (as was already checked very usefully in the grid sample for ref count reasons), and their total count in a grid when inserting and deleting rows and columns. While the tests are not particularly useful for the next intended grid change, they do contain some functions that also work for upcoming tests so these (harmless) tests are included as well. --- tests/controls/gridtest.cpp | 112 +++++++++++++++++++++++++++++++++++- 1 file changed, 111 insertions(+), 1 deletion(-) diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 2b99364655..90b870b960 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -31,7 +31,7 @@ namespace { -// Derive a new class inheriting from wxGrid just to get access to its +// Derive a new class inheriting from wxGrid, also to get access to its // protected GetCellAttr(). This is not pretty, but we don't have any other way // of testing this function. class TestableGrid : public wxGrid @@ -46,6 +46,32 @@ public: { return GetCellAttr(row, col); } + + bool HasAttr(int row, int col, wxGridCellAttr::wxAttrKind kind) const + { + // Can't use GetCellAttr() here as it always returns an attr. + wxGridCellAttr* attr = GetTable()->GetAttr(row, col, kind); + if ( attr ) + attr->DecRef(); + + return attr != NULL; + } + + size_t GetCellAttrCount() const + { + // Note that only attributes in grid range can easily be checked + // and this function only counts those, not any outside of + // the grid (e.g. with invalid negative coords). + size_t count = 0; + for ( int row = 0; row < GetNumberRows(); ++row ) + { + for ( int col = 0; col < GetNumberCols(); ++col ) + count += HasAttr(row, col, wxGridCellAttr::Cell); + } + + return count; + } + }; } // anonymous namespace @@ -106,6 +132,16 @@ protected: } } + bool HasCellAttr(int row, int col) const + { + return m_grid->HasAttr(row, col, wxGridCellAttr::Cell); + } + + void SetCellAttr(int row, int col) + { + m_grid->SetAttr(row, col, new wxGridCellAttr); + } + TestableGrid *m_grid; wxDECLARE_NO_COPY_CLASS(GridTestCase); @@ -1446,6 +1482,80 @@ TEST_CASE_METHOD(GridTestCase, "Grid::DrawInvalidCell", "[grid][multicell]") wxYield(); } +#define CHECK_ATTR_COUNT(n) CHECK( m_grid->GetCellAttrCount() == n ) + +TEST_CASE_METHOD(GridTestCase, "Grid::CellAttribute", "[attr][cell][grid]") +{ + SECTION("Overwrite") + { + CHECK_ATTR_COUNT( 0 ); + + m_grid->SetAttr(0, 0, NULL); + CHECK_ATTR_COUNT( 0 ); + + SetCellAttr(0, 0); + CHECK_ATTR_COUNT( 1 ); + + m_grid->SetAttr(0, 0, NULL); + CHECK_ATTR_COUNT( 0 ); + + SetCellAttr(0, 0); + m_grid->SetCellBackgroundColour(0, 1, *wxGREEN); + CHECK_ATTR_COUNT( 2 ); + + m_grid->SetAttr(0, 1, NULL); + m_grid->SetAttr(0, 0, NULL); + CHECK_ATTR_COUNT( 0 ); + } + + + // Fill the grid with attributes for next sections. + + const int numRows = m_grid->GetNumberRows(); + const int numCols = m_grid->GetNumberCols(); + + for ( int row = 0; row < numRows; ++row ) + { + for ( int col = 0; col < numCols; ++col ) + SetCellAttr(row, col); + } + + size_t numAttrs = static_cast(numRows * numCols); + + CHECK_ATTR_COUNT( numAttrs ); + + SECTION("Expanding") + { + CHECK( !HasCellAttr(numRows, numCols) ); + + m_grid->InsertCols(); + CHECK_ATTR_COUNT( numAttrs ); + CHECK( !HasCellAttr(0, 0) ); + CHECK( HasCellAttr(0, numCols) ); + + m_grid->InsertRows(); + CHECK_ATTR_COUNT( numAttrs ); + CHECK( HasCellAttr(numRows, numCols) ); + } + + SECTION("Shrinking") + { + CHECK( HasCellAttr(numRows - 1 , numCols - 1) ); + CHECK( HasCellAttr(0, numCols - 1) ); + + m_grid->DeleteCols(); + numAttrs -= m_grid->GetNumberRows(); + CHECK_ATTR_COUNT( numAttrs ); + CHECK( HasCellAttr(0, 0) ); + CHECK( !HasCellAttr(0, numCols - 1) ); + + m_grid->DeleteRows(); + numAttrs -= m_grid->GetNumberCols(); + CHECK_ATTR_COUNT( numAttrs ); + CHECK( !HasCellAttr(numRows - 1 , numCols - 1) ); + } +} + // Test wxGridBlockCoords here because it'a a part of grid sources. std::ostream& operator<<(std::ostream& os, const wxGridBlockCoords& block) { From 058192dde8aa4305d5c2d6aed37bf22aa3c1f981 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Fri, 22 Jan 2021 22:22:22 +0100 Subject: [PATCH 2/7] Add a few accelerators to menu items of grid sample Add convenience accelerators for insert and delete menu options. --- samples/grid/griddemo.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/samples/grid/griddemo.cpp b/samples/grid/griddemo.cpp index 2d345f6796..e8fa60b9c4 100644 --- a/samples/grid/griddemo.cpp +++ b/samples/grid/griddemo.cpp @@ -502,10 +502,10 @@ GridFrame::GridFrame() colMenu->Append( ID_SET_CELL_BG_COLOUR, "Set cell &background colour..." ); wxMenu *editMenu = new wxMenu; - editMenu->Append( ID_INSERTROW, "Insert &row" ); - editMenu->Append( ID_INSERTCOL, "Insert &column" ); - editMenu->Append( ID_DELETEROW, "Delete selected ro&ws" ); - editMenu->Append( ID_DELETECOL, "Delete selected co&ls" ); + editMenu->Append( ID_INSERTROW, "Insert &row\tCtrl+I" ); + editMenu->Append( ID_INSERTCOL, "Insert &column\tCtrl+Shift+I" ); + editMenu->Append( ID_DELETEROW, "Delete selected ro&ws\tCtrl+D" ); + editMenu->Append( ID_DELETECOL, "Delete selected co&ls\tCtrl+Shift+D" ); editMenu->Append( ID_CLEARGRID, "Cl&ear grid cell contents" ); editMenu->Append( ID_EDITCELL, "&Edit current cell" ); editMenu->Append( ID_SETCORNERLABEL, "&Set corner label..." ); From 30de99470a8549bf56daa8b56bab5c89de90c4c7 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Fri, 22 Jan 2021 22:26:59 +0100 Subject: [PATCH 3/7] Add multiple insertion support to grid sample Only single insertions of rows and columns were supported, allow N insertions provided the row/column selection is consecutive. Do the same for deletions: while deleting selected rows and columns already works it's useful to test deleting multiple positions at once. --- samples/grid/griddemo.cpp | 125 ++++++++++++++++++++++++++++++-------- 1 file changed, 99 insertions(+), 26 deletions(-) diff --git a/samples/grid/griddemo.cpp b/samples/grid/griddemo.cpp index e8fa60b9c4..037d9cc6fb 100644 --- a/samples/grid/griddemo.cpp +++ b/samples/grid/griddemo.cpp @@ -502,8 +502,8 @@ GridFrame::GridFrame() colMenu->Append( ID_SET_CELL_BG_COLOUR, "Set cell &background colour..." ); wxMenu *editMenu = new wxMenu; - editMenu->Append( ID_INSERTROW, "Insert &row\tCtrl+I" ); - editMenu->Append( ID_INSERTCOL, "Insert &column\tCtrl+Shift+I" ); + editMenu->Append( ID_INSERTROW, "Insert &rows\tCtrl+I" ); + editMenu->Append( ID_INSERTCOL, "Insert &columns\tCtrl+Shift+I" ); editMenu->Append( ID_DELETEROW, "Delete selected ro&ws\tCtrl+D" ); editMenu->Append( ID_DELETECOL, "Delete selected co&ls\tCtrl+Shift+D" ); editMenu->Append( ID_CLEARGRID, "Cl&ear grid cell contents" ); @@ -1235,32 +1235,115 @@ void GridFrame::SetGridLineColour( wxCommandEvent& WXUNUSED(ev) ) } } +namespace +{ + +// Helper used to insert/delete rows/columns. Allows changing by more than +// one row/column at once. +void HandleEdits(wxGrid* grid, wxGridDirection direction, bool isInsertion) +{ + const bool isRow = (direction == wxGRID_ROW); + + const wxGridBlocks selected = grid->GetSelectedBlocks(); + wxGridBlocks::iterator it = selected.begin(); + + if ( isInsertion ) + { + int pos, count; + // Only do multiple insertions if we have a single consecutive + // selection (mimicking LibreOffice), otherwise do a single insertion + // at cursor. + if ( it != selected.end() && ++it == selected.end() ) + { + const wxGridBlockCoords& b = *selected.begin(); + pos = isRow ? b.GetTopRow() : b.GetLeftCol(); + count = (isRow ? b.GetBottomRow() : b.GetRightCol()) - pos + 1; + } + else + { + pos = isRow ? grid->GetGridCursorRow() : grid->GetGridCursorCol(); + count = 1; + } + + if ( isRow ) + grid->InsertRows(pos, count); + else + grid->InsertCols(pos, count); + + return; + } + + wxGridUpdateLocker locker(grid); + + wxVector deletions; + + for (; it != selected.end(); ++it ) + { + const wxGridBlockCoords& b = *it; + const int begin = isRow ? b.GetTopRow() : b.GetLeftCol(); + const int end = isRow ? b.GetBottomRow() : b.GetRightCol(); + + for ( int n = begin; n <= end; ++n ) + { + if ( !wxVectorContains(deletions, n) ) + deletions.push_back(n); + } + } + + wxVectorSort(deletions); + + const size_t deletionCount = deletions.size(); + + if ( !deletionCount ) + return; + + int seqEnd = 0, seqCount = 0; + for ( size_t i = deletionCount; i > 0; --i ) + { + const int n = deletions[i - 1]; + + if ( n != seqEnd - seqCount ) + { + if (i != deletionCount) + { + const int seqStart = seqEnd - seqCount + 1; + if ( isRow ) + grid->DeleteRows(seqStart, seqCount); + else + grid->DeleteCols(seqStart, seqCount); + } + + seqEnd = n; + seqCount = 0; + } + + seqCount++; + } + + const int seqStart = seqEnd - seqCount + 1; + if ( isRow ) + grid->DeleteRows(seqStart, seqCount); + else + grid->DeleteCols(seqStart, seqCount); +} + +} // anoymous namespace void GridFrame::InsertRow( wxCommandEvent& WXUNUSED(ev) ) { - grid->InsertRows( grid->GetGridCursorRow(), 1 ); + HandleEdits(grid, wxGRID_ROW, /* isInsertion = */ true); } void GridFrame::InsertCol( wxCommandEvent& WXUNUSED(ev) ) { - grid->InsertCols( grid->GetGridCursorCol(), 1 ); + HandleEdits(grid, wxGRID_COLUMN, /* isInsertion = */ true); } void GridFrame::DeleteSelectedRows( wxCommandEvent& WXUNUSED(ev) ) { - if ( grid->IsSelection() ) - { - wxGridUpdateLocker locker(grid); - for ( int n = 0; n < grid->GetNumberRows(); ) - { - if ( grid->IsInSelection( n , 0 ) ) - grid->DeleteRows( n, 1 ); - else - n++; - } - } + HandleEdits(grid, wxGRID_ROW, /* isInsertion = */ false); } @@ -1323,17 +1406,7 @@ void GridFrame::AutoSizeTable(wxCommandEvent& WXUNUSED(event)) void GridFrame::DeleteSelectedCols( wxCommandEvent& WXUNUSED(ev) ) { - if ( grid->IsSelection() ) - { - wxGridUpdateLocker locker(grid); - for ( int n = 0; n < grid->GetNumberCols(); ) - { - if ( grid->IsInSelection( 0 , n ) ) - grid->DeleteCols( n, 1 ); - else - n++; - } - } + HandleEdits(grid, wxGRID_COLUMN, /* isInsertion = */ false); } From 04a3dda5c5d26fd416aa15eac00c24406aba60eb Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Fri, 22 Jan 2021 22:28:08 +0100 Subject: [PATCH 4/7] Refactor common wxGridCellAttrData::UpdateAttrRows/Cols code Move two symmetrical functions into one and clean up, preparing it for multicell support. --- src/generic/grid.cpp | 95 ++++++++++++++++++-------------------------- 1 file changed, 39 insertions(+), 56 deletions(-) diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index 2cf170caec..d62aafe9d1 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -806,72 +806,55 @@ wxGridCellAttr *wxGridCellAttrData::GetAttr(int row, int col) const return attr; } -void wxGridCellAttrData::UpdateAttrRows( size_t pos, int numRows ) +namespace { - size_t count = m_attrs.GetCount(); + +void UpdateCellAttrRowsOrCols(wxGridCellWithAttrArray& attrs, int editPos, + int editRowCount, int editColCount) +{ + wxASSERT( !editRowCount || !editColCount ); + + const bool isEditingRows = (editRowCount != 0); + const int editCount = (isEditingRows ? editRowCount : editColCount); + + size_t count = attrs.GetCount(); for ( size_t n = 0; n < count; n++ ) { - wxGridCellCoords& coords = m_attrs[n].coords; - wxCoord row = coords.GetRow(); - if ((size_t)row >= pos) + wxGridCellCoords& coords = attrs[n].coords; + const wxCoord cellRow = coords.GetRow(), + cellCol = coords.GetCol(), + cellPos = (isEditingRows ? cellRow : cellCol); + + if ( cellPos < editPos ) + continue; + + if ( editCount < 0 && cellPos < editPos - editCount ) { - if (numRows > 0) - { - // If rows inserted, increment row counter where necessary - coords.SetRow(row + numRows); - } - else if (numRows < 0) - { - // If rows deleted ... - if ((size_t)row >= pos - numRows) - { - // ...either decrement row counter (if row still exists)... - coords.SetRow(row + numRows); - } - else - { - // ...or remove the attribute - m_attrs.RemoveAt(n); - n--; - count--; - } - } + // This row/col is deleted and the cell doesn't exist any longer: + // Remove the attribute. + attrs.RemoveAt(n); + n--; + count--; + + continue; } + + // Rows/cols inserted or deleted (and this cell still exists): + // Adjust cell coords. + coords.Set(cellRow + editRowCount, cellCol + editColCount); } } +} // anonymous namespace + +void wxGridCellAttrData::UpdateAttrRows( size_t pos, int numRows ) +{ + UpdateCellAttrRowsOrCols(m_attrs, static_cast(pos), numRows, 0); +} + void wxGridCellAttrData::UpdateAttrCols( size_t pos, int numCols ) { - size_t count = m_attrs.GetCount(); - for ( size_t n = 0; n < count; n++ ) - { - wxGridCellCoords& coords = m_attrs[n].coords; - wxCoord col = coords.GetCol(); - if ( (size_t)col >= pos ) - { - if ( numCols > 0 ) - { - // If cols inserted, increment col counter where necessary - coords.SetCol(col + numCols); - } - else if (numCols < 0) - { - // If cols deleted ... - if ((size_t)col >= pos - numCols) - { - // ...either decrement col counter (if col still exists)... - coords.SetCol(col + numCols); - } - else - { - // ...or remove the attribute - m_attrs.RemoveAt(n); - n--; - count--; - } - } - } - } + UpdateCellAttrRowsOrCols(m_attrs, static_cast(pos), 0, numCols); } int wxGridCellAttrData::FindIndex(int row, int col) const From d282ccf69622bc28676b2b07c03b86a816ba2d28 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Sat, 23 Jan 2021 23:23:23 +0100 Subject: [PATCH 5/7] Add grid tests for multicell integrity after insertions/deletions Multicells currently don't get any special treatment when inserting or deleting rows or columns so neither a multicell's main size nor inside cells' sizes (which are offsets to the main cell) are updated. Most tests fail and will be fixed by the next commit. See #4238. --- tests/controls/gridtest.cpp | 723 +++++++++++++++++++++++++++++++++++- 1 file changed, 720 insertions(+), 3 deletions(-) diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 90b870b960..e1c1aa47b6 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -31,6 +31,60 @@ namespace { +wxString CellCoordsToString(int row, int col) +{ + return wxString::Format("R%dC%d", col + 1, row + 1); +} + +wxString CellSizeToString(int rows, int cols) +{ + return wxString::Format("%dx%d", rows, cols); +} + +struct Multicell +{ + int row, col, rows, cols; + + Multicell(int row, int col, int rows, int cols) + : row(row), col(col), rows(rows), cols(cols) { } + + wxString Coords() const { return CellCoordsToString(row, col); } + + wxString Size() const { return CellSizeToString(rows, cols); } + + wxString ToString() const + { + wxString s; + + if ( rows == 1 && cols == 1 ) + { + s = "cell"; + } + else + { + s = Size() + " "; + if ( rows > 1 || cols > 1 ) + s += "multicell"; + else + s += "inside cell"; + } + + return wxString::Format("%s at %s", s, Coords()); + } +}; + +// Stores insertion/deletion info of rows or columns. +struct EditInfo +{ + int pos, count; + wxGridDirection direction; + + EditInfo(int pos = 0, + int count = 0, + wxGridDirection direction = wxGRID_COLUMN) + : pos(pos), count(count), direction(direction) { } +}; + // Derive a new class inheriting from wxGrid, also to get access to its // protected GetCellAttr(). This is not pretty, but we don't have any other way // of testing this function. @@ -47,7 +101,8 @@ public: return GetCellAttr(row, col); } - bool HasAttr(int row, int col, wxGridCellAttr::wxAttrKind kind) const + bool HasAttr(int row, int col, + wxGridCellAttr::wxAttrKind kind = wxGridCellAttr::Cell) const { // Can't use GetCellAttr() here as it always returns an attr. wxGridCellAttr* attr = GetTable()->GetAttr(row, col, kind); @@ -66,16 +121,95 @@ public: for ( int row = 0; row < GetNumberRows(); ++row ) { for ( int col = 0; col < GetNumberCols(); ++col ) - count += HasAttr(row, col, wxGridCellAttr::Cell); + count += HasAttr(row, col); } return count; } + void SetMulticell(const Multicell& multi) + { + SetCellSize(multi.row, multi.col, multi.rows, multi.cols); + } + + // Performs given insertions/deletions on either rows or columns. + void DoEdit(const EditInfo& edit); + + // Returns annotated grid represented as a string. + wxString ToString() const; + + // Used when drawing annotated grid to know what happens to it. + EditInfo m_edit; + + // Grid as string before editing, with edit info annotated. + wxString m_beforeGridAnnotated; }; +// Compares two grids, checking for differences with attribute presence and +// cell sizes. +class GridAttrMatcher : public Catch::MatcherBase +{ +public: + GridAttrMatcher(const TestableGrid& grid); + + bool match(const TestableGrid& other) const wxOVERRIDE; + + std::string describe() const wxOVERRIDE; + +private: + const TestableGrid* m_grid; + + mutable wxString m_diffDesc; + mutable wxString m_expectedGridDesc; +}; + +// Helper function for recreating a grid to fit (only) a multicell. +void FitGridToMulticell(TestableGrid* grid, const Multicell& multi) +{ + const int oldRowCount = grid->GetNumberRows(); + const int oldColCount = grid->GetNumberCols(); + + const int margin = 1; + const int newRowCount = multi.row + multi.rows + margin; + const int newColCount = multi.col + multi.cols + margin; + + if ( !oldRowCount && !oldColCount ) + { + grid->CreateGrid(newRowCount, newColCount); + } + else + { + grid->DeleteRows(0, oldRowCount); + grid->DeleteCols(0, oldColCount); + grid->AppendRows(newRowCount); + grid->AppendCols(newColCount); + } +} + } // anonymous namespace +namespace Catch +{ + +template <> struct StringMaker +{ + static std::string convert(const TestableGrid& grid) + { + return ("Content before edit:\n" + grid.m_beforeGridAnnotated + + "\nContent after edit:\n" + grid.ToString()).ToStdString(); + } +}; + +template <> struct StringMaker +{ + static std::string convert(const Multicell& multi) + { + return multi.ToString().ToStdString(); + } +}; + +} // namespace Catch + class GridTestCase { public: @@ -83,6 +217,26 @@ public: ~GridTestCase(); protected: + void InsertRows(int pos = 0, int count = 1) + { + m_grid->DoEdit(EditInfo(pos, count, wxGRID_ROW)); + } + + void InsertCols(int pos = 0, int count = 1) + { + m_grid->DoEdit(EditInfo(pos, count, wxGRID_COLUMN)); + } + + void DeleteRows(int pos = 0, int count = 1) + { + m_grid->DoEdit(EditInfo(pos, -count, wxGRID_ROW)); + } + + void DeleteCols(int pos = 0, int count = 1) + { + m_grid->DoEdit(EditInfo(pos, -count, wxGRID_COLUMN)); + } + // The helper function to determine the width of the column label depending // on whether the native column header is used. int GetColumnLabelWidth(wxClientDC& dc, int col, int margin) const @@ -142,12 +296,52 @@ protected: m_grid->SetAttr(row, col, new wxGridCellAttr); } + // Fills temp. grid with a multicell and returns a matcher with it. + GridAttrMatcher HasMulticellOnly(const Multicell& multi) + { + return CheckMulticell(multi); + } + + // Returns a matcher with empty (temp.) grid. + GridAttrMatcher HasEmptyGrid() + { + return CheckMulticell(Multicell(0, 0, 1, 1)); + } + + // Helper function used by the previous two functions. + GridAttrMatcher CheckMulticell(const Multicell& multi) + { + TestableGrid* grid = GetTempGrid(); + + FitGridToMulticell(grid, multi); + + if ( multi.rows > 1 || multi.cols > 1 ) + grid->SetMulticell(multi); + + return GridAttrMatcher(*grid); + } + + TestableGrid* GetTempGrid() + { + if ( !m_tempGrid ) + { + m_tempGrid = new TestableGrid(wxTheApp->GetTopWindow()); + m_tempGrid->Hide(); + } + + return m_tempGrid; + } + TestableGrid *m_grid; + // Temporary/scratch grid filled with expected content, used when + // comparing against m_grid. + TestableGrid *m_tempGrid; + wxDECLARE_NO_COPY_CLASS(GridTestCase); }; -GridTestCase::GridTestCase() +GridTestCase::GridTestCase() : m_tempGrid(NULL) { m_grid = new TestableGrid(wxTheApp->GetTopWindow()); m_grid->CreateGrid(10, 2); @@ -176,6 +370,7 @@ GridTestCase::~GridTestCase() win->ReleaseMouse(); wxDELETE(m_grid); + delete m_tempGrid; } TEST_CASE_METHOD(GridTestCase, "Grid::CellEdit", "[grid]") @@ -1556,6 +1751,313 @@ TEST_CASE_METHOD(GridTestCase, "Grid::CellAttribute", "[attr][cell][grid]") } } +#define CHECK_MULTICELL() CHECK_THAT( *m_grid, HasMulticellOnly(multi) ) + +#define CHECK_NO_MULTICELL() CHECK_THAT( *m_grid, HasEmptyGrid() ) + +#define WHEN_N(s, n) WHEN(wxString::Format(s, n).ToStdString()) + +TEST_CASE_METHOD(GridTestCase, + "Grid::InsertionsWithMulticell", + "[attr][cell][grid][insert][multicell]") +{ + int insertions = 0, offset = 0; + + Multicell multi(1, 1, 3, 5); + + SECTION("Sanity checks") + { + FitGridToMulticell(m_grid, multi); + m_grid->SetMulticell(multi); + + REQUIRE( static_cast(m_grid->GetCellAttrCount()) + == multi.rows * multi.cols ); + + int row, col, rows, cols; + + // Check main cell. + row = multi.row, + col = multi.col; + wxGrid::CellSpan span = m_grid->GetCellSize(row, col, &rows, &cols); + + REQUIRE( span == wxGrid::CellSpan_Main ); + REQUIRE( rows == multi.rows ); + REQUIRE( cols == multi.cols ); + + // Check inside cell at opposite of main. + row = multi.row + multi.rows - 1; + col = multi.col + multi.cols - 1; + span = m_grid->GetCellSize(row, col, &rows, &cols); + + REQUIRE( span == wxGrid::CellSpan_Inside ); + REQUIRE( rows == multi.row - row ); + REQUIRE( cols == multi.col - col ); + } + + // Do some basic testing with column insertions first and do more tests + // with edge cases later on just with rows. It's not really needed to + // repeat the same tests for both rows and columns as the code for + // updating them works symmetrically. + + GIVEN(Catch::toString(multi)) + { + FitGridToMulticell(m_grid, multi); + m_grid->SetMulticell(multi); + + insertions = 2; + + WHEN("inserting any columns in multicell, at main") + { + InsertCols(multi.col + 0, insertions); + + THEN("the position changes but not the size") + { + multi.col += insertions; + CHECK_MULTICELL(); + } + } + + WHEN("inserting any columns in multicell, just after main") + { + InsertCols(multi.col + 1, insertions); + + THEN("the size changes but not the position") + { + multi.cols += insertions; + CHECK_MULTICELL(); + } + } + + } + + // Do more extensive testing with rows. + + wxSwap(multi.rows, multi.cols); + + GIVEN(Catch::toString(multi)) + { + FitGridToMulticell(m_grid, multi); + m_grid->SetMulticell(multi); + + const int insertionCounts[] = {1, 2, multi.rows}; + + for ( size_t i = 0; i < WXSIZEOF(insertionCounts); ++i ) + { + insertions = insertionCounts[i]; + + WHEN_N("inserting %d row(s), just before main", insertions) + { + InsertRows(multi.row - 1, insertions); + + THEN("the position changes but not the size") + { + multi.row += insertions; + CHECK_MULTICELL(); + } + } + + WHEN_N("inserting %d row(s) in multicell, at main", insertions) + { + InsertRows(multi.row + 0, insertions); + + THEN("the position changes but not the size") + { + multi.row += insertions; + CHECK_MULTICELL(); + } + } + } + + insertions = multi.rows / 2; + + // Check insertions within multicell, at and near edges. + const int insertionOffsets[] = {1, 2, multi.rows - 2, multi.rows - 1}; + + for ( size_t i = 0; i < WXSIZEOF(insertionOffsets); ++i ) + { + offset = insertionOffsets[i]; + + WHEN_N("inserting rows in multicell, %d row(s) after main", offset) + { + InsertRows(multi.row + offset, insertions); + + THEN("the size changes but not the position") + { + multi.rows += insertions; + CHECK_MULTICELL(); + } + } + } + + // Check at least one case of inserting after multicell. + WHEN("inserting rows, just after multicell") + { + insertions = 2; + InsertRows(multi.row + multi.rows, insertions); + + THEN("neither size nor position change") + { + CHECK_MULTICELL(); + } + } + } + +} + +TEST_CASE_METHOD(GridTestCase, + "GridMulticell::DeletionsWithMulticell", + "[cellattr][delete][grid][multicell]") +{ + int deletions = 0, offset = 0; + + // Same as with the previous (insertions) test case but instead of some + // basic testing with columns first, this time use rows for that and do more + // extensive testing with columns. + + Multicell multi(1, 1, 5, 3); + + GIVEN(Catch::toString(multi)) + { + FitGridToMulticell(m_grid, multi); + m_grid->SetMulticell(multi); + + WHEN("deleting any rows, at main") + { + deletions = 1; + DeleteRows(multi.row + 0, deletions); + + THEN("the multicell is deleted") + { + CHECK_NO_MULTICELL(); + } + } + + WHEN("deleting more rows than length of multicell," + " at end of multicell") + { + deletions = multi.rows + 2; + offset = multi.rows - 1; + DeleteRows(multi.row + offset, deletions); + + THEN("the size changes but not the position") + { + multi.rows = offset; + CHECK_MULTICELL(); + } + } + } + + // Do more extensive testing with columns. + + wxSwap(multi.rows, multi.cols); + + GIVEN(Catch::toString(multi)) + { + FitGridToMulticell(m_grid, multi); + m_grid->SetMulticell(multi); + + WHEN("deleting one column, just before main") + { + DeleteCols(multi.col - 1); + + THEN("the position changes but not the size") + { + multi.col--; + CHECK_MULTICELL(); + } + } + + WHEN("deleting multiple columns, just before main") + { + deletions = 2; // Must be at least 2 to affect main. + DeleteCols(multi.col - 1, wxMax(2, deletions)); + + THEN("the multicell is deleted") + { + CHECK_NO_MULTICELL(); + } + } + + WHEN("deleting any columns, at main") + { + deletions = 1; + DeleteCols(multi.col + 0, deletions); + + THEN("the multicell is deleted") + { + CHECK_NO_MULTICELL(); + } + } + + WHEN("deleting one column within multicell, after main") + { + offset = 1; + offset = wxClip(offset, 1, multi.cols - 1); + DeleteCols(multi.col + offset, 1); + + THEN("the size changes but not the position") + { + multi.cols--; + CHECK_MULTICELL(); + } + } + + deletions = 2; + + // Check deletions within multicell, at and near edges. + const int offsets[] = {1, 2, multi.cols - deletions}; + + for ( size_t i = 0; i < WXSIZEOF(offsets); ++i ) + { + offset = offsets[i]; + + WHEN_N("deleting columns only within multicell," + " %d column(s) after main", offset) + { + DeleteCols(multi.col + offset, deletions); + + THEN("the size changes but not the position") + { + multi.cols -= deletions; + CHECK_MULTICELL(); + } + } + } + + // Instead of stuffing the multicell's length logic in the above test, + // separately check at least two cases of starting many deletions + // within multicell. + + WHEN("deleting more columns than length of multicell, just after main") + { + deletions = multi.cols + 2; + offset = 1; + DeleteCols(multi.col + offset, deletions); + + THEN("the size changes but not the position") + { + multi.cols = offset; + CHECK_MULTICELL(); + } + } + + WHEN("deleting more columns than length of multicell," + " at end of multicell") + { + deletions = multi.cols + 2; + offset = multi.cols - 1; + DeleteCols(multi.col + offset, deletions); + + THEN("the size changes but not the position") + { + multi.cols = offset; + CHECK_MULTICELL(); + } + } + } + +} + // Test wxGridBlockCoords here because it'a a part of grid sources. std::ostream& operator<<(std::ostream& os, const wxGridBlockCoords& block) { @@ -1742,4 +2244,219 @@ TEST_CASE("GridBlockCoords::SymDifference", "[grid]") } } +// +// TestableGrid +// + +void TestableGrid::DoEdit(const EditInfo& edit) +{ + m_edit = edit; + m_beforeGridAnnotated = ToString(); + + switch ( edit.direction ) + { + case wxGRID_COLUMN: + if ( edit.count < 0 ) + DeleteCols(edit.pos, -edit.count); + else + InsertCols(edit.pos, edit.count); + break; + + case wxGRID_ROW: + if ( edit.count < 0 ) + DeleteRows(edit.pos, -edit.count); + else + InsertRows(edit.pos, edit.count); + break; + } +} + +wxString TestableGrid::ToString() const +{ + const int numRows = GetNumberRows(); + const int numCols = GetNumberCols(); + + const int colMargin = GetRowLabelValue(numRows - 1).length(); + const wxString leftIndent = wxString(' ', colMargin + 1); + + // String s contains the rendering of the grid, start with drawing + // the header columns. + wxString s = leftIndent; + + const int base = 10; + // Draw the multiples of 10. + for ( int col = base; col <= numCols; col += base) + { + s += wxString(' ', base - 1); + s += ('0' + (col / base)); + } + + if ( numCols >= base ) + s += "\n" + leftIndent; + + // Draw the single digits. + for ( int col = 1; col <= numCols; ++col ) + s += '0' + (col % base); + s += "\n"; + + // Draw horizontal divider. + s += wxString(' ', colMargin) + '+' + wxString('-', numCols) + "\n"; + + const int absEditCount = abs(m_edit.count); + wxString action; + action.Printf(" %s: %d", + m_edit.count < 0 ? "deletions" : "insertions", + absEditCount); + + // Will contain summary of grid (only multicells mentioned). + wxString content; + + // Draw grid content. + for ( int row = 0; row < numRows; ++row ) + { + const wxString label = GetRowLabelValue(row); + s += wxString(' ', colMargin - label.length()); + s += label + '|'; + + for ( int col = 0; col < numCols; ++col ) + { + char c = 'x'; + int rows, cols; + switch ( GetCellSize(row, col, &rows, &cols) ) + { + case wxGrid::CellSpan_None: + c = HasAttr(row, col) ? '*' : '.'; + break; + + case wxGrid::CellSpan_Main: + c = 'M'; + content += Multicell(row, col, rows, cols).ToString() + "\n"; + break; + + case wxGrid::CellSpan_Inside: + // Check if the offset to main cell is correct. + c = (GetCellSize(row + rows, col + cols, &rows, &cols) + == wxGrid::CellSpan_Main) ? 'm' : '?'; + break; + } + + s += c; + } + + // If applicable draw annotated row edits. + if ( m_edit.count && m_edit.direction == wxGRID_ROW + && row >= m_edit.pos && row < m_edit.pos + absEditCount) + { + s += (m_edit.count < 0 ? " ^" : " v"); + + if ( row == m_edit.pos ) + s += action; + } + + s += "\n"; + } + + // Draw annotated footer if columns edited. + if ( m_edit.count && m_edit.direction == wxGRID_COLUMN ) + { + s += leftIndent; + + for ( int col = 0; col < m_edit.pos + absEditCount; ++col ) + { + if ( col < m_edit.pos ) + s += " "; + else + s += (m_edit.count < 0 ? "<" : ">"); + } + + s += action + "\n"; + } + + s += "\n"; + + if ( content.empty() ) + content = "Empty grid\n"; + + return content + s; +} + +// +// GridAttrMatcher +// + +GridAttrMatcher::GridAttrMatcher(const TestableGrid& grid) : m_grid(&grid) +{ + m_expectedGridDesc = m_grid->ToString(); +} + +bool GridAttrMatcher::match(const TestableGrid& other) const +{ + const int rows = wxMax(m_grid->GetNumberRows(), other.GetNumberRows()); + const int cols = wxMax(m_grid->GetNumberCols(), other.GetNumberCols()); + + for ( int row = 0; row < rows; ++row ) + { + for ( int col = 0; col < cols; ++col ) + { + const bool hasAttr = m_grid->HasAttr(row, col); + if ( hasAttr != other.HasAttr(row, col) ) + { + m_diffDesc.Printf("%s: attribute presence (%d, expected %d)", + CellCoordsToString(row, col), + !hasAttr, hasAttr); + + return false; + } + + int thisRows, thisCols; + const wxGrid::CellSpan thisSpan + = m_grid->GetCellSize(row, col, &thisRows, &thisCols); + + int otherRows, otherCols; + (void) other.GetCellSize(row, col, &otherRows, &otherCols); + + if ( thisRows != otherRows || thisCols != otherCols ) + { + wxString mismatchKind; + switch ( thisSpan ) + { + case wxGrid::CellSpan_None: + mismatchKind = "different cell size"; + break; + + case wxGrid::CellSpan_Main: + mismatchKind = "different multicell size"; + break; + + case wxGrid::CellSpan_Inside: + mismatchKind = "main offset mismatch"; + break; + } + + m_diffDesc.Printf( "%s: %s (%s, expected %s)", + CellCoordsToString(row, col), + mismatchKind, + CellSizeToString(otherRows, otherCols), + CellSizeToString(thisRows, thisCols) + ); + + return false; + } + } + } + + return true; +} + +std::string GridAttrMatcher::describe() const +{ + std::string desc = (m_diffDesc.empty() ? "Matches" : "Doesn't match"); + desc += " expected content:\n" + m_expectedGridDesc.ToStdString(); + + if ( !m_diffDesc.empty() ) + desc += + "first difference at " + m_diffDesc.ToStdString(); + + return desc; +} + #endif //wxUSE_GRID From 3123c763948e3dcff49ccc2a5a877040afcbe8d1 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Mon, 25 Jan 2021 21:30:09 +0100 Subject: [PATCH 6/7] Fix grid multicell integrity after inserting/deleting rows/columns Deal with possible size changes of a multicell (both main and inside cells) when inserting or deleting rows or columns and, in the case of deletion only, the disappearing of a multicell's main cell. Closes #4238. --- src/generic/grid.cpp | 132 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 130 insertions(+), 2 deletions(-) diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index d62aafe9d1..53f982f6e6 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -820,13 +820,82 @@ void UpdateCellAttrRowsOrCols(wxGridCellWithAttrArray& attrs, int editPos, size_t count = attrs.GetCount(); for ( size_t n = 0; n < count; n++ ) { + wxGridCellAttr* cellAttr = attrs[n].attr; + int cellRows, cellCols; + cellAttr->GetSize(&cellRows, &cellCols); + wxGridCellCoords& coords = attrs[n].coords; const wxCoord cellRow = coords.GetRow(), cellCol = coords.GetCol(), cellPos = (isEditingRows ? cellRow : cellCol); if ( cellPos < editPos ) + { + // This cell's coords aren't influenced by the editing, however + // do adjust a multicell's main size, if needed. + if ( GetCellSpan(cellRows, cellCols) == wxGrid::CellSpan_Main ) + { + int mainSize = isEditingRows ? cellRows : cellCols; + if ( cellPos + mainSize > editPos ) + { + // Multicell is within affected range: + // Adjust its size. + + if ( editCount >= 0 ) + { + mainSize += editCount; + } + else + { + // Reduce multicell size by number of deletions, but + // never more than the multicell's size minus one: + // cellPos (the main cell) is always less than editPos + // at this point, then with the below code a multicell + // with size 7 is at most reduced by: + // cellPos + 7 - (cellPos + 1) = 7 - 1 = 6. + mainSize -= wxMin(-editCount, + cellPos + mainSize - editPos); + /* + The above was derived from: + first_del = edit + last_del = min(edit - count - 1, cell + size - 1) + size -= (last_del + 1 - first_del) + + eliminating the 1's: + + first_del = edit + last_del = min(edit - count, cell + size) + size -= (last_del - first_del) + + reducing each by edit: + + first_del = 0 + last_del_plus_1 = min(0 - count, cell + size - edit) + size -= (last_del_plus_1 - 0) + + after eliminating the 0's and substitution, leaving: + + size -= min(-count, cell + size - edit) + + E.g. with a multicell of size 7 and at 2 positions + after the main cell 100 positions are deleted then + the size will not (/can't) be reduced by 100 cells + but by: + + cellPos + 7 - editPos = # editPos = cellPos + 2 + cellPos + 7 - (cellPos + 2) = # eliminate cellPos + 7 - 2 = + 5 cells, making the final size 7 - 5 = 2. + */ + } + + cellAttr->SetSize(isEditingRows ? mainSize : cellRows, + isEditingRows ? cellCols : mainSize); + } + } + continue; + } if ( editCount < 0 && cellPos < editPos - editCount ) { @@ -839,9 +908,68 @@ void UpdateCellAttrRowsOrCols(wxGridCellWithAttrArray& attrs, int editPos, continue; } - // Rows/cols inserted or deleted (and this cell still exists): - // Adjust cell coords. + 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); + + // Nothing more to do: cell is not an inside cell of a multicell. + continue; + } + + // Handle inside cell's existence, coords, and size. + + const int mainPos = cellPos + (isEditingRows ? cellRows : cellCols); + + if ( editCount < 0 + && mainPos >= editPos && mainPos < editPos - editCount ) + { + // 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); + n--; + count--; + + continue; + } + + // Rows/cols inserted or deleted (and this inside cell still exists): + // Adjust (inside) cell coords. coords.Set(cellRow + editRowCount, cellCol + editColCount); + + if ( mainPos >= editPos ) + { + // Nothing more to do: the multicell that this inside cell is part + // of is moving its main cell as well so offsets to the main cell + // don't change and there are no edits changing the multicell size. + continue; + } + + if ( editCount > 0 && cellPos == editPos ) + { + // At an (old) position that is newly inserted: this is the only + // opportunity to add required inside cells that point to + // the main cell. E.g. with a 2x1 multicell that increases in size + // there's only one inside cell that will be visited while there + // can be multiple insertions. + for ( int i = 0; i < editCount; ++i ) + { + const int adjustRows = i * isEditingRows, + adjustCols = i * !isEditingRows; + + wxGridCellAttr* attr = new wxGridCellAttr; + attr->SetSize(cellRows - adjustRows, cellCols - adjustCols); + + attrs.Add(new wxGridCellWithAttr(cellRow + adjustRows, + cellCol + adjustCols, + attr)); + } + } + + // Let this inside cell's size point to the main cell of the multicell. + cellAttr->SetSize(cellRows - editRowCount, cellCols - editColCount); } } From cc239f36cc049a803dd2b1f8e59ab60c572a3ae9 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Mon, 25 Jan 2021 21:48:47 +0100 Subject: [PATCH 7/7] Remove doc warning regarding wxGrid multicells and deletion Since the parent commit multicells work correctly when deleting (and inserting) rows (or columns) intersecting with them and the comment warning about it resulting in a crash is no longer needed. This reverts commit 4d8e8355b4 (Added a warning about multi-cells in wxGrid::DeleteRows() docs., 2011-12-25). See #4238. --- interface/wx/grid.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/interface/wx/grid.h b/interface/wx/grid.h index 3a6129a3f6..02d8767444 100644 --- a/interface/wx/grid.h +++ b/interface/wx/grid.h @@ -2511,9 +2511,6 @@ public: /** Delete rows from the table. - Notice that currently deleting a row intersecting a multi-cell (see - SetCellSize()) is not supported and will result in a crash. - @param pos The first row to delete. @param numRows