sc/inc/brdcst.hxx | 8 ++- sc/inc/document.hxx | 2 sc/inc/grouparealistener.hxx | 2 sc/source/core/data/bcaslot.cxx | 32 ++++++++++---- sc/source/core/data/documen7.cxx | 66 +++++++++++++++++++----------- sc/source/core/tool/dbdata.cxx | 6 +- sc/source/core/tool/grouparealistener.cxx | 7 +-- 7 files changed, 80 insertions(+), 43 deletions(-)
New commits: commit 8579b6e39ea30048ae7763f27d77e093b26e76dc Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Mon Dec 6 02:20:46 2021 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Dec 6 22:19:43 2021 +0100 process broadcasts for adjacent cells together (tdf#119083) The problem in tdf#119083 is that it sets up listening for the range of cells used by VLOOKUP, and when adding a new column changes all those cells, it results in repeated SfxHintId::ScDataChanged broadcasts to the cells with the VLOOKUP formula. This commit makes ScHint include a row count, making it possible to group adjacent rows for SfxHintId::ScDataChanged together and send notifications just once for the range. Change-Id: Ib3439de58a2b1e5e8f01b037a62608e38b8e9125 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126395 Tested-by: Jenkins Reviewed-by: Luboš Luňák <l.lu...@collabora.com> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126448 diff --git a/sc/inc/brdcst.hxx b/sc/inc/brdcst.hxx index 3d57e7644370..9558723ba0ac 100644 --- a/sc/inc/brdcst.hxx +++ b/sc/inc/brdcst.hxx @@ -24,10 +24,14 @@ class ScHint final : public SfxHint { ScAddress aAddress; + SCROW nRowCount; public: - ScHint( SfxHintId n, const ScAddress& a ) : SfxHint(n), aAddress(a) {} - const ScAddress& GetAddress() const { return aAddress; } + ScHint( SfxHintId n, const ScAddress& a, SCROW rowCount = 1 ) : SfxHint(n), aAddress(a), nRowCount( rowCount ) {} + const ScAddress& GetStartAddress() const { return aAddress; } + SCROW GetRowCount() const { return nRowCount; } + ScRange GetRange() const + { return ScRange(aAddress, ScAddress(aAddress.Col(), aAddress.Row() + nRowCount - 1, aAddress.Tab())); } void SetAddressTab(SCTAB nTab) { aAddress.SetTab(nTab); } void SetAddressCol(SCCOL nCol) { aAddress.SetCol(nCol); } void SetAddressRow(SCROW nRow) { aAddress.SetRow(nRow); } diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx index c6246aec96b1..50edcb77493e 100644 --- a/sc/inc/document.hxx +++ b/sc/inc/document.hxx @@ -2617,6 +2617,8 @@ private: void EndListeningGroups( const std::vector<ScAddress>& rPosArray ); void SetNeedsListeningGroups( const std::vector<ScAddress>& rPosArray ); + + bool BroadcastHintInternal( const ScHint &rHint ); }; typedef std::unique_ptr<ScDocument, o3tl::default_delete<ScDocument>> ScDocumentUniquePtr; diff --git a/sc/inc/grouparealistener.hxx b/sc/inc/grouparealistener.hxx index 5d11541244eb..d823fc987627 100644 --- a/sc/inc/grouparealistener.hxx +++ b/sc/inc/grouparealistener.hxx @@ -61,7 +61,7 @@ public: void collectFormulaCells( SCROW nRow1, SCROW nRow2, std::vector<ScFormulaCell*>& rCells ) const; private: - void notifyCellChange( const SfxHint& rHint, const ScAddress& rPos ); + void notifyCellChange( const SfxHint& rHint, const ScAddress& rPos, SCROW nNumRows ); void notifyBulkChange( const BulkDataHint& rHint ); const ScFormulaCell* getTopCell() const; }; diff --git a/sc/source/core/data/bcaslot.cxx b/sc/source/core/data/bcaslot.cxx index 2627bdf9b990..de772399e0e8 100644 --- a/sc/source/core/data/bcaslot.cxx +++ b/sc/source/core/data/bcaslot.cxx @@ -295,7 +295,7 @@ bool ScBroadcastAreaSlot::AreaBroadcast( const ScHint& rHint) mbHasErasedArea = false; - const ScAddress& rAddress = rHint.GetAddress(); + const ScRange& rRange = rHint.GetRange(); for (ScBroadcastAreas::const_iterator aIter( aBroadcastAreaTbl.begin()), aIterEnd( aBroadcastAreaTbl.end()); aIter != aIterEnd; ++aIter ) { @@ -304,13 +304,13 @@ bool ScBroadcastAreaSlot::AreaBroadcast( const ScHint& rHint) ScBroadcastArea* pArea = (*aIter).mpArea; const ScRange& rAreaRange = pArea->GetRange(); - if (rAreaRange.In( rAddress)) + if (rAreaRange.In( rRange)) { if (pArea->IsGroupListening()) { if (pBASM->IsInBulkBroadcast()) { - pBASM->InsertBulkGroupArea(pArea, rAddress); + pBASM->InsertBulkGroupArea(pArea, rRange); } else { @@ -808,7 +808,7 @@ bool ScBroadcastAreaSlotMachine::AreaBroadcast( const ScRange& rRange, SfxHintId bool ScBroadcastAreaSlotMachine::AreaBroadcast( const ScHint& rHint ) const { - const ScAddress& rAddress = rHint.GetAddress(); + const ScAddress& rAddress = rHint.GetStartAddress(); if ( rAddress == BCA_BRDCST_ALWAYS ) { if ( pBCAlways ) @@ -824,12 +824,24 @@ bool ScBroadcastAreaSlotMachine::AreaBroadcast( const ScHint& rHint ) const TableSlotsMap::const_iterator iTab( aTableSlotsMap.find( rAddress.Tab())); if (iTab == aTableSlotsMap.end()) return false; - ScBroadcastAreaSlot* pSlot = (*iTab).second->getAreaSlot( - ComputeSlotOffset( rAddress)); - if ( pSlot ) - return pSlot->AreaBroadcast( rHint ); - else - return false; + // Process all slots for the given row range, but it's enough to process + // each only once. + ScBroadcastAreaSlot* pLastSlot = nullptr; + ScAddress address(rAddress); + bool wasBroadcast = false; + for( SCROW nRow = rAddress.Row(); nRow < rAddress.Row() + rHint.GetRowCount(); ++nRow ) + { + address.SetRow(nRow); + ScBroadcastAreaSlot* pSlot = (*iTab).second->getAreaSlot( + ComputeSlotOffset( address)); + if ( pSlot && pSlot != pLastSlot ) + { + if(pSlot->AreaBroadcast( rHint )) + wasBroadcast = true; + pLastSlot = pSlot; + } + } + return wasBroadcast; } } diff --git a/sc/source/core/data/documen7.cxx b/sc/source/core/data/documen7.cxx index ab6ae63cb549..bf0d38cb4ec9 100644 --- a/sc/source/core/data/documen7.cxx +++ b/sc/source/core/data/documen7.cxx @@ -121,25 +121,40 @@ void ScDocument::Broadcast( const ScHint& rHint ) if ( eHardRecalcState == HardRecalcState::OFF ) { ScBulkBroadcast aBulkBroadcast( pBASM.get(), rHint.GetId()); // scoped bulk broadcast - bool bIsBroadcasted = false; - SvtBroadcaster* pBC = GetBroadcaster(rHint.GetAddress()); - if ( pBC ) - { - pBC->Broadcast( rHint ); - bIsBroadcasted = true; - } + bool bIsBroadcasted = BroadcastHintInternal(rHint); if ( pBASM->AreaBroadcast( rHint ) || bIsBroadcasted ) TrackFormulas( rHint.GetId() ); } - if ( rHint.GetAddress() != BCA_BRDCST_ALWAYS ) + if ( rHint.GetStartAddress() != BCA_BRDCST_ALWAYS ) { - SCTAB nTab = rHint.GetAddress().Tab(); + SCTAB nTab = rHint.GetStartAddress().Tab(); if (nTab < static_cast<SCTAB>(maTabs.size()) && maTabs[nTab]) maTabs[nTab]->SetStreamValid(false); } } +bool ScDocument::BroadcastHintInternal( const ScHint& rHint ) +{ + bool bIsBroadcasted = false; + const ScAddress address(rHint.GetStartAddress()); + SvtBroadcaster* pLastBC = nullptr; + // Process all broadcasters for the given row range. + for( SCROW nRow = address.Row(); nRow < address.Row() + rHint.GetRowCount(); ++nRow ) + { + ScAddress a(address); + a.SetRow(nRow); + SvtBroadcaster* pBC = GetBroadcaster(a); + if ( pBC && pBC != pLastBC ) + { + pBC->Broadcast( rHint ); + bIsBroadcasted = true; + pLastBC = pBC; + } + } + return bIsBroadcasted; +} + void ScDocument::BroadcastCells( const ScRange& rRange, SfxHintId nHint, bool bBroadcastSingleBroadcasters ) { PrepareFormulaCalc(); @@ -528,32 +543,35 @@ void ScDocument::TrackFormulas( SfxHintId nHintId ) { // outside the loop, check if any sheet has a "calculate" event script bool bCalcEvent = HasAnySheetEventScript( ScSheetEventId::CALCULATE, true ); - ScFormulaCell* pTrack; - ScFormulaCell* pNext; - pTrack = pFormulaTrack; - do + for( ScFormulaCell* pTrack = pFormulaTrack; pTrack != nullptr; pTrack = pTrack->GetNextTrack()) { - SvtBroadcaster* pBC = GetBroadcaster(pTrack->aPos); - ScHint aHint(nHintId, pTrack->aPos); - if (pBC) - pBC->Broadcast( aHint ); + SCROW rowCount = 1; + ScAddress address = pTrack->aPos; + // Compress to include all adjacent cells in the same column. + for(ScFormulaCell* pNext = pTrack->GetNextTrack(); pNext != nullptr; pNext = pNext->GetNextTrack()) + { + if(pNext->aPos != ScAddress(address.Col(), address.Row() + rowCount, address.Tab())) + break; + ++rowCount; + pTrack = pNext; + } + ScHint aHint( nHintId, address, rowCount ); + BroadcastHintInternal( aHint ); pBASM->AreaBroadcast( aHint ); // for "calculate" event, keep track of which sheets are affected by tracked formulas if ( bCalcEvent ) - SetCalcNotification( pTrack->aPos.Tab() ); - pTrack = pTrack->GetNextTrack(); - } while ( pTrack ); - pTrack = pFormulaTrack; + SetCalcNotification( address.Tab() ); + } bool bHaveForced = false; - do + for( ScFormulaCell* pTrack = pFormulaTrack; pTrack != nullptr;) { - pNext = pTrack->GetNextTrack(); + ScFormulaCell* pNext = pTrack->GetNextTrack(); RemoveFromFormulaTrack( pTrack ); PutInFormulaTree( pTrack ); if ( pTrack->GetCode()->IsRecalcModeForced() ) bHaveForced = true; pTrack = pNext; - } while ( pTrack ); + } if ( bHaveForced ) { SetForcedFormulas( true ); diff --git a/sc/source/core/tool/dbdata.cxx b/sc/source/core/tool/dbdata.cxx index b4e52cebb108..e97d01972973 100644 --- a/sc/source/core/tool/dbdata.cxx +++ b/sc/source/core/tool/dbdata.cxx @@ -937,11 +937,11 @@ void ScDBData::Notify( const SfxHint& rHint ) if (aHeaderRange.IsValid()) { mpContainer->GetDirtyTableColumnNames().Join( aHeaderRange); - if (!aHeaderRange.In( pScHint->GetAddress())) - mpContainer->GetDirtyTableColumnNames().Join( pScHint->GetAddress()); + if (!aHeaderRange.In( pScHint->GetRange())) + mpContainer->GetDirtyTableColumnNames().Join( pScHint->GetRange()); } else - mpContainer->GetDirtyTableColumnNames().Join( pScHint->GetAddress()); + mpContainer->GetDirtyTableColumnNames().Join( pScHint->GetRange()); } // Do not refresh column names here, which might trigger unwanted diff --git a/sc/source/core/tool/grouparealistener.cxx b/sc/source/core/tool/grouparealistener.cxx index 3fab01bd2c75..86ebb8b27b42 100644 --- a/sc/source/core/tool/grouparealistener.cxx +++ b/sc/source/core/tool/grouparealistener.cxx @@ -119,7 +119,8 @@ void FormulaGroupAreaListener::Notify( const SfxHint& rHint ) } else if (rHint.GetId() == SfxHintId::ScDataChanged || rHint.GetId() == SfxHintId::ScTableOpDirty) { - notifyCellChange(rHint, static_cast<const ScHint*>(&rHint)->GetAddress()); + const ScHint& rScHint = static_cast<const ScHint&>(rHint); + notifyCellChange(rHint, rScHint.GetStartAddress(), rScHint.GetRowCount()); } } @@ -332,11 +333,11 @@ const ScFormulaCell* FormulaGroupAreaListener::getTopCell() const return pp ? *pp : nullptr; } -void FormulaGroupAreaListener::notifyCellChange( const SfxHint& rHint, const ScAddress& rPos ) +void FormulaGroupAreaListener::notifyCellChange( const SfxHint& rHint, const ScAddress& rPos, SCROW nNumRows ) { // Determine which formula cells within the group need to be notified of this change. std::vector<ScFormulaCell*> aCells; - collectFormulaCells(rPos.Tab(), rPos.Col(), rPos.Row(), rPos.Row(), aCells); + collectFormulaCells(rPos.Tab(), rPos.Col(), rPos.Row(), rPos.Row() + nNumRows - 1, aCells); std::for_each(aCells.begin(), aCells.end(), Notifier(rHint)); }