From eac58e7f874a2cf3b0651257a1dba896b035b442 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 31 May 2020 16:09:29 +0200 Subject: [PATCH 01/12] Simplify wxGrid best size computations Remove needless subtraction of row/column label size before adding it back again, as this seems completely unnecessary. No real changes, this is just a simplification. --- src/generic/grid.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index f6ed381ba2..266107031a 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -10248,8 +10248,8 @@ void wxGrid::AutoSize() { wxGridUpdateLocker locker(this); - wxSize size(SetOrCalcColumnSizes(false) - m_rowLabelWidth + m_extraWidth, - SetOrCalcRowSizes(false) - m_colLabelHeight + m_extraHeight); + wxSize size(SetOrCalcColumnSizes(false) + m_extraWidth, + SetOrCalcRowSizes(false) + m_extraHeight); // we know that we're not going to have scrollbars so disable them now to // avoid trouble in SetClientSize() which can otherwise set the correct @@ -10257,7 +10257,7 @@ void wxGrid::AutoSize() SetScrollbars(m_xScrollPixelsPerLine, m_yScrollPixelsPerLine, 0, 0, 0, 0, true); - SetClientSize(size.x + m_rowLabelWidth, size.y + m_colLabelHeight); + SetClientSize(size); } void wxGrid::AutoSizeRowLabelSize( int row ) @@ -10298,11 +10298,10 @@ wxSize wxGrid::DoGetBestSize() const // we do the same as in AutoSize() here with the exception that we don't // change the column/row sizes, only calculate them - wxSize size(self->SetOrCalcColumnSizes(true) - m_rowLabelWidth + m_extraWidth, - self->SetOrCalcRowSizes(true) - m_colLabelHeight + m_extraHeight); + wxSize size(self->SetOrCalcColumnSizes(true) + m_extraWidth, + self->SetOrCalcRowSizes(true) + m_extraHeight); - return wxSize(size.x + m_rowLabelWidth, size.y + m_colLabelHeight) - + GetWindowBorderSize(); + return size + GetWindowBorderSize(); } void wxGrid::Fit() From 123e21c1810845b997b8e2aa350b6ce2d4289150 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 31 May 2020 17:46:32 +0200 Subject: [PATCH 02/12] Refactor wxGrid::SetOrCalcColumnSizes() and SetOrCalcRowSizes() Get rid of the unnecessarily complicated functions doing two quite different things depending on whether their first boolean parameter was true of false. Instead, split their body between AutoSize{Columns,Rows}() (which used to call them) and DoGetBestSize(), keeping just the part needed in each case. This is much simpler and even more efficient, as it avoids a completely unnecessary call to CalcDimensions() and Refresh() from DoGetBestSize(), which doesn't change the current size at all and so doesn't need to refresh anything, but previously did it and not only once, but twice, because both of SetOrCalc{Column,Row}Sizes() did it. --- include/wx/generic/grid.h | 11 ++------- src/generic/grid.cpp | 52 ++++++++++++--------------------------- 2 files changed, 18 insertions(+), 45 deletions(-) diff --git a/include/wx/generic/grid.h b/include/wx/generic/grid.h index 5bc3c9dfff..3411de34ee 100644 --- a/include/wx/generic/grid.h +++ b/include/wx/generic/grid.h @@ -1869,11 +1869,8 @@ public: { AutoSizeColOrRow(row, setAsMin, wxGRID_ROW); } // auto size all columns (very ineffective for big grids!) - void AutoSizeColumns( bool setAsMin = true ) - { (void)SetOrCalcColumnSizes(false, setAsMin); } - - void AutoSizeRows( bool setAsMin = true ) - { (void)SetOrCalcRowSizes(false, setAsMin); } + void AutoSizeColumns( bool setAsMin = true ); + void AutoSizeRows( bool setAsMin = true ); // auto size the grid, that is make the columns/rows of the "right" size // and also set the grid size to just fit its contents @@ -2414,10 +2411,6 @@ protected: wxColour m_gridFrozenBorderColour; int m_gridFrozenBorderPenWidth; - // common part of AutoSizeColumn/Row() and GetBestSize() - int SetOrCalcColumnSizes(bool calcOnly, bool setAsMin = true); - int SetOrCalcRowSizes(bool calcOnly, bool setAsMin = true); - // common part of AutoSizeColumn/Row() void AutoSizeColOrRow(int n, bool setAsMin, wxGridDirection direction); diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index 266107031a..181e9b57a6 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -10206,50 +10206,28 @@ wxCoord wxGrid::CalcColOrRowLabelAreaMinSize(wxGridDirection direction) return extentMax; } -int wxGrid::SetOrCalcColumnSizes(bool calcOnly, bool setAsMin) +void wxGrid::AutoSizeColumns(bool setAsMin) { - int width = m_rowLabelWidth; - - wxGridUpdateLocker locker; - if(!calcOnly) - locker.Create(this); + wxGridUpdateLocker locker(this); for ( int col = 0; col < m_numCols; col++ ) - { - if ( !calcOnly ) - AutoSizeColumn(col, setAsMin); - - width += GetColWidth(col); - } - - return width; + AutoSizeColumn(col, setAsMin); } -int wxGrid::SetOrCalcRowSizes(bool calcOnly, bool setAsMin) +void wxGrid::AutoSizeRows(bool setAsMin) { - int height = m_colLabelHeight; - - wxGridUpdateLocker locker; - if(!calcOnly) - locker.Create(this); + wxGridUpdateLocker locker(this); for ( int row = 0; row < m_numRows; row++ ) - { - if ( !calcOnly ) - AutoSizeRow(row, setAsMin); - - height += GetRowHeight(row); - } - - return height; + AutoSizeRow(row, setAsMin); } void wxGrid::AutoSize() { wxGridUpdateLocker locker(this); - wxSize size(SetOrCalcColumnSizes(false) + m_extraWidth, - SetOrCalcRowSizes(false) + m_extraHeight); + AutoSizeColumns(); + AutoSizeRows(); // we know that we're not going to have scrollbars so disable them now to // avoid trouble in SetClientSize() which can otherwise set the correct @@ -10257,7 +10235,7 @@ void wxGrid::AutoSize() SetScrollbars(m_xScrollPixelsPerLine, m_yScrollPixelsPerLine, 0, 0, 0, 0, true); - SetClientSize(size); + SetSize(DoGetBestSize()); } void wxGrid::AutoSizeRowLabelSize( int row ) @@ -10294,12 +10272,14 @@ void wxGrid::AutoSizeColLabelSize( int col ) wxSize wxGrid::DoGetBestSize() const { - wxGrid * const self = const_cast(this); + wxSize size(m_rowLabelWidth + m_extraWidth, + m_colLabelHeight + m_extraHeight); - // we do the same as in AutoSize() here with the exception that we don't - // change the column/row sizes, only calculate them - wxSize size(self->SetOrCalcColumnSizes(true) + m_extraWidth, - self->SetOrCalcRowSizes(true) + m_extraHeight); + for ( int col = 0; col < m_numCols; col++ ) + size.x += GetColWidth(col); + + for ( int row = 0; row < m_numRows; row++ ) + size.y += GetRowHeight(row); return size + GetWindowBorderSize(); } From 79d25664eb97b8422013cb67e60af07ff5af6f8e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 7 Jun 2020 17:04:49 +0200 Subject: [PATCH 03/12] Optimize wxGrid::GetBestSize() when using uniform row/column size There is no need to iterate over all rows or columns if all of them have the same size anyhow. --- src/generic/grid.cpp | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index 181e9b57a6..80d26edf90 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -10275,11 +10275,25 @@ wxSize wxGrid::DoGetBestSize() const wxSize size(m_rowLabelWidth + m_extraWidth, m_colLabelHeight + m_extraHeight); - for ( int col = 0; col < m_numCols; col++ ) - size.x += GetColWidth(col); + if ( m_colWidths.empty() ) + { + size.x += m_defaultColWidth*m_numCols; + } + else + { + for ( int col = 0; col < m_numCols; col++ ) + size.x += GetColWidth(col); + } - for ( int row = 0; row < m_numRows; row++ ) - size.y += GetRowHeight(row); + if ( m_rowHeights.empty() ) + { + size.y += m_defaultRowHeight*m_numRows; + } + else + { + for ( int row = 0; row < m_numRows; row++ ) + size.y += GetRowHeight(row); + } return size + GetWindowBorderSize(); } From 09ecfaec8fa06e2f7ebd7aa62c0c5a7dd4b5df16 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 10 Jun 2020 22:57:17 +0200 Subject: [PATCH 04/12] Reuse wxDC::GetMultiLineTextExtent() in wxGridCellStringRenderer There is no need to reimplement the same logic in wxGrid code when we already have a perfectly cromulent way to do it in wxDC itself (which hadn't existed when this code was originally written, explaining why it wasn't used here before). This makes the code shorter and also a bit faster, as we avoid using wxStringTokenizer unnecessarily. No changes in behaviour. --- src/generic/gridctrl.cpp | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/generic/gridctrl.cpp b/src/generic/gridctrl.cpp index baf5937796..f40acbf0fd 100644 --- a/src/generic/gridctrl.cpp +++ b/src/generic/gridctrl.cpp @@ -557,18 +557,8 @@ wxSize wxGridCellStringRenderer::DoGetBestSize(const wxGridCellAttr& attr, wxDC& dc, const wxString& text) { - wxCoord x = 0, y = 0, max_x = 0; dc.SetFont(attr.GetFont()); - wxStringTokenizer tk(text, wxT('\n')); - while ( tk.HasMoreTokens() ) - { - dc.GetTextExtent(tk.GetNextToken(), &x, &y); - max_x = wxMax(max_x, x); - } - - y *= 1 + text.Freq(wxT('\n')); // multiply by the number of lines. - - return wxSize(max_x, y); + return dc.GetMultiLineTextExtent(text); } wxSize wxGridCellStringRenderer::GetBestSize(wxGrid& grid, From bfeae1922dc13484f57b42567cff56173e4e6621 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 10 Jun 2020 23:36:03 +0200 Subject: [PATCH 05/12] Minor optimizations in GetMultiLineTextExtent() Handle the case of a single line string specially, it's faster to do this than use slow loop over all characters. Also avoid constructing the string with the characters to measure one by one and do it all at once instead. Add a possibility to benchmark GetMultiLineTextExtent() rather than GetTextExtent() in the graphics benchmark. --- src/common/textmeasurecmn.cpp | 23 +++++++++++++++-------- tests/benchmarks/graphics.cpp | 9 ++++++++- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/common/textmeasurecmn.cpp b/src/common/textmeasurecmn.cpp index 8d828d4b71..16448d2664 100644 --- a/src/common/textmeasurecmn.cpp +++ b/src/common/textmeasurecmn.cpp @@ -105,17 +105,27 @@ void wxTextMeasureBase::GetMultiLineTextExtent(const wxString& text, wxCoord *height, wxCoord *heightOneLine) { + // It's noticeably faster to handle the case of a string which isn't + // actually multiline specially here, to skip iteration above in this case. + if ( text.find('\n') == wxString::npos ) + { + GetTextExtent(text, width, height); + if ( heightOneLine && height ) + *heightOneLine = *height; + return; + } + MeasuringGuard guard(*this); wxCoord widthTextMax = 0, widthLine, heightTextTotal = 0, heightLineDefault = 0, heightLine = 0; - wxString curLine; + wxString::const_iterator lineStart = text.begin(); for ( wxString::const_iterator pc = text.begin(); ; ++pc ) { if ( pc == text.end() || *pc == wxS('\n') ) { - if ( curLine.empty() ) + if ( pc == lineStart ) { // we can't use GetTextExtent - it will return 0 for both width // and height and an empty line should count in height @@ -137,7 +147,7 @@ void wxTextMeasureBase::GetMultiLineTextExtent(const wxString& text, } else { - CallGetTextExtent(curLine, &widthLine, &heightLine); + CallGetTextExtent(wxString(lineStart, pc), &widthLine, &heightLine); if ( widthLine > widthTextMax ) widthTextMax = widthLine; heightTextTotal += heightLine; @@ -149,13 +159,10 @@ void wxTextMeasureBase::GetMultiLineTextExtent(const wxString& text, } else // '\n' { - curLine.clear(); + lineStart = pc; + ++lineStart; } } - else - { - curLine += *pc; - } } if ( width ) diff --git a/tests/benchmarks/graphics.cpp b/tests/benchmarks/graphics.cpp index e2714de5de..bbbec0cd5a 100644 --- a/tests/benchmarks/graphics.cpp +++ b/tests/benchmarks/graphics.cpp @@ -66,6 +66,7 @@ struct GraphicsBenchmarkOptions testCircles = testEllipses = testTextExtent = + testMultiLineTextExtent = testPartialTextExtents = false; usePaint = @@ -95,6 +96,7 @@ struct GraphicsBenchmarkOptions testCircles, testEllipses, testTextExtent, + testMultiLineTextExtent, testPartialTextExtents; bool usePaint, @@ -634,7 +636,10 @@ private: wxStopWatch sw; for ( long n = 0; n < opts.numIters; n++ ) { - size += dc.GetTextExtent(str); + if ( opts.testMultiLineTextExtent ) + size += dc.GetMultiLineTextExtent(str); + else + size += dc.GetTextExtent(str); } const long t = sw.Time(); @@ -847,6 +852,7 @@ public: { wxCMD_LINE_SWITCH, "", "circles" }, { wxCMD_LINE_SWITCH, "", "ellipses" }, { wxCMD_LINE_SWITCH, "", "textextent" }, + { wxCMD_LINE_SWITCH, "", "multilinetextextent" }, { wxCMD_LINE_SWITCH, "", "partialtextextents" }, { wxCMD_LINE_SWITCH, "", "paint" }, { wxCMD_LINE_SWITCH, "", "client" }, @@ -922,6 +928,7 @@ public: opts.testCircles = parser.Found("circles"); opts.testEllipses = parser.Found("ellipses"); opts.testTextExtent = parser.Found("textextent"); + opts.testMultiLineTextExtent = parser.Found("multilinetextextent"); opts.testPartialTextExtents = parser.Found("partialtextextents"); if ( !(opts.testBitmaps || opts.testImages || opts.testLines || opts.testRawBitmaps || opts.testRectangles From 70768a33d2f7bc8dc01f1a1e8ecea8a2aeb1f550 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 10 Jun 2020 23:46:13 +0200 Subject: [PATCH 06/12] Dramatically speed up measuring text extent in wxMSW Skip correction for the under/overhang for non-italic fonts: it seems to be pretty small for them and avoiding the calls to ::GetCharABCWidths() makes GetTextExtent() 7-8 times faster, which seems to be a worthwhile compensation. If we decide to restore these calls in the future, we will need to implement some kind of cache for them, as they're just too slow otherwise. --- src/msw/textmeasure.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/msw/textmeasure.cpp b/src/msw/textmeasure.cpp index 7b68982d27..624dc520c8 100644 --- a/src/msw/textmeasure.cpp +++ b/src/msw/textmeasure.cpp @@ -108,7 +108,12 @@ void wxTextMeasure::DoGetTextExtent(const wxString& string, // the result computed by GetTextExtentPoint32() may be too small as it // accounts for under/overhang of the first/last character while we want // just the bounding rect for this string so adjust the width as needed - if ( len > 0 ) + // when using italic fonts as the difference is really noticeable for them + // (it may still exist, but seems to be at most 1px for the other fonts, + // and calling GetCharABCWidths() is pretty slow and much slower than + // calling GetTextExtentPoint32() itself, so avoid its overhead unless it's + // really, really necessary). + if ( GetFont().GetStyle() != wxFONTSTYLE_NORMAL && len > 0 ) { ABC widthABC; const wxChar chFirst = *string.begin(); From 249db04dd33f35a1af79f26c2c372d4b0520f242 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 11 Jun 2020 09:43:08 +0200 Subject: [PATCH 07/12] Add wxGridTableBase::CanMeasureColUsingSameAttr() This allows to optimize AutoSizeColumns() in the common case when all cells in the same column can be measured using the same attribute. --- include/wx/generic/grid.h | 10 ++++++++++ interface/wx/grid.h | 19 +++++++++++++++++++ src/generic/grid.cpp | 16 ++++++++++++++-- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/include/wx/generic/grid.h b/include/wx/generic/grid.h index 3411de34ee..6ebf09a1d0 100644 --- a/include/wx/generic/grid.h +++ b/include/wx/generic/grid.h @@ -1065,6 +1065,16 @@ public: return wxGridCellAttrPtr(GetAttr(row, col, kind)); } + // This is an optimization for a common case when the entire column uses + // roughly the same attribute, which can thus be reused for measuring all + // the cells in this column. Override this to return true (possibly for + // some columns only) to speed up AutoSizeColumns() for the grids using + // this table. + virtual bool CanMeasureColUsingSameAttr(int WXUNUSED(col)) const + { + return false; + } + // these functions take ownership of the pointer virtual void SetAttr(wxGridCellAttr* attr, int row, int col); virtual void SetRowAttr(wxGridCellAttr *attr, int row); diff --git a/interface/wx/grid.h b/interface/wx/grid.h index 33edf24f5f..c15e466735 100644 --- a/interface/wx/grid.h +++ b/interface/wx/grid.h @@ -2517,6 +2517,25 @@ public: returns @true. */ virtual bool CanHaveAttributes(); + + /** + Override to return true if the same attribute can be used for measuring + all cells in the given column. + + This function is provided for optimization purposes: it returns @false + by default, but can be overridden to return @true when all the cells in + the same grid column use sensibly the same attribute, i.e. they use the + same renderer (either explicitly, or implicitly, due to their type as + returned by GetTypeName()) and the font of the same size. + + Returning @true from this function allows AutoSizeColumns() to skip + looking up the attribute and the renderer for each individual cell, + which results in very noticeable performance improvements for the grids + with many rows. + + @since 3.1.4 + */ + virtual bool CanMeasureColUsingSameAttr(int col) const; }; diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index 80d26edf90..ec102e3f69 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -9967,6 +9967,14 @@ wxGrid::AutoSizeColOrRow(int colOrRow, bool setAsMin, wxGridDirection direction) col = -1; } + // If possible, reuse the same attribute and renderer for all cells: this + // is an important optimization (resulting in up to 80% speed up of + // AutoSizeColumns()) as finding the attribute and renderer for the cell + // are very slow operations, due to the number of steps involved in them. + const bool canReuseAttr = column && m_table->CanMeasureColUsingSameAttr(col); + wxGridCellAttrPtr attr; + wxGridCellRendererPtr renderer; + wxCoord extent, extentMax = 0; int max = column ? m_numRows : m_numCols; for ( int rowOrCol = 0; rowOrCol < max; rowOrCol++ ) @@ -10005,8 +10013,12 @@ wxGrid::AutoSizeColOrRow(int colOrRow, bool setAsMin, wxGridDirection direction) } // get cell ( main cell if CellSpan_Inside ) renderer best size - wxGridCellAttrPtr attr = GetCellAttrPtr(row, col); - wxGridCellRendererPtr renderer = attr->GetRendererPtr(this, row, col); + if ( !canReuseAttr || !attr ) + { + attr = GetCellAttrPtr(row, col); + renderer = attr->GetRendererPtr(this, row, col); + } + if ( renderer ) { extent = column From 71d42a8290ee2b68bded6813cede6ee5529e6b86 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 13 Jun 2020 15:51:20 +0200 Subject: [PATCH 08/12] Add wxGridCellRenderer::GetMaxBestSize() This is another optimization, useful for the renderers that are used with the values of a fixed form or part of a limited set, as it is much faster to compute the best size for all values of the set rather than computing them for all the cells in the column. --- include/wx/generic/grid.h | 14 ++++++++++++ include/wx/generic/gridctrl.h | 12 ++++++++++ interface/wx/grid.h | 20 ++++++++++++++++ src/generic/grid.cpp | 15 ++++++++++++ src/generic/gridctrl.cpp | 43 +++++++++++++++++++++++++++++++++-- 5 files changed, 102 insertions(+), 2 deletions(-) diff --git a/include/wx/generic/grid.h b/include/wx/generic/grid.h index 6ebf09a1d0..948aba4d0b 100644 --- a/include/wx/generic/grid.h +++ b/include/wx/generic/grid.h @@ -212,6 +212,20 @@ public: return GetBestSize(grid, attr, dc, row, col).GetWidth(); } + + // Unlike GetBestSize(), this functions is optional: it is used when + // auto-sizing columns to determine the best width without iterating over + // all cells in this column, if possible. + // + // If it isn't, return wxDefaultSize as the base class version does by + // default. + virtual wxSize GetMaxBestSize(wxGrid& WXUNUSED(grid), + wxGridCellAttr& WXUNUSED(attr), + wxDC& WXUNUSED(dc)) + { + return wxDefaultSize; + } + // create a new object which is the copy of this one virtual wxGridCellRenderer *Clone() const = 0; }; diff --git a/include/wx/generic/gridctrl.h b/include/wx/generic/gridctrl.h index c06937f0b2..68e3f08323 100644 --- a/include/wx/generic/gridctrl.h +++ b/include/wx/generic/gridctrl.h @@ -141,6 +141,10 @@ public: wxDC& dc, int row, int col) wxOVERRIDE; + virtual wxSize GetMaxBestSize(wxGrid& grid, + wxGridCellAttr& attr, + wxDC& dc) wxOVERRIDE; + virtual wxGridCellRenderer *Clone() const wxOVERRIDE { return new wxGridCellBoolRenderer; } }; @@ -175,6 +179,10 @@ public: wxDC& dc, int row, int col) wxOVERRIDE; + virtual wxSize GetMaxBestSize(wxGrid& grid, + wxGridCellAttr& attr, + wxDC& dc) wxOVERRIDE; + virtual wxGridCellRenderer *Clone() const wxOVERRIDE; // output strptime()-like format string @@ -232,6 +240,10 @@ public: wxDC& dc, int row, int col) wxOVERRIDE; + virtual wxSize GetMaxBestSize(wxGrid& grid, + wxGridCellAttr& attr, + wxDC& dc) wxOVERRIDE; + virtual wxGridCellRenderer *Clone() const wxOVERRIDE; // parameters string format is "item1[,item2[...,itemN]]" where itemN will diff --git a/interface/wx/grid.h b/interface/wx/grid.h index c15e466735..24d2dc8e0e 100644 --- a/interface/wx/grid.h +++ b/interface/wx/grid.h @@ -88,6 +88,26 @@ public: virtual int GetBestWidth(wxGrid& grid, wxGridCellAttr& attr, wxDC& dc, int row, int col, int height); + /** + Get the maximum possible size for a cell using this renderer, if + possible. + + This function may be overridden in the derived class if it can return + the maximum size needed for displaying the cells rendered it without + iterating over all cells. The base class version simply returns + ::wxDefaultSize, indicating that this is infeasible and that + GetBestSize() should be called for each cell individually. + + Note that this method will only be used if + wxGridTableBase::CanMeasureColUsingSameAttr() is overriden to return + @true. + + @since 3.1.4 + */ + virtual wxSize GetMaxBestSize(wxGrid& grid, + wxGridCellAttr& attr, + wxDC& dc); + protected: /** The destructor is private because only DecRef() can delete us. diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index ec102e3f69..cbf2658f7b 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -10017,6 +10017,21 @@ wxGrid::AutoSizeColOrRow(int colOrRow, bool setAsMin, wxGridDirection direction) { attr = GetCellAttrPtr(row, col); renderer = attr->GetRendererPtr(this, row, col); + + if ( canReuseAttr ) + { + // Try to get the best width for the entire column at once, if + // it's supported by the renderer. + extent = renderer->GetMaxBestSize(*this, *attr, dc).x; + + if ( extent != wxDefaultCoord ) + { + extentMax = extent; + + // No need to check all the values. + break; + } + } } if ( renderer ) diff --git a/src/generic/gridctrl.cpp b/src/generic/gridctrl.cpp index f40acbf0fd..bb9e3dd12c 100644 --- a/src/generic/gridctrl.cpp +++ b/src/generic/gridctrl.cpp @@ -165,6 +165,24 @@ wxSize wxGridCellDateRenderer::GetBestSize(wxGrid& grid, return DoGetBestSize(attr, dc, GetString(grid, row, col)); } +wxSize wxGridCellDateRenderer::GetMaxBestSize(wxGrid& WXUNUSED(grid), + wxGridCellAttr& attr, + wxDC& dc) +{ + wxSize size; + + // Try to produce the longest string in the current format: as we don't + // know which month is the longest, we need to try all of them. + for ( int m = wxDateTime::Jan; m <= wxDateTime::Dec; ++m ) + { + const wxDateTime d(28, static_cast(m), 9999); + + size.IncTo(DoGetBestSize(attr, dc, d.Format(m_oformat, m_tz))); + } + + return size; +} + void wxGridCellDateRenderer::SetParameters(const wxString& params) { if (!params.empty()) @@ -260,6 +278,20 @@ wxSize wxGridCellEnumRenderer::GetBestSize(wxGrid& grid, return DoGetBestSize(attr, dc, GetString(grid, row, col)); } +wxSize wxGridCellEnumRenderer::GetMaxBestSize(wxGrid& WXUNUSED(grid), + wxGridCellAttr& attr, + wxDC& dc) +{ + wxSize size; + + for ( size_t n = 0; n < m_choices.size(); ++n ) + { + size.IncTo(DoGetBestSize(attr, dc, m_choices[n])); + } + + return size; +} + void wxGridCellEnumRenderer::SetParameters(const wxString& params) { if ( !params ) @@ -912,10 +944,17 @@ void wxGridCellFloatRenderer::SetParameters(const wxString& params) // ---------------------------------------------------------------------------- wxSize wxGridCellBoolRenderer::GetBestSize(wxGrid& grid, - wxGridCellAttr& WXUNUSED(attr), - wxDC& WXUNUSED(dc), + wxGridCellAttr& attr, + wxDC& dc, int WXUNUSED(row), int WXUNUSED(col)) +{ + return GetMaxBestSize(grid, attr, dc); +} + +wxSize wxGridCellBoolRenderer::GetMaxBestSize(wxGrid& grid, + wxGridCellAttr& WXUNUSED(attr), + wxDC& WXUNUSED(dc)) { static wxPrivate::DpiDependentValue s_sizeCheckMark; From 96de24d1bb4cf61d6accaccdcb338ce4d660d770 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 13 Jun 2020 16:10:14 +0200 Subject: [PATCH 09/12] Add wxGridCellChoiceRenderer to optimize column size calculation Previously columns using a set of predetermined values used plain wxGridCellStringRenderer, which didn't allow to determine their best size efficiently, as wxGrid had to iterate over all the rows of the table, even if they only took a couple of possible values. Add wxGridCellChoiceRenderer (refactoring wxGridCellEnumRenderer to extract the common code from it in the process) which implements GetMaxBestSize() by just finding the best size of all of these values, which is much faster for large grids. This does result in a change in behaviour, as the column now adapts to its "theoretical" best size and not just the size of the values actually shown in it, but this seems to be a worthwhile trade-off and could even be seen as an advantage, as editing a cell won't make its value overflow the auto-sized column width any more, as it is wide enough to show any of the column values. --- include/wx/generic/gridctrl.h | 41 ++++++++++++++++------ src/generic/grid.cpp | 2 +- src/generic/gridctrl.cpp | 64 +++++++++++++++++------------------ 3 files changed, 63 insertions(+), 44 deletions(-) diff --git a/include/wx/generic/gridctrl.h b/include/wx/generic/gridctrl.h index 68e3f08323..0991110ba8 100644 --- a/include/wx/generic/gridctrl.h +++ b/include/wx/generic/gridctrl.h @@ -221,8 +221,37 @@ protected: #endif // wxUSE_DATETIME +// Renderer for fields taking one of a limited set of values: this is the same +// as the renderer for strings, except that it can implement GetMaxBestSize(). +class WXDLLIMPEXP_ADV wxGridCellChoiceRenderer : public wxGridCellStringRenderer +{ +public: + wxGridCellChoiceRenderer() { } + + virtual wxSize GetMaxBestSize(wxGrid& grid, + wxGridCellAttr& attr, + wxDC& dc) wxOVERRIDE; + + // Parameters string is a comma-separated list of values. + virtual void SetParameters(const wxString& params) wxOVERRIDE; + + virtual wxGridCellRenderer *Clone() const wxOVERRIDE + { + return new wxGridCellChoiceRenderer(*this); + } + +protected: + wxGridCellChoiceRenderer(const wxGridCellChoiceRenderer& other) + : m_choices(other.m_choices) + { + } + + wxArrayString m_choices; +}; + + // renders a number using the corresponding text string -class WXDLLIMPEXP_ADV wxGridCellEnumRenderer : public wxGridCellStringRenderer +class WXDLLIMPEXP_ADV wxGridCellEnumRenderer : public wxGridCellChoiceRenderer { public: wxGridCellEnumRenderer( const wxString& choices = wxEmptyString ); @@ -240,20 +269,10 @@ public: wxDC& dc, int row, int col) wxOVERRIDE; - virtual wxSize GetMaxBestSize(wxGrid& grid, - wxGridCellAttr& attr, - wxDC& dc) wxOVERRIDE; - virtual wxGridCellRenderer *Clone() const wxOVERRIDE; - // parameters string format is "item1[,item2[...,itemN]]" where itemN will - // be used if the cell value is N-1 - virtual void SetParameters(const wxString& params) wxOVERRIDE; - protected: wxString GetString(const wxGrid& grid, int row, int col); - - wxArrayString m_choices; }; diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index cbf2658f7b..ec8aeaddbc 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -11017,7 +11017,7 @@ int wxGridTypeRegistry::FindDataType(const wxString& typeName) if ( typeName == wxGRID_VALUE_CHOICE ) { RegisterDataType(wxGRID_VALUE_CHOICE, - new wxGridCellStringRenderer, + new wxGridCellChoiceRenderer, new wxGridCellChoiceEditor); } else diff --git a/src/generic/gridctrl.cpp b/src/generic/gridctrl.cpp index bb9e3dd12c..4f2860912c 100644 --- a/src/generic/gridctrl.cpp +++ b/src/generic/gridctrl.cpp @@ -212,6 +212,38 @@ bool wxGridCellDateTimeRenderer::Parse(const wxString& text, wxDateTime& result) #endif // wxUSE_DATETIME +// ---------------------------------------------------------------------------- +// wxGridCellChoiceRenderer +// ---------------------------------------------------------------------------- + +wxSize wxGridCellChoiceRenderer::GetMaxBestSize(wxGrid& WXUNUSED(grid), + wxGridCellAttr& attr, + wxDC& dc) +{ + wxSize size; + + for ( size_t n = 0; n < m_choices.size(); ++n ) + { + size.IncTo(DoGetBestSize(attr, dc, m_choices[n])); + } + + return size; +} + +void wxGridCellChoiceRenderer::SetParameters(const wxString& params) +{ + m_choices.clear(); + + if ( params.empty() ) + return; + + wxStringTokenizer tk(params, wxT(',')); + while ( tk.HasMoreTokens() ) + { + m_choices.Add(tk.GetNextToken()); + } +} + // ---------------------------------------------------------------------------- // wxGridCellEnumRenderer // ---------------------------------------------------------------------------- @@ -278,38 +310,6 @@ wxSize wxGridCellEnumRenderer::GetBestSize(wxGrid& grid, return DoGetBestSize(attr, dc, GetString(grid, row, col)); } -wxSize wxGridCellEnumRenderer::GetMaxBestSize(wxGrid& WXUNUSED(grid), - wxGridCellAttr& attr, - wxDC& dc) -{ - wxSize size; - - for ( size_t n = 0; n < m_choices.size(); ++n ) - { - size.IncTo(DoGetBestSize(attr, dc, m_choices[n])); - } - - return size; -} - -void wxGridCellEnumRenderer::SetParameters(const wxString& params) -{ - if ( !params ) - { - // what can we do? - return; - } - - m_choices.Empty(); - - wxStringTokenizer tk(params, wxT(',')); - while ( tk.HasMoreTokens() ) - { - m_choices.Add(tk.GetNextToken()); - } -} - - // ---------------------------------------------------------------------------- // wxGridCellAutoWrapStringRenderer // ---------------------------------------------------------------------------- From d2a403408f82de46eacb0479fc051d4b5484cb01 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 13 Jun 2020 16:31:33 +0200 Subject: [PATCH 10/12] Implement wxGridCellNumberRenderer::GetMaxBestSize() This allows to make computing the best width of numeric columns an O(1) operation instead of O(number-of-rows), which can make a huge difference for big grids. --- include/wx/generic/gridctrl.h | 19 ++++++++++++++++++- src/generic/gridctrl.cpp | 28 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/include/wx/generic/gridctrl.h b/include/wx/generic/gridctrl.h index 0991110ba8..167201132c 100644 --- a/include/wx/generic/gridctrl.h +++ b/include/wx/generic/gridctrl.h @@ -57,6 +57,13 @@ protected: class WXDLLIMPEXP_ADV wxGridCellNumberRenderer : public wxGridCellStringRenderer { public: + explicit wxGridCellNumberRenderer(long minValue = LONG_MIN, + long maxValue = LONG_MAX) + : m_minValue(minValue), + m_maxValue(maxValue) + { + } + // draw the string right aligned virtual void Draw(wxGrid& grid, wxGridCellAttr& attr, @@ -70,11 +77,21 @@ public: wxDC& dc, int row, int col) wxOVERRIDE; + virtual wxSize GetMaxBestSize(wxGrid& grid, + wxGridCellAttr& attr, + wxDC& dc) wxOVERRIDE; + + // Optional parameters for this renderer are ",". + virtual void SetParameters(const wxString& params) wxOVERRIDE; + virtual wxGridCellRenderer *Clone() const wxOVERRIDE - { return new wxGridCellNumberRenderer; } + { return new wxGridCellNumberRenderer(m_minValue, m_maxValue); } protected: wxString GetString(const wxGrid& grid, int row, int col); + + long m_minValue, + m_maxValue; }; class WXDLLIMPEXP_ADV wxGridCellFloatRenderer : public wxGridCellStringRenderer diff --git a/src/generic/gridctrl.cpp b/src/generic/gridctrl.cpp index 4f2860912c..df2f831205 100644 --- a/src/generic/gridctrl.cpp +++ b/src/generic/gridctrl.cpp @@ -748,6 +748,34 @@ wxSize wxGridCellNumberRenderer::GetBestSize(wxGrid& grid, return DoGetBestSize(attr, dc, GetString(grid, row, col)); } +wxSize wxGridCellNumberRenderer::GetMaxBestSize(wxGrid& WXUNUSED(grid), + wxGridCellAttr& attr, + wxDC& dc) +{ + // In theory, it's possible that there is a value in min..max range which + // is longer than both min and max, e.g. we could conceivably have "88" be + // wider than both "87" and "91" with some fonts, but it seems something + // too exotic to worry about in practice. + wxSize size = DoGetBestSize(attr, dc, wxString::Format("%ld", m_minValue)); + size.IncTo(DoGetBestSize(attr, dc, wxString::Format("%ld", m_maxValue))); + + return size; +} + +void wxGridCellNumberRenderer::SetParameters(const wxString& params) +{ + if ( params.empty() ) + return; + + wxString maxStr; + const wxString minStr = params.BeforeFirst(',', &maxStr); + + if ( !minStr.ToLong(&m_minValue) || !maxStr.ToLong(&m_maxValue) ) + { + wxLogDebug("Invalid wxGridCellNumberRenderer parameters \"%s\"", params); + } +} + // ---------------------------------------------------------------------------- // wxGridCellFloatRenderer // ---------------------------------------------------------------------------- From 585ed986a24f33d792b6755a9c43ed2c3ffa6954 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 13 Jun 2020 17:29:20 +0200 Subject: [PATCH 11/12] Make wxGridCellChoiceEditor big enough in both directions Previously, we ensured that the combobox was tall enough, but not that it was wide enough, which could result in truncation of its contents if the cell rectangle was too small. Fix this by increasing the size in both directions, if necessary. Also make the code simpler by using higher-level wxSize and wxRect methods instead of fiddling with the coordinates directly. --- src/generic/grideditors.cpp | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/generic/grideditors.cpp b/src/generic/grideditors.cpp index 9ac3da7786..ae7eff24ae 100644 --- a/src/generic/grideditors.cpp +++ b/src/generic/grideditors.cpp @@ -1451,22 +1451,12 @@ void wxGridCellChoiceEditor::SetSize(const wxRect& rect) wxASSERT_MSG(m_control, wxT("The wxGridCellChoiceEditor must be created first!")); - // Check that the height is not too small to fit the combobox. - wxRect rectTallEnough = rect; - const wxSize bestSize = m_control->GetBestSize(); - const wxCoord diffY = bestSize.GetHeight() - rectTallEnough.GetHeight(); - if ( diffY > 0 ) - { - // Do make it tall enough. - rectTallEnough.height += diffY; + // Check that the rectangle is big enough to fit the combobox, we can't + // afford truncating it. + wxSize size = rect.GetSize(); + size.IncTo(m_control->GetBestSize()); - // Also centre the effective rectangle vertically with respect to the - // original one. - rectTallEnough.y -= diffY/2; - } - //else: The rectangle provided is already tall enough. - - wxGridCellEditor::SetSize(rectTallEnough); + wxGridCellEditor::SetSize(wxRect(size).CentreIn(rect)); } void wxGridCellChoiceEditor::PaintBackground(wxDC& dc, From 14aa0f9e9fedb438bfecc1fda0c244abc4beafb0 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 13 Jun 2020 23:45:51 +0200 Subject: [PATCH 12/12] Check for valid font in wxTextMeasure before using it It turns out that wxEnhMetaFileDC doesn't have any valid font in wxMSW, so the changes of 70768a33d2 (Dramatically speed up measuring text extent in wxMSW, 2020-06-10) broke its GetTextExtent(). Fix this by checking if we have a valid font explicitly, although perhaps a better fix would be to ensure that wxEnhMetaFileDC also always has a default font, as the other wxDC classes. --- src/msw/textmeasure.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/msw/textmeasure.cpp b/src/msw/textmeasure.cpp index 624dc520c8..31133f9778 100644 --- a/src/msw/textmeasure.cpp +++ b/src/msw/textmeasure.cpp @@ -113,7 +113,8 @@ void wxTextMeasure::DoGetTextExtent(const wxString& string, // and calling GetCharABCWidths() is pretty slow and much slower than // calling GetTextExtentPoint32() itself, so avoid its overhead unless it's // really, really necessary). - if ( GetFont().GetStyle() != wxFONTSTYLE_NORMAL && len > 0 ) + const wxFont font = GetFont(); + if ( font.IsOk() && font.GetStyle() != wxFONTSTYLE_NORMAL && len > 0 ) { ABC widthABC; const wxChar chFirst = *string.begin();