From f478a4f6008be939ae9141ff7a31ab9b8d790975 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Tue, 30 Jun 2026 19:26:56 +0200 Subject: [PATCH] speedup ScTable::SetDrawPageSize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit which calls GetColOffset() with a very large column value, which causes it to loop, repeatedly searching the hidden-columns data-structure. With large spreadsheets, this becomes expensive. Change-Id: If1a1a7f85e2f36273e2de8da41be2756f52be8a5 Signed-off-by: Noel Grandin Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/5764 Reviewed-by: Mike Kaganski Reviewed-by: Caolán McNamara Tested-by: Jenkins CPCI (cherry picked from commit 86a8a9ec70c039a493986cf78abaa17882f49bd7) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/207624 Tested-by: Jenkins Reviewed-by: Xisco Fauli --- sc/inc/FilterData.hxx | 2 ++ sc/inc/segmenttree.hxx | 12 ++++++- sc/source/core/data/segmenttree.cxx | 31 ++++++++++++++++- sc/source/core/data/table2.cxx | 54 +++++++++++++++++++++++++---- 4 files changed, 90 insertions(+), 9 deletions(-) diff --git a/sc/inc/FilterData.hxx b/sc/inc/FilterData.hxx index 82e161738087..6dbbfd7d854b 100644 --- a/sc/inc/FilterData.hxx +++ b/sc/inc/FilterData.hxx @@ -84,6 +84,8 @@ public: SCROW countVisibleRows(SCROW nStartRow, SCROW nEndRow) const; SCCOL countVisibleCols(SCCOL nStartCol, SCCOL nEndCol) const; SCCOLROW lastHiddenColRow(SCCOLROW nPos, bool bCol) const; + + const ScFlatBoolColSegments& getHiddenCols() const { return *mpHiddenCols; } }; /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/inc/segmenttree.hxx b/sc/inc/segmenttree.hxx index c980f9db56a0..24afc688e325 100644 --- a/sc/inc/segmenttree.hxx +++ b/sc/inc/segmenttree.hxx @@ -94,13 +94,23 @@ public: SCCOL mnCol2; bool mbValue; }; + class RangeIterator + { + public: + explicit RangeIterator(ScFlatBoolColSegments const& rSegs); + bool getFirst(RangeData& rRange); + bool getNext(RangeData& rRange); + + private: + ScFlatBoolColSegments const& mrSegs; + }; ScFlatBoolColSegments(SCCOL nMaxCol); ScFlatBoolColSegments(const ScFlatBoolColSegments& r); ~ScFlatBoolColSegments(); bool setTrue(SCCOL nCol1, SCCOL nCol2); bool setFalse(SCCOL nCol1, SCCOL nCol2); - bool getRangeData(SCCOL nCol, RangeData& rData); + bool getRangeData(SCCOL nCol, RangeData& rData) const; void removeSegment(SCCOL nCol1, SCCOL nCol2); void insertSegment(SCCOL nCol, SCCOL nSize); diff --git a/sc/source/core/data/segmenttree.cxx b/sc/source/core/data/segmenttree.cxx index e3962aa510fc..9bb121cb9e45 100644 --- a/sc/source/core/data/segmenttree.cxx +++ b/sc/source/core/data/segmenttree.cxx @@ -506,6 +506,35 @@ OString ScFlatBoolRowSegments::dumpAsString() return aOutput.makeStringAndClear(); } +ScFlatBoolColSegments::RangeIterator::RangeIterator(ScFlatBoolColSegments const & rSegs) : + mrSegs(rSegs) +{ +} + +bool ScFlatBoolColSegments::RangeIterator::getFirst(RangeData& rRange) +{ + ScFlatBoolSegmentsImpl::RangeData aData; + if (!mrSegs.mpImpl->getFirst(aData)) + return false; + + rRange.mnCol1 = static_cast(aData.mnPos1); + rRange.mnCol2 = static_cast(aData.mnPos2); + rRange.mbValue = static_cast(aData.mnValue); + return true; +} + +bool ScFlatBoolColSegments::RangeIterator::getNext(RangeData& rRange) +{ + ScFlatBoolSegmentsImpl::RangeData aData; + if (!mrSegs.mpImpl->getNext(aData)) + return false; + + rRange.mnCol1 = static_cast(aData.mnPos1); + rRange.mnCol2 = static_cast(aData.mnPos2); + rRange.mbValue = static_cast(aData.mnValue); + return true; +} + ScFlatBoolColSegments::ScFlatBoolColSegments(SCCOL nMaxCol) : mpImpl(new ScFlatBoolSegmentsImpl(nMaxCol)) { @@ -530,7 +559,7 @@ bool ScFlatBoolColSegments::setFalse(SCCOL nCol1, SCCOL nCol2) return mpImpl->setFalse(static_cast(nCol1), static_cast(nCol2)); } -bool ScFlatBoolColSegments::getRangeData(SCCOL nCol, RangeData& rData) +bool ScFlatBoolColSegments::getRangeData(SCCOL nCol, RangeData& rData) const { ScFlatBoolSegmentsImpl::RangeData aData; if (!mpImpl->getRangeData(static_cast(nCol), aData)) diff --git a/sc/source/core/data/table2.cxx b/sc/source/core/data/table2.cxx index ed4b8d750455..cc432391a5d8 100644 --- a/sc/source/core/data/table2.cxx +++ b/sc/source/core/data/table2.cxx @@ -4494,17 +4494,57 @@ SCROW ScTable::GetRowForHeightPixels(SCROW nStartRow, tools::Long& rStartRowHeig tools::Long ScTable::GetColOffset( SCCOL nCol, bool bHiddenAsZero ) const { - tools::Long n = 0; - if ( mpColWidth ) + if ( !mpColWidth ) { - auto colWidthIt = mpColWidth->begin(); - for (SCCOL i = 0; i < nCol; (++i < nCol) ? ++colWidthIt : (void)false) - if (!( bHiddenAsZero && ColHidden(i) )) - n += *colWidthIt; + OSL_FAIL("GetColumnOffset: Data missing"); + return 0; + } + tools::Long n = 0; + if (bHiddenAsZero) + { + // Walk the two data structures in parallel, for performance. + size_t nColWidthIndex; + SCCOL nColWidthRangeEnd; + sal_uInt16 nColWidth = mpColWidth->GetValue(0, nColWidthIndex, nColWidthRangeEnd); + ScFlatBoolColSegments::RangeData aHiddenColsRange; + ScFlatBoolColSegments::RangeIterator aHiddenColsRangeItr(maFilterData.getHiddenCols()); + bool bHasHiddenColsRange = aHiddenColsRangeItr.getFirst(aHiddenColsRange); + for (SCCOL i = 0; i < nCol; ++i) + { + // Fetch the next width range if necessary. + if (i > nColWidthRangeEnd) + nColWidth = mpColWidth->GetNextValue(nColWidthIndex, nColWidthRangeEnd); + // Fetch the next hidden cols range if necessary. + if (bHasHiddenColsRange && i > aHiddenColsRange.mnCol2) + bHasHiddenColsRange = aHiddenColsRangeItr.getNext(aHiddenColsRange); + + if (!bHasHiddenColsRange) + { + // Zero width, because absence of data implies hidden, see FilterData::colHidden. + } + else if (i >= aHiddenColsRange.mnCol1 && i <= aHiddenColsRange.mnCol2) + { + // Inside a range for which we have "is-hidden" data + if (!aHiddenColsRange.mbValue) + n += nColWidth; + } + else + { + n += nColWidth; + } + } } else { - OSL_FAIL("GetColumnOffset: Data missing"); + size_t nColWidthIndex; + SCCOL nColWidthRangeEnd; + sal_uInt16 nColWidth = mpColWidth->GetValue(0, nColWidthIndex, nColWidthRangeEnd); + for (SCCOL i = 0; i < nCol; ++i) + { + if (i > nColWidthRangeEnd) + nColWidth = mpColWidth->GetNextValue(nColWidthIndex, nColWidthRangeEnd); + n += nColWidth; + } } return n; }