Really fix recent regression in grid content autosizing

Fix autosizing broken in 3c72396a36 and
not fully fixed by f7e335c031.

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 <the_siv@mail.ru>

src/generic/grid.cpp

@@ -9397,51 +9397,66 @@ 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.
+            // Note that GetColumnTitleWidth already adds margins internally,
-            addMargin = false;
+            // so we don't need to add them here.
-            h = 0;
         }
         else
         {
-            dc.GetMultiLineTextExtent( GetColLabelValue(colOrRow), &w, &h );
+            const wxSize
-            if ( GetColLabelTextOrientation() == wxVERTICAL )
+                size = dc.GetMultiLineTextExtent(GetColLabelValue(colOrRow));
-                w = h;
+            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
+        // Special case: all the cells are empty, use the label extent.
-        // than default extent but != 0, it's OK)
+        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 )
-    {
-        // leave some space around text
-        if ( column )
-            extentMax += 10;
-        else
-            extentMax += 6;
     }
+    else // We have some data in the column cells.
+    {
+        // 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 )
     {

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