From 6d3dbc3fe50c948ba96611c4c8e72f81c7a3fe33 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Tue, 19 Jan 2021 22:01:23 +0100 Subject: [PATCH 1/4] Add test demonstrating drawing of an invalid grid cell The way the test grid is set up forces drawing of an inside cell (part of a multicell) which shouldn't normally occur. In this case it leads to an infinite recursion while drawing the inside cell. Drawing of inside cells will be fixed by the next commit. --- tests/controls/gridtest.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 87e5475749..72d6f8ae4b 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -1423,6 +1423,32 @@ TEST_CASE_METHOD(GridTestCase, "Grid::AutoSizeColumn", "[grid]") } } +TEST_CASE_METHOD(GridTestCase, "Grid::DrawInvalidCell", "[grid][multicell]") +{ + // Set up a multicell with inside an overflowing cell. + // This is artificial and normally inside cells are probably not expected + // to have a value but this is merely done to check if inside cells are + // drawn, which they shouldn't be. + m_grid->SetCellSize(0, 0, 2, 1); + m_grid->SetCellValue( 1, 0, wxString('W', 42) ); + + // Update()s, yields and sleep are needed to try to make the test fail with + // macOS, GTK and MSW. + // MSW needs just the yields (or updates), macOS in addition needs to sleep + // (doesn't work with updates) and for GTK it's usually enough to just do + // two updates (not yields). This test does all unconditionally. + m_grid->Update(); + wxYield(); + wxMilliSleep(20); + + // Try to force redrawing of the inside cell: if it still draws there will + // be an infinite recursion. + m_grid->SetColSize(1, m_grid->GetColSize(1) + 1); + + m_grid->Update(); + wxYield(); +} + // Test wxGridBlockCoords here because it'a a part of grid sources. std::ostream& operator<<(std::ostream& os, const wxGridBlockCoords& block) { From 00c497125e727154bc243b32813447c927d38ce7 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Tue, 19 Jan 2021 22:17:41 +0100 Subject: [PATCH 2/4] Fix drawing of grid cells appearing inside a multicell Grid cells are considered for redrawing solely based on having a (text) value. This can lead to infinite recursion with overflowing inside cells if wxGridCellStringRenderer::Draw() wants to draw cells appearing after this one but instead visits the same cell again (because of a negative cell size as opposed to expected default cell size of 1x1 or a larger spanning size) and calls DrawCell() again for this cell which will call the renderer's Draw() again etc... Fix by not taking inside cells into consideration for redrawing. This is the right thing to do as earlier on in the same function a cell is not drawn for the same reason. Also the aforementioned Draw() mentions it shouldn't be called for cell sizes <= 0. Also fixes the crashing grid test just introduced in 6d3dbc3fe5. --- src/generic/grid.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index f6b7244760..566f0a8102 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -6217,6 +6217,12 @@ void wxGrid::DrawGridCellArea( wxDC& dc, const wxGridCellCoordsArray& cells ) { if (!m_table->IsEmptyCell(row + l, j)) { + int numRows, numCols; + if ( GetCellSize(row + l, j, &numRows, &numCols) + == wxGrid::CellSpan_Inside ) + // As above: don't bother drawing inside cells. + continue; + if ( GetCellAttrPtr(row + l, j)->CanOverflow() ) { wxGridCellCoords cell(row + l, j); From 860d9d09bceee989f0ca813fe8bfac0429126643 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Tue, 19 Jan 2021 22:26:03 +0100 Subject: [PATCH 3/4] Refactor code deciding the kind of span of a grid cell Move wxGrid's GetCellSize() cell span logic into GetCellSpan() for future usage. --- src/generic/grid.cpp | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index 566f0a8102..cd06df1b72 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -158,6 +158,24 @@ wxDEFINE_EVENT( wxEVT_GRID_TABBING, wxGridEvent ); // implementation // ============================================================================ +namespace +{ + +// Helper function for consistent cell span determination based on cell size. +wxGrid::CellSpan GetCellSpan(int numRows, int numCols) +{ + if ( numRows == 1 && numCols == 1 ) + return wxGrid::CellSpan_None; // just a normal cell + + if ( numRows < 0 || numCols < 0 ) + return wxGrid::CellSpan_Inside; // covered by a multi-span cell + + // this cell spans multiple cells to its right/bottom + return wxGrid::CellSpan_Main; +} + +} // anonymous namespace + wxIMPLEMENT_ABSTRACT_CLASS(wxGridCellEditorEvtHandler, wxEvtHandler); wxBEGIN_EVENT_TABLE( wxGridCellEditorEvtHandler, wxEvtHandler ) @@ -9126,14 +9144,7 @@ wxGrid::GetCellSize( int row, int col, int *num_rows, int *num_cols ) const { GetCellAttrPtr(row, col)->GetSize( num_rows, num_cols ); - if ( *num_rows == 1 && *num_cols == 1 ) - return CellSpan_None; // just a normal cell - - if ( *num_rows < 0 || *num_cols < 0 ) - return CellSpan_Inside; // covered by a multi-span cell - - // this cell spans multiple cells to its right/bottom - return CellSpan_Main; + return GetCellSpan(*num_rows, *num_cols); } wxGridCellRenderer* wxGrid::GetCellRenderer(int row, int col) const From 1f5b77bad3737ec4c3bcd5ce6a5f915c4746db7b Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Tue, 19 Jan 2021 22:30:33 +0100 Subject: [PATCH 4/4] Make use of grid helper function GetCellSpan() Also gets rid of an additional attribute lookup that was recently added in 00c497125e containing a minimal fix. --- src/generic/grid.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index cd06df1b72..2cf170caec 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -6235,13 +6235,15 @@ void wxGrid::DrawGridCellArea( wxDC& dc, const wxGridCellCoordsArray& cells ) { if (!m_table->IsEmptyCell(row + l, j)) { + wxGridCellAttrPtr attr = GetCellAttrPtr(row + l, j); int numRows, numCols; - if ( GetCellSize(row + l, j, &numRows, &numCols) + attr->GetSize(&numRows, &numCols); + if ( GetCellSpan(numRows, numCols) == wxGrid::CellSpan_Inside ) // As above: don't bother drawing inside cells. continue; - if ( GetCellAttrPtr(row + l, j)->CanOverflow() ) + if ( attr->CanOverflow() ) { wxGridCellCoords cell(row + l, j); bool marked = false;