From f29b6564b1a9b39eb1e03b097f284c71efc2c3da Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 19 Oct 2019 18:28:03 +0200 Subject: [PATCH] Really fix recent regression in grid content autosizing Fix autosizing broken in 3c72396a3670326e6824d1605954920b351a76d2 and not fully fixed by f7e335c031e83874bf0ee64568ae72d5d0df73cf. Simplify the code to make it more obviously correct, by separating the computation of the extent suitable for the label and determining the size to use taking into account the extents of both the column data and the its column. Also add the unit test checking that auto-sizing works correctly in all the different cases. Closes https://github.com/wxWidgets/wxWidgets/pull/1600 Co-Authored-By: Ilya Sinitsyn --- src/generic/grid.cpp | 59 +++++++++++-------- tests/controls/gridtest.cpp | 110 ++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 22 deletions(-) diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index 9643698bb8..da5fbe00fa 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -9397,52 +9397,67 @@ wxGrid::AutoSizeColOrRow(int colOrRow, bool setAsMin, wxGridDirection direction) } // now also compare with the column label extent - wxCoord w, h; + wxCoord extentLabel; dc.SetFont( GetLabelFont() ); - bool addMargin = true; + // We add some margin around text for better readability. + const int margin = column ? 10 : 6; if ( column ) { if ( m_useNativeHeader ) { - w = GetGridColHeader()->GetColumnTitleWidth(colOrRow); + extentLabel = GetGridColHeader()->GetColumnTitleWidth(colOrRow); - // GetColumnTitleWidth already adds margins internally. - addMargin = false; - h = 0; + // Note that GetColumnTitleWidth already adds margins internally, + // so we don't need to add them here. } else { - dc.GetMultiLineTextExtent( GetColLabelValue(colOrRow), &w, &h ); - if ( GetColLabelTextOrientation() == wxVERTICAL ) - w = h; + const wxSize + size = dc.GetMultiLineTextExtent(GetColLabelValue(colOrRow)); + extentLabel = GetColLabelTextOrientation() == wxVERTICAL + ? size.y + : size.x; + + // Add some margins around text for better readability. + extentLabel += margin; } } else { - dc.GetMultiLineTextExtent( GetRowLabelValue(colOrRow), &w, &h ); + extentLabel = dc.GetMultiLineTextExtent(GetRowLabelValue(colOrRow)).y; + + // As above, add some margins for readability, although a smaller one + // in vertical direction. + extentLabel += margin; } - extent = column ? w : h; - if ( extent > extentMax ) - extentMax = extent; + // Finally determine the suitable extent fitting both the cells contents + // and the label. if ( !extentMax ) { - // empty column - give default extent (notice that if extentMax is less - // than default extent but != 0, it's OK) - extentMax = column ? m_defaultColWidth : m_defaultRowHeight; + // Special case: all the cells are empty, use the label extent. + extentMax = extentLabel; + if ( !extentMax ) + { + // But if the label is empty too, use the default width/height. + extentMax = column ? m_defaultColWidth : m_defaultRowHeight; + } } - else if ( addMargin ) + else // We have some data in the column cells. { - // leave some space around text - if ( column ) - extentMax += 10; - else - extentMax += 6; + // Ensure we have the same margin around the cells text as we use + // around the label. + extentMax += margin; + + // And increase it to fit the label if necessary. + if ( extentLabel > extentMax ) + extentMax = extentLabel; } + if ( column ) { // Ensure automatic width is not less than minimal width. See the diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 62771e7400..e8d83e7474 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -16,9 +16,11 @@ #ifndef WX_PRECOMP #include "wx/app.h" + #include "wx/dcclient.h" #endif // WX_PRECOMP #include "wx/grid.h" +#include "wx/headerctrl.h" #include "testableframe.h" #include "asserthelper.h" #include "wx/uiaction.h" @@ -62,18 +64,21 @@ private: WXUISIM_TEST( ReadOnly ); WXUISIM_TEST( ResizeScrolledHeader ); WXUISIM_TEST( ColumnMinWidth ); + WXUISIM_TEST( AutoSizeColumn ); CPPUNIT_TEST( PseudoTest_NativeHeader ); WXUISIM_TEST( LabelClick ); WXUISIM_TEST( SortClick ); CPPUNIT_TEST( ColumnOrder ); WXUISIM_TEST( ResizeScrolledHeader ); WXUISIM_TEST( ColumnMinWidth ); + WXUISIM_TEST( AutoSizeColumn ); CPPUNIT_TEST( DeleteAndAddRowCol ); CPPUNIT_TEST( PseudoTest_NativeLabels ); WXUISIM_TEST( LabelClick ); WXUISIM_TEST( SortClick ); CPPUNIT_TEST( ColumnOrder ); WXUISIM_TEST( WindowAsEditorControl ); + WXUISIM_TEST( AutoSizeColumn ); CPPUNIT_TEST_SUITE_END(); void CellEdit(); @@ -100,10 +105,25 @@ private: void WindowAsEditorControl(); void ResizeScrolledHeader(); void ColumnMinWidth(); + void AutoSizeColumn(); void PseudoTest_NativeHeader() { ms_nativeheader = true; } void PseudoTest_NativeLabels() { ms_nativeheader = false; ms_nativelabels = true; } + // The helper function to determine the width of the column label depending + // on whether the native column is used. + int GetColumnLabelWidth(wxClientDC& dc, int col, int margin) const + { + if (ms_nativeheader) + return m_grid->GetGridColHeader()->GetColumnTitleWidth(col); + + int w, h; + dc.GetMultiLineTextExtent(m_grid->GetColLabelValue(col), &w, &h); + return w + margin; + } + + void CheckFirstColAutoSize(int expected); + static bool ms_nativeheader; static bool ms_nativelabels; @@ -988,4 +1008,94 @@ void GridTestCase::ColumnMinWidth() #endif } +void GridTestCase::CheckFirstColAutoSize(int expected) +{ + m_grid->AutoSizeColumn(0); + + wxYield(); + CHECK(m_grid->GetColSize(0) == expected); +} + +void GridTestCase::AutoSizeColumn() +{ + // Hardcoded extra margin for the columns used in grid.cpp. + const int margin = 10; + + wxGridCellAttr *attr = m_grid->GetOrCreateCellAttr(0, 0); + wxGridCellRenderer *renderer = attr->GetRenderer(m_grid, 0, 0); + REQUIRE(renderer != NULL); + + wxClientDC dcCell(m_grid->GetGridWindow()); + + wxClientDC dcLabel(m_grid->GetGridWindow()); + dcLabel.SetFont(m_grid->GetLabelFont()); + + const wxString shortStr = "W"; + const wxString mediumStr = "WWWW"; + const wxString longStr = "WWWWWWWW"; + const wxString multilineStr = mediumStr + "\n" + longStr; + + SECTION("Empty column and label") + { + m_grid->SetColLabelValue(0, wxString()); + CheckFirstColAutoSize( m_grid->GetDefaultColSize() ); + } + + SECTION("Empty column with label") + { + m_grid->SetColLabelValue(0, mediumStr); + CheckFirstColAutoSize( GetColumnLabelWidth(dcLabel, 0, margin) ); + } + + SECTION("Column with empty label") + { + m_grid->SetColLabelValue(0, wxString()); + m_grid->SetCellValue(0, 0, mediumStr); + m_grid->SetCellValue(1, 0, shortStr); + m_grid->SetCellValue(3, 0, longStr); + + CheckFirstColAutoSize( + renderer->GetBestWidth(*m_grid, *attr, dcCell, 3, 0, + m_grid->GetRowHeight(3)) + margin ); + } + + SECTION("Column with label longer than contents") + { + m_grid->SetColLabelValue(0, multilineStr); + m_grid->SetCellValue(0, 0, mediumStr); + m_grid->SetCellValue(1, 0, shortStr); + CheckFirstColAutoSize( GetColumnLabelWidth(dcLabel, 0, margin) ); + } + + SECTION("Column with contents longer than label") + { + m_grid->SetColLabelValue(0, mediumStr); + m_grid->SetCellValue(0, 0, mediumStr); + m_grid->SetCellValue(1, 0, shortStr); + m_grid->SetCellValue(3, 0, multilineStr); + CheckFirstColAutoSize( + renderer->GetBestWidth(*m_grid, *attr, dcCell, 3, 0, + m_grid->GetRowHeight(3)) + margin ); + } + + SECTION("Column with equally sized contents and label") + { + m_grid->SetColLabelValue(0, mediumStr); + m_grid->SetCellValue(0, 0, mediumStr); + m_grid->SetCellValue(1, 0, mediumStr); + m_grid->SetCellValue(3, 0, mediumStr); + + const int labelWidth = GetColumnLabelWidth(dcLabel, 0, margin); + + const int cellWidth = + renderer->GetBestWidth(*m_grid, *attr, dcCell, 3, 0, + m_grid->GetRowHeight(3)) + + margin; + + // We can't be sure which size will be greater because of different fonts + // so just calculate the maximum width. + CheckFirstColAutoSize( wxMax(labelWidth, cellWidth) ); + } +} + #endif //wxUSE_GRID