From 5834bef2f7dafe5b4f4c91f387f1ef6ec4f5bb16 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 31 Mar 2020 19:24:28 +0200 Subject: [PATCH 1/3] Fix memory leaks in wxGrid unit tests Use smart pointers to ensure that various objects are released, as they have to be. --- tests/controls/gridtest.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 735d571d70..7277ab8817 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -899,7 +899,7 @@ TEST_CASE_METHOD(GridTestCase, "Grid::GetNonDefaultAlignment", "[grid]") // GetNonDefaultAlignment() is used by several renderers having their own // preferred alignment, so check that if we don't reset the alignment // explicitly, it doesn't override the alignment used by default. - wxGridCellAttr* attr = NULL; + wxGridCellAttrPtr attr; int hAlign = wxALIGN_RIGHT, vAlign = wxALIGN_INVALID; @@ -1174,9 +1174,9 @@ TEST_CASE_METHOD(GridTestCase, "Grid::AutoSizeColumn", "[grid]") // Hardcoded extra margin for the columns used in grid.cpp. const int margin = m_grid->FromDIP(10); - wxGridCellAttr *attr = m_grid->GetOrCreateCellAttr(0, 0); - wxGridCellRenderer *renderer = attr->GetRenderer(m_grid, 0, 0); - REQUIRE(renderer != NULL); + wxGridCellAttrPtr attr(m_grid->GetOrCreateCellAttr(0, 0)); + wxGridCellRendererPtr renderer(attr->GetRenderer(m_grid, 0, 0)); + REQUIRE(renderer); wxClientDC dcCell(m_grid->GetGridWindow()); From ca3016976715bf0844f97ca0ddf293c81675696c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 31 Mar 2020 19:27:34 +0200 Subject: [PATCH 2/3] Restore overflowing for cells with non-default vertical alignment Since the changes in a40acbb28e (Add CanOverflow function to wxGridCellAttr, 2020-02-06), cells with non-default vertical alignment didn't overflow any more, even if their horizontal alignment was unchanged and still defaulted to left-aligned. This was due to assuming that if the alignment of wxGridCellAttr itself was different from wxALIGN_LEFT, it meant that it wasn't left-aligned, which seems logical but is in fact false, as the alignment can also be wxALIGN_INVALID, in which case the real alignment is taken from the default grid attribute. Fix this by using GetNonDefaultAlignment() to get the alignment value effectively used and add a unit test, as well as an example in the sample, showing that this now works correctly. --- samples/grid/griddemo.cpp | 1 + src/generic/grid.cpp | 14 +++++++++++--- tests/controls/gridtest.cpp | 8 ++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/samples/grid/griddemo.cpp b/samples/grid/griddemo.cpp index b02ef8c0f9..013afb6045 100644 --- a/samples/grid/griddemo.cpp +++ b/samples/grid/griddemo.cpp @@ -614,6 +614,7 @@ GridFrame::GridFrame() grid->SetRowSize(10, 30); attr = new wxGridCellAttr; attr->SetBackgroundColour(*wxLIGHT_GREY); + attr->SetAlignment(wxALIGN_INVALID, wxALIGN_CENTRE); grid->SetRowAttr(10, attr); grid->SetCellValue(10, 0, "You can't resize this row interactively -- try it"); diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index ad0f62d585..f4196a3969 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -647,9 +647,17 @@ wxGridFitMode wxGridCellAttr::GetFitMode() const bool wxGridCellAttr::CanOverflow() const { - int hAlign; - GetAlignment(&hAlign, NULL); - return GetOverflow() && (hAlign == wxALIGN_LEFT); + // If overflow is disabled anyhow, we definitely can't overflow. + if ( !GetOverflow() ) + return false; + + // But if it is enabled, we still don't use it for right-aligned or + // centered cells because it's not really clear how it should work for + // them. + int hAlign = wxALIGN_LEFT; + GetNonDefaultAlignment(&hAlign, NULL); + + return hAlign == wxALIGN_LEFT; } // GetRenderer and GetEditor use a slightly different decision path about diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 7277ab8817..23de3046a5 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -925,6 +925,14 @@ TEST_CASE_METHOD(GridTestCase, "Grid::GetNonDefaultAlignment", "[grid]") attr->GetNonDefaultAlignment(&hAlign, &vAlign); CHECK( hAlign == wxALIGN_RIGHT ); CHECK( vAlign == wxALIGN_CENTRE_VERTICAL ); + + // This is only indirectly related, but test here for CanOverflow() working + // correctly for the cells with non-default alignment, as this used to be + // broken. + m_grid->SetCellAlignment(0, 0, wxALIGN_INVALID, wxALIGN_CENTRE); + attr = m_grid->CallGetCellAttr(0, 0); + REQUIRE( attr ); + CHECK( attr->CanOverflow() ); } TEST_CASE_METHOD(GridTestCase, "Grid::Editable", "[grid]") From 104733550eb99e88f3100557b773036703eb145d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 31 Mar 2020 17:09:40 +0200 Subject: [PATCH 3/3] Use wxGridCellAttrPtr typedef No real changes, just make the code slightly shorter. --- samples/grid/griddemo.cpp | 3 +-- samples/grid/griddemo.h | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/samples/grid/griddemo.cpp b/samples/grid/griddemo.cpp index 013afb6045..62560cc04a 100644 --- a/samples/grid/griddemo.cpp +++ b/samples/grid/griddemo.cpp @@ -1605,8 +1605,7 @@ MyGridCellAttrProvider::MyGridCellAttrProvider() wxGridCellAttr *MyGridCellAttrProvider::GetAttr(int row, int col, wxGridCellAttr::wxAttrKind kind /* = wxGridCellAttr::Any */) const { - wxObjectDataPtr - attr(wxGridCellAttrProvider::GetAttr(row, col, kind)); + wxGridCellAttrPtr attr(wxGridCellAttrProvider::GetAttr(row, col, kind)); if ( row % 2 ) { diff --git a/samples/grid/griddemo.h b/samples/grid/griddemo.h index 68319b204a..784e0fca3d 100644 --- a/samples/grid/griddemo.h +++ b/samples/grid/griddemo.h @@ -301,7 +301,7 @@ public: wxGridCellAttr::wxAttrKind kind) const wxOVERRIDE; private: - wxObjectDataPtr m_attrForOddRows; + wxGridCellAttrPtr m_attrForOddRows; }; // ----------------------------------------------------------------------------