include/svl/itempool.hxx | 7 +++ sc/inc/attarray.hxx | 16 +++++++- sc/inc/column.hxx | 7 ++- sc/inc/docpool.hxx | 4 +- sc/inc/document.hxx | 2 + sc/inc/table.hxx | 8 +--- sc/source/core/data/attarray.cxx | 71 ++++++++++++++++++-------------------- sc/source/core/data/column.cxx | 15 ++++++-- sc/source/core/data/column4.cxx | 6 +-- sc/source/core/data/docpool.cxx | 6 +-- sc/source/core/data/document.cxx | 8 ++++ sc/source/core/data/table2.cxx | 19 ++++++++++ sc/source/core/data/table3.cxx | 6 +-- sc/source/filter/rtf/eeimpars.cxx | 44 +++++++++++------------ sc/source/ui/undo/undocell.cxx | 6 +-- svl/source/items/itempool.cxx | 20 +++++++++- 16 files changed, 157 insertions(+), 88 deletions(-)
New commits: commit c757117afb398277a46e79ba22066c5bbf2c9f72 Author: Noel Grandin <noel.gran...@collabora.co.uk> AuthorDate: Sat Apr 20 13:03:33 2019 +0200 Commit: Noel Grandin <noel.gran...@collabora.co.uk> CommitDate: Sun Apr 21 07:58:14 2019 +0200 tdf#81765 slow loading of .ods with >1000 of conditional formats, part 2 This takes the loading time from 15s to 14s. Reduce unnecessary allocation/copying by passing down ownership of the newly created ScPatternAttr to the item pool Change-Id: Iec38bbff572d10ff8d86f5e65fbe9a96b6a5a706 Reviewed-on: https://gerrit.libreoffice.org/71010 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> diff --git a/include/svl/itempool.hxx b/include/svl/itempool.hxx index f987cf15ae62..391e1b942544 100644 --- a/include/svl/itempool.hxx +++ b/include/svl/itempool.hxx @@ -146,7 +146,10 @@ public: virtual SfxItemPool* Clone() const; const OUString& GetName() const; - virtual const SfxPoolItem& Put( const SfxPoolItem&, sal_uInt16 nWhich = 0 ); + const SfxPoolItem& Put( std::unique_ptr<SfxPoolItem> xItem, sal_uInt16 nWhich = 0 ) + { return PutImpl( *xItem.release(), nWhich, /*bPassingOwnership*/true); } + const SfxPoolItem& Put( const SfxPoolItem& rItem, sal_uInt16 nWhich = 0 ) + { return PutImpl( rItem, nWhich, /*bPassingOwnership*/false); } void Remove( const SfxPoolItem& ); const SfxPoolItem& GetDefaultItem( sal_uInt16 nWhich ) const; @@ -195,6 +198,8 @@ public: void dumpAsXml(xmlTextWriterPtr pWriter) const; +protected: + virtual const SfxPoolItem& PutImpl( const SfxPoolItem&, sal_uInt16 nWhich = 0, bool bPassingOwnership = false ); private: const SfxItemPool& operator=(const SfxItemPool &) = delete; diff --git a/sc/inc/attarray.hxx b/sc/inc/attarray.hxx index d2ca2d7c842f..b43befd45e84 100644 --- a/sc/inc/attarray.hxx +++ b/sc/inc/attarray.hxx @@ -137,9 +137,16 @@ public: void ApplyBlockFrame(const SvxBoxItem& rLineOuter, const SvxBoxInfoItem* pLineInner, SCROW nStartRow, SCROW nEndRow, bool bLeft, SCCOL nDistRight); - void SetPattern( SCROW nRow, const ScPatternAttr* pPattern, bool bPutToPool = false ); + void SetPattern( SCROW nRow, const ScPatternAttr* pPattern, bool bPutToPool = false ) + { SetPatternAreaImpl(nRow, nRow, pPattern, bPutToPool, nullptr, /*bPassingOwnership*/false); } + const ScPatternAttr* SetPattern( SCROW nRow, std::unique_ptr<ScPatternAttr> pPattern, bool bPutToPool = false ) + { return SetPatternAreaImpl(nRow, nRow, pPattern.release(), bPutToPool, nullptr, /*bPassingOwnership*/true); } + void SetPatternArea( SCROW nStartRow, SCROW nEndRow, std::unique_ptr<ScPatternAttr> pPattern, + bool bPutToPool = false, ScEditDataArray* pDataArray = nullptr) + { SetPatternAreaImpl(nStartRow, nEndRow, pPattern.release(), bPutToPool, pDataArray, /*bPassingOwnership*/true); } void SetPatternArea( SCROW nStartRow, SCROW nEndRow, const ScPatternAttr* pPattern, - bool bPutToPool = false, ScEditDataArray* pDataArray = nullptr ); + bool bPutToPool = false, ScEditDataArray* pDataArray = nullptr) + { SetPatternAreaImpl(nStartRow, nEndRow, pPattern, bPutToPool, pDataArray, /*bPassingOwnership*/false); } void ApplyStyleArea( SCROW nStartRow, SCROW nEndRow, const ScStyleSheet& rStyle ); void ApplyCacheArea( SCROW nStartRow, SCROW nEndRow, SfxItemPoolCache* pCache, ScEditDataArray* pDataArray = nullptr, bool* const pIsChanged = nullptr ); @@ -210,6 +217,11 @@ public: bool Reserve( SCSIZE nReserve ); SCSIZE Count() const { return mvData.size(); } SCSIZE Count( SCROW nRow1, SCROW nRow2 ) const; + +private: + const ScPatternAttr* SetPatternAreaImpl( SCROW nStartRow, SCROW nEndRow, const ScPatternAttr* pPattern, + bool bPutToPool = false, ScEditDataArray* pDataArray = nullptr, + bool bPassingPatternOwnership = false ); }; // Iterator for attributes diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx index b836fabef770..22d8191f616d 100644 --- a/sc/inc/column.hxx +++ b/sc/inc/column.hxx @@ -460,9 +460,10 @@ public: void ApplyPatternArea( SCROW nStartRow, SCROW nEndRow, const ScPatternAttr& rPatAttr, ScEditDataArray* pDataArray = nullptr, bool* const pIsChanged = nullptr); - void SetPattern( SCROW nRow, const ScPatternAttr& rPatAttr ); - void SetPatternArea( SCROW nStartRow, SCROW nEndRow, - const ScPatternAttr& rPatAttr ); + const ScPatternAttr* SetPattern( SCROW nRow, std::unique_ptr<ScPatternAttr> ); + void SetPattern( SCROW nRow, const ScPatternAttr& ); + void SetPatternArea( SCROW nStartRow, SCROW nEndRow, std::unique_ptr<ScPatternAttr> ); + void SetPatternArea( SCROW nStartRow, SCROW nEndRow, const ScPatternAttr& ); void ApplyPatternIfNumberformatIncompatible( const ScRange& rRange, const ScPatternAttr& rPattern, SvNumFormatType nNewType ); diff --git a/sc/inc/docpool.hxx b/sc/inc/docpool.hxx index 2939f4addd7a..0b91a5494526 100644 --- a/sc/inc/docpool.hxx +++ b/sc/inc/docpool.hxx @@ -41,14 +41,14 @@ public: virtual SfxItemPool* Clone() const override; virtual MapUnit GetMetric( sal_uInt16 nWhich ) const override; - virtual const SfxPoolItem& Put( const SfxPoolItem&, sal_uInt16 nWhich = 0 ) override; - void StyleDeleted( const ScStyleSheet* pStyle ); // delete templates(?) in organizer void CellStyleCreated( const OUString& rName, const ScDocument* pDoc ); virtual bool GetPresentation( const SfxPoolItem& rItem, MapUnit ePresentationMetric, OUString& rText, const IntlWrapper& rIntl ) const override; +protected: + virtual const SfxPoolItem& PutImpl( const SfxPoolItem&, sal_uInt16 nWhich = 0, bool bPassingOwnership = false ) override; }; #endif diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx index cdeab88fdfcb..615760e87c3a 100644 --- a/sc/inc/document.hxx +++ b/sc/inc/document.hxx @@ -1789,7 +1789,9 @@ public: SCCOL nEndCol, SCROW nEndRow, SCTAB nTab, ScMF nFlags ); + SC_DLLPUBLIC void SetPattern( const ScAddress&, std::unique_ptr<ScPatternAttr> ); SC_DLLPUBLIC void SetPattern( const ScAddress&, const ScPatternAttr& rAttr ); + SC_DLLPUBLIC const ScPatternAttr* SetPattern( SCCOL nCol, SCROW nRow, SCTAB nTab, std::unique_ptr<ScPatternAttr> ); SC_DLLPUBLIC void SetPattern( SCCOL nCol, SCROW nRow, SCTAB nTab, const ScPatternAttr& rAttr ); void AutoFormat( SCCOL nStartCol, SCROW nStartRow, SCCOL nEndCol, SCROW nEndRow, diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx index e9b1e3f54c65..50f6c5f85aef 100644 --- a/sc/inc/table.hxx +++ b/sc/inc/table.hxx @@ -707,11 +707,9 @@ public: const ScPatternAttr& rAttr, ScEditDataArray* pDataArray = nullptr, bool* const pIsChanged = nullptr ); - void SetPattern( const ScAddress& rPos, const ScPatternAttr& rAttr ) - { - if (ValidColRow(rPos.Col(),rPos.Row())) - aCol[rPos.Col()].SetPattern( rPos.Row(), rAttr ); - } + void SetPattern( const ScAddress& rPos, std::unique_ptr<ScPatternAttr> ); + void SetPattern( const ScAddress& rPos, const ScPatternAttr& rAttr ); + const ScPatternAttr* SetPattern( SCCOL nCol, SCROW nRow, std::unique_ptr<ScPatternAttr> ); void SetPattern( SCCOL nCol, SCROW nRow, const ScPatternAttr& rAttr ); void ApplyPatternIfNumberformatIncompatible( const ScRange& rRange, const ScPatternAttr& rPattern, SvNumFormatType nNewType ); diff --git a/sc/source/core/data/attarray.cxx b/sc/source/core/data/attarray.cxx index 080906afce62..282f6ce666cf 100644 --- a/sc/source/core/data/attarray.cxx +++ b/sc/source/core/data/attarray.cxx @@ -321,7 +321,7 @@ void ScAttrArray::AddCondFormat( SCROW nStartRow, SCROW nEndRow, sal_uInt32 nInd nTempEndRow = nEndRow; } - SetPatternArea( nTempStartRow, nTempEndRow, pNewPattern.get(), true ); + SetPatternArea( nTempStartRow, nTempEndRow, std::move(pNewPattern), true ); nTempStartRow = nTempEndRow + 1; } while(nTempEndRow < nEndRow); @@ -345,7 +345,6 @@ void ScAttrArray::RemoveCondFormat( SCROW nStartRow, SCROW nEndRow, sal_uInt32 n if(pPattern) { - ScPatternAttr aPattern( *pPattern ); SCROW nPatternStartRow; SCROW nPatternEndRow; GetPatternRange( nPatternStartRow, nPatternEndRow, nTempStartRow ); @@ -355,6 +354,7 @@ void ScAttrArray::RemoveCondFormat( SCROW nStartRow, SCROW nEndRow, sal_uInt32 n pPattern->GetItemSet().GetItemState( ATTR_CONDITIONAL, true, &pItem ); if(pItem) { + auto pPatternAttr = std::make_unique<ScPatternAttr>( *pPattern ); std::vector< sal_uInt32 > aCondFormatData = static_cast<const ScCondFormatItem*>(pItem)->GetCondFormatData(); std::vector<sal_uInt32>::iterator itr = std::find(aCondFormatData.begin(), aCondFormatData.end(), nIndex); if(itr != aCondFormatData.end() || nIndex == 0) @@ -365,8 +365,8 @@ void ScAttrArray::RemoveCondFormat( SCROW nStartRow, SCROW nEndRow, sal_uInt32 n else aCondFormatData.erase(itr); aItem.SetCondFormatData( aCondFormatData ); - aPattern.GetItemSet().Put( aItem ); - SetPatternArea( nTempStartRow, nTempEndRow, &aPattern, true ); + pPatternAttr->GetItemSet().Put( aItem ); + SetPatternArea( nTempStartRow, nTempEndRow, std::move(pPatternAttr), true ); } } } @@ -381,11 +381,6 @@ void ScAttrArray::RemoveCondFormat( SCROW nStartRow, SCROW nEndRow, sal_uInt32 n } -void ScAttrArray::SetPattern( SCROW nRow, const ScPatternAttr* pPattern, bool bPutToPool ) -{ - SetPatternArea( nRow, nRow, pPattern, bPutToPool ); -} - void ScAttrArray::RemoveCellCharAttribs( SCROW nStartRow, SCROW nEndRow, const ScPatternAttr* pPattern, ScEditDataArray* pDataArray ) { @@ -440,14 +435,18 @@ bool ScAttrArray::Reserve( SCSIZE nReserve ) return false; } -void ScAttrArray::SetPatternArea(SCROW nStartRow, SCROW nEndRow, const ScPatternAttr *pPattern, - bool bPutToPool, ScEditDataArray* pDataArray ) +const ScPatternAttr* ScAttrArray::SetPatternAreaImpl(SCROW nStartRow, SCROW nEndRow, const ScPatternAttr* pPattern, + bool bPutToPool, ScEditDataArray* pDataArray, bool bPassingOwnership ) { if (ValidRow(nStartRow) && ValidRow(nEndRow)) { if (bPutToPool) - pPattern = static_cast<const ScPatternAttr*>(&pDocument->GetPool()->Put(*pPattern)); - + { + if (bPassingOwnership) + pPattern = static_cast<const ScPatternAttr*>(&pDocument->GetPool()->Put(std::unique_ptr<ScPatternAttr>(const_cast<ScPatternAttr*>(pPattern)))); + else + pPattern = static_cast<const ScPatternAttr*>(&pDocument->GetPool()->Put(*pPattern)); + } if ((nStartRow == 0) && (nEndRow == MAXROW)) Reset(pPattern); else @@ -611,6 +610,7 @@ void ScAttrArray::SetPatternArea(SCROW nStartRow, SCROW nEndRow, const ScPattern #if DEBUG_SC_TESTATTRARRAY TestData(); #endif + return pPattern; } void ScAttrArray::ApplyStyleArea( SCROW nStartRow, SCROW nEndRow, const ScStyleSheet& rStyle ) @@ -648,7 +648,7 @@ void ScAttrArray::ApplyStyleArea( SCROW nStartRow, SCROW nEndRow, const ScStyleS { if (nY1 < nStartRow) nY1=nStartRow; if (nY2 > nEndRow) nY2=nEndRow; - SetPatternArea( nY1, nY2, pNewPattern.get(), true ); + SetPatternArea( nY1, nY2, std::move(pNewPattern), true ); Search( nStart, nPos ); } else @@ -805,7 +805,7 @@ void ScAttrArray::ApplyLineStyleArea( SCROW nStartRow, SCROW nEndRow, { if (nY1 < nStartRow) nY1=nStartRow; if (nY2 > nEndRow) nY2=nEndRow; - SetPatternArea( nY1, nY2, pNewPattern.get(), true ); + SetPatternArea( nY1, nY2, std::move(pNewPattern), true ); Search( nStart, nPos ); } else @@ -813,7 +813,7 @@ void ScAttrArray::ApplyLineStyleArea( SCROW nStartRow, SCROW nEndRow, // remove from pool ? pDocument->GetPool()->Remove(*mvData[nPos].pPattern); mvData[nPos].pPattern = static_cast<const ScPatternAttr*>( - &pDocument->GetPool()->Put(*pNewPattern) ); + &pDocument->GetPool()->Put(std::move(pNewPattern)) ); if (Concat(nPos)) Search(nStart, nPos); @@ -1569,9 +1569,8 @@ void ScAttrArray::SetPatternAreaSafe( SCROW nStartRow, SCROW nEndRow, // Instead, the document's GetDefPattern is copied. Since it is passed as // pWantedPattern, no special treatment of default is needed here anymore. std::unique_ptr<ScPatternAttr> pNewPattern(new ScPatternAttr( *pWantedPattern )); - SfxItemSet* pSet = &pNewPattern->GetItemSet(); - pSet->Put( *pItem ); - SetPatternArea( nThisRow, nAttrRow, pNewPattern.get(), true ); + pNewPattern->GetItemSet().Put( *pItem ); + SetPatternArea( nThisRow, nAttrRow, std::move(pNewPattern), true ); } else { @@ -1617,9 +1616,9 @@ bool ScAttrArray::ApplyFlags( SCROW nStartRow, SCROW nEndRow, ScMF nFlags ) { nRow = mvData[nIndex].nEndRow; SCROW nAttrRow = std::min( nRow, nEndRow ); - ScPatternAttr aNewPattern(*pOldPattern); - aNewPattern.GetItemSet().Put( ScMergeFlagAttr( nOldValue | nFlags ) ); - SetPatternArea( nThisRow, nAttrRow, &aNewPattern, true ); + auto pNewPattern = std::make_unique<ScPatternAttr>(*pOldPattern); + pNewPattern->GetItemSet().Put( ScMergeFlagAttr( nOldValue | nFlags ) ); + SetPatternArea( nThisRow, nAttrRow, std::move(pNewPattern), true ); Search( nThisRow, nIndex ); // data changed bChanged = true; } @@ -1654,9 +1653,9 @@ bool ScAttrArray::RemoveFlags( SCROW nStartRow, SCROW nEndRow, ScMF nFlags ) { nRow = mvData[nIndex].nEndRow; SCROW nAttrRow = std::min( nRow, nEndRow ); - ScPatternAttr aNewPattern(*pOldPattern); - aNewPattern.GetItemSet().Put( ScMergeFlagAttr( nOldValue & ~nFlags ) ); - SetPatternArea( nThisRow, nAttrRow, &aNewPattern, true ); + auto pNewPattern = std::make_unique<ScPatternAttr>(*pOldPattern); + pNewPattern->GetItemSet().Put( ScMergeFlagAttr( nOldValue & ~nFlags ) ); + SetPatternArea( nThisRow, nAttrRow, std::move(pNewPattern), true ); Search( nThisRow, nIndex ); // data changed bChanged = true; } @@ -1684,12 +1683,12 @@ void ScAttrArray::ClearItems( SCROW nStartRow, SCROW nEndRow, const sal_uInt16* const ScPatternAttr* pOldPattern = mvData[nIndex].pPattern; if ( pOldPattern->HasItemsSet( pWhich ) ) { - ScPatternAttr aNewPattern(*pOldPattern); - aNewPattern.ClearItems( pWhich ); + auto pNewPattern = std::make_unique<ScPatternAttr>(*pOldPattern); + pNewPattern->ClearItems( pWhich ); nRow = mvData[nIndex].nEndRow; SCROW nAttrRow = std::min( nRow, nEndRow ); - SetPatternArea( nThisRow, nAttrRow, &aNewPattern, true ); + SetPatternArea( nThisRow, nAttrRow, std::move(pNewPattern), true ); Search( nThisRow, nIndex ); // data changed } @@ -1742,12 +1741,12 @@ void ScAttrArray::ChangeIndent( SCROW nStartRow, SCROW nEndRow, bool bIncrement { SCROW nThisEnd = mvData[nIndex].nEndRow; SCROW nAttrRow = std::min( nThisEnd, nEndRow ); - ScPatternAttr aNewPattern(*pOldPattern); - aNewPattern.GetItemSet().Put( SfxUInt16Item( ATTR_INDENT, nNewValue ) ); + auto pNewPattern = std::make_unique<ScPatternAttr>(*pOldPattern); + pNewPattern->GetItemSet().Put( SfxUInt16Item( ATTR_INDENT, nNewValue ) ); if ( bNeedJust ) - aNewPattern.GetItemSet().Put( + pNewPattern->GetItemSet().Put( SvxHorJustifyItem( SvxCellHorJustify::Left, ATTR_HOR_JUSTIFY ) ); - SetPatternArea( nThisStart, nAttrRow, &aNewPattern, true ); + SetPatternArea( nThisStart, nAttrRow, std::move(pNewPattern), true ); nThisStart = nThisEnd + 1; Search( nThisStart, nIndex ); // data changed @@ -2339,16 +2338,16 @@ void ScAttrArray::DeleteHardAttr(SCROW nStartRow, SCROW nEndRow) nRow = mvData[nIndex].nEndRow; SCROW nAttrRow = std::min( nRow, nEndRow ); - ScPatternAttr aNewPattern(*pOldPattern); - SfxItemSet& rSet = aNewPattern.GetItemSet(); + auto pNewPattern = std::make_unique<ScPatternAttr>(*pOldPattern); + SfxItemSet& rSet = pNewPattern->GetItemSet(); for (sal_uInt16 nId = ATTR_PATTERN_START; nId <= ATTR_PATTERN_END; nId++) if (nId != ATTR_MERGE && nId != ATTR_MERGE_FLAG) rSet.ClearItem(nId); - if ( aNewPattern == *pDefPattern ) + if ( *pNewPattern == *pDefPattern ) SetPatternArea( nThisRow, nAttrRow, pDefPattern ); else - SetPatternArea( nThisRow, nAttrRow, &aNewPattern, true ); + SetPatternArea( nThisRow, nAttrRow, std::move(pNewPattern), true ); Search( nThisRow, nIndex ); // data changed } diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx index 7e606ea2a6d6..b5e3577c35a4 100644 --- a/sc/source/core/data/column.cxx +++ b/sc/source/core/data/column.cxx @@ -553,7 +553,7 @@ void ScColumn::ApplyStyle( SCROW nRow, const ScStyleSheet* rStyle ) const ScPatternAttr* pPattern = pAttrArray->GetPattern(nRow); std::unique_ptr<ScPatternAttr> pNewPattern(new ScPatternAttr(*pPattern)); pNewPattern->SetStyleSheet(const_cast<ScStyleSheet*>(rStyle)); - pAttrArray->SetPattern(nRow, pNewPattern.get(), true); + pAttrArray->SetPattern(nRow, std::move(pNewPattern), true); } void ScColumn::ApplyStyleArea( SCROW nStartRow, SCROW nEndRow, const ScStyleSheet& rStyle ) @@ -683,12 +683,23 @@ void ScColumn::ClearItems( SCROW nStartRow, SCROW nEndRow, const sal_uInt16* pWh pAttrArray->ClearItems( nStartRow, nEndRow, pWhich ); } +const ScPatternAttr* ScColumn::SetPattern( SCROW nRow, std::unique_ptr<ScPatternAttr> pPatAttr ) +{ + return pAttrArray->SetPattern( nRow, std::move(pPatAttr), true/*bPutToPool*/ ); +} + void ScColumn::SetPattern( SCROW nRow, const ScPatternAttr& rPatAttr ) { pAttrArray->SetPattern( nRow, &rPatAttr, true/*bPutToPool*/ ); } void ScColumn::SetPatternArea( SCROW nStartRow, SCROW nEndRow, + std::unique_ptr<ScPatternAttr> pPatAttr ) +{ + pAttrArray->SetPatternArea( nStartRow, nEndRow, std::move(pPatAttr), true/*bPutToPool*/ ); +} + +void ScColumn::SetPatternArea( SCROW nStartRow, SCROW nEndRow, const ScPatternAttr& rPatAttr ) { pAttrArray->SetPatternArea( nStartRow, nEndRow, &rPatAttr, true/*bPutToPool*/ ); @@ -1692,7 +1703,7 @@ void ScColumn::CopyToColumn( const ScPatternAttr* pPattern = pAttrArray->GetPattern( nRow ); std::unique_ptr<ScPatternAttr> pNewPattern(new ScPatternAttr( *pPattern )); pNewPattern->SetStyleSheet( const_cast<ScStyleSheet*>(pStyle) ); - rColumn.pAttrArray->SetPattern( nRow, pNewPattern.get(), true ); + rColumn.pAttrArray->SetPattern( nRow, std::move(pNewPattern), true ); } } else diff --git a/sc/source/core/data/column4.cxx b/sc/source/core/data/column4.cxx index bdefbe8e2b3e..162537f169a8 100644 --- a/sc/source/core/data/column4.cxx +++ b/sc/source/core/data/column4.cxx @@ -214,12 +214,12 @@ void ScColumn::CopyOneCellFromClip( sc::CopyFromClipContext& rCxt, SCROW nRow1, const ScPatternAttr* pAttr = (bSameDocPool ? rCxt.getSingleCellPattern(nColOffset) : rCxt.getSingleCellPattern(nColOffset)->PutInPool( pDocument, rCxt.getClipDoc())); - ScPatternAttr aNewPattern(*pAttr); + auto pNewPattern = std::make_unique<ScPatternAttr>(*pAttr); sal_uInt16 pItems[2]; pItems[0] = ATTR_CONDITIONAL; pItems[1] = 0; - aNewPattern.ClearItems(pItems); - pAttrArray->SetPatternArea(nRow1, nRow2, &aNewPattern, true); + pNewPattern->ClearItems(pItems); + pAttrArray->SetPatternArea(nRow1, nRow2, std::move(pNewPattern), true); } } diff --git a/sc/source/core/data/docpool.cxx b/sc/source/core/data/docpool.cxx index 3c21de8c8d69..74f60996cb5b 100644 --- a/sc/source/core/data/docpool.cxx +++ b/sc/source/core/data/docpool.cxx @@ -327,17 +327,17 @@ ScDocumentPool::~ScDocumentPool() } } -const SfxPoolItem& ScDocumentPool::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich ) +const SfxPoolItem& ScDocumentPool::PutImpl( const SfxPoolItem& rItem, sal_uInt16 nWhich, bool bPassingOwnership ) { if ( rItem.Which() != ATTR_PATTERN ) // Only Pattern is special - return SfxItemPool::Put( rItem, nWhich ); + return SfxItemPool::PutImpl( rItem, nWhich, bPassingOwnership ); // Don't copy the default pattern of this Pool if (&rItem == mvPoolDefaults[ ATTR_PATTERN - ATTR_STARTINDEX ]) return rItem; // Else Put must always happen, because it could be another Pool - const SfxPoolItem& rNew = SfxItemPool::Put( rItem, nWhich ); + const SfxPoolItem& rNew = SfxItemPool::PutImpl( rItem, nWhich, bPassingOwnership ); sal_uInt32 nRef = rNew.GetRefCount(); if (nRef == 1) { diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx index b4baa4b67da4..9c1875869099 100644 --- a/sc/source/core/data/document.cxx +++ b/sc/source/core/data/document.cxx @@ -5033,6 +5033,14 @@ bool ScDocument::RemoveFlagsTab( SCCOL nStartCol, SCROW nStartRow, return false; } +const ScPatternAttr* ScDocument::SetPattern( SCCOL nCol, SCROW nRow, SCTAB nTab, std::unique_ptr<ScPatternAttr> pAttr ) +{ + if (ValidTab(nTab) && nTab < static_cast<SCTAB>(maTabs.size())) + if (maTabs[nTab]) + return maTabs[nTab]->SetPattern( nCol, nRow, std::move(pAttr) ); + return nullptr; +} + void ScDocument::SetPattern( SCCOL nCol, SCROW nRow, SCTAB nTab, const ScPatternAttr& rAttr ) { if (ValidTab(nTab) && nTab < static_cast<SCTAB>(maTabs.size())) diff --git a/sc/source/core/data/table2.cxx b/sc/source/core/data/table2.cxx index 4880e02d45c7..f4656246bf22 100644 --- a/sc/source/core/data/table2.cxx +++ b/sc/source/core/data/table2.cxx @@ -2799,6 +2799,25 @@ bool ScTable::RemoveFlags( SCCOL nStartCol, SCROW nStartRow, SCCOL nEndCol, SCRO return bChanged; } +void ScTable::SetPattern( const ScAddress& rPos, std::unique_ptr<ScPatternAttr> pAttr ) +{ + if (ValidColRow(rPos.Col(),rPos.Row())) + aCol[rPos.Col()].SetPattern( rPos.Row(), std::move(pAttr) ); +} + +void ScTable::SetPattern( const ScAddress& rPos, const ScPatternAttr& rAttr ) +{ + if (ValidColRow(rPos.Col(),rPos.Row())) + aCol[rPos.Col()].SetPattern( rPos.Row(), rAttr ); +} + +const ScPatternAttr* ScTable::SetPattern( SCCOL nCol, SCROW nRow, std::unique_ptr<ScPatternAttr> pAttr ) +{ + if (ValidColRow(nCol,nRow)) + return aCol[nCol].SetPattern( nRow, std::move(pAttr) ); + return nullptr; +} + void ScTable::SetPattern( SCCOL nCol, SCROW nRow, const ScPatternAttr& rAttr ) { if (ValidColRow(nCol,nRow)) diff --git a/sc/source/core/data/table3.cxx b/sc/source/core/data/table3.cxx index 819bfa6f87db..d04d704937d2 100644 --- a/sc/source/core/data/table3.cxx +++ b/sc/source/core/data/table3.cxx @@ -1886,11 +1886,11 @@ static void lcl_RemoveNumberFormat( ScTable* pTab, SCCOL nCol, SCROW nRow ) if ( pPattern->GetItemSet().GetItemState( ATTR_VALUE_FORMAT, false ) == SfxItemState::SET ) { - ScPatternAttr aNewPattern( *pPattern ); - SfxItemSet& rSet = aNewPattern.GetItemSet(); + auto pNewPattern = std::make_unique<ScPatternAttr>( *pPattern ); + SfxItemSet& rSet = pNewPattern->GetItemSet(); rSet.ClearItem( ATTR_VALUE_FORMAT ); rSet.ClearItem( ATTR_LANGUAGE_FORMAT ); - pTab->SetPattern( nCol, nRow, aNewPattern ); + pTab->SetPattern( nCol, nRow, std::move(pNewPattern) ); } } diff --git a/sc/source/filter/rtf/eeimpars.cxx b/sc/source/filter/rtf/eeimpars.cxx index e49448b8f8a4..3f3710326292 100644 --- a/sc/source/filter/rtf/eeimpars.cxx +++ b/sc/source/filter/rtf/eeimpars.cxx @@ -213,35 +213,35 @@ void ScEEImport::WriteToDocument( bool bSizeColsRows, double nOutputFactor, SvNu } // Set attributes - ScPatternAttr aAttr( pDocPool ); - aAttr.GetFromEditItemSet( &aSet ); - SfxItemSet& rSet = aAttr.GetItemSet(); + auto pAttr = std::make_unique<ScPatternAttr>( pDocPool ); + pAttr->GetFromEditItemSet( &aSet ); + SfxItemSet* pAttrItemSet = &pAttr->GetItemSet(); if (!aNumStr.isEmpty()) { - rSet.Put( SfxUInt32Item( ATTR_VALUE_FORMAT, nNumForm ) ); - rSet.Put( SvxLanguageItem( eNumLang, ATTR_LANGUAGE_FORMAT ) ); + pAttrItemSet->Put( SfxUInt32Item( ATTR_VALUE_FORMAT, nNumForm ) ); + pAttrItemSet->Put( SvxLanguageItem( eNumLang, ATTR_LANGUAGE_FORMAT ) ); } const SfxItemSet& rESet = pE->aItemSet; if ( rESet.Count() ) { const SfxPoolItem* pItem; if ( rESet.GetItemState( ATTR_BACKGROUND, false, &pItem) == SfxItemState::SET ) - rSet.Put( *pItem ); + pAttrItemSet->Put( *pItem ); if ( rESet.GetItemState( ATTR_BORDER, false, &pItem) == SfxItemState::SET ) - rSet.Put( *pItem ); + pAttrItemSet->Put( *pItem ); if ( rESet.GetItemState( ATTR_SHADOW, false, &pItem) == SfxItemState::SET ) - rSet.Put( *pItem ); + pAttrItemSet->Put( *pItem ); // HTML if ( rESet.GetItemState( ATTR_HOR_JUSTIFY, false, &pItem) == SfxItemState::SET ) - rSet.Put( *pItem ); + pAttrItemSet->Put( *pItem ); if ( rESet.GetItemState( ATTR_VER_JUSTIFY, false, &pItem) == SfxItemState::SET ) - rSet.Put( *pItem ); + pAttrItemSet->Put( *pItem ); if ( rESet.GetItemState( ATTR_LINEBREAK, false, &pItem) == SfxItemState::SET ) - rSet.Put( *pItem ); + pAttrItemSet->Put( *pItem ); if ( rESet.GetItemState( ATTR_FONT_COLOR, false, &pItem) == SfxItemState::SET ) - rSet.Put( *pItem ); + pAttrItemSet->Put( *pItem ); if ( rESet.GetItemState( ATTR_FONT_UNDERLINE, false, &pItem) == SfxItemState::SET ) - rSet.Put( *pItem ); + pAttrItemSet->Put( *pItem ); // HTML LATIN/CJK/CTL script type dependent const SfxPoolItem* pFont; if ( rESet.GetItemState( ATTR_FONT, false, &pFont) != SfxItemState::SET ) @@ -258,7 +258,7 @@ void ScEEImport::WriteToDocument( bool bSizeColsRows, double nOutputFactor, SvNu // Number format const SfxPoolItem* pNumFmt = nullptr; if ( rESet.GetItemState(ATTR_VALUE_FORMAT, false, &pNumFmt) == SfxItemState::SET ) - rSet.Put(*pNumFmt); + pAttrItemSet->Put(*pNumFmt); if ( pFont || pHeight || pWeight || pPosture ) { OUString aStr( mpEngine->GetText( pE->aSel ) ); @@ -273,25 +273,25 @@ void ScEEImport::WriteToDocument( bool bSizeColsRows, double nOutputFactor, SvNu { std::unique_ptr<SfxPoolItem> pNewItem(pFont->CloneSetWhich( ScGlobal::GetScriptedWhichID(nScript, ATTR_FONT ))); - rSet.Put( *pNewItem ); + pAttrItemSet->Put( *pNewItem ); } if ( pHeight ) { std::unique_ptr<SfxPoolItem> pNewItem(pHeight->CloneSetWhich( ScGlobal::GetScriptedWhichID(nScript, ATTR_FONT_HEIGHT ))); - rSet.Put( *pNewItem ); + pAttrItemSet->Put( *pNewItem ); } if ( pWeight ) { std::unique_ptr<SfxPoolItem> pNewItem(pWeight->CloneSetWhich( ScGlobal::GetScriptedWhichID(nScript, ATTR_FONT_WEIGHT ))); - rSet.Put( *pNewItem ); + pAttrItemSet->Put( *pNewItem ); } if ( pPosture ) { std::unique_ptr<SfxPoolItem> pNewItem(pPosture->CloneSetWhich( ScGlobal::GetScriptedWhichID(nScript, ATTR_FONT_POSTURE ))); - rSet.Put( *pNewItem ); + pAttrItemSet->Put( *pNewItem ); } } } @@ -301,7 +301,7 @@ void ScEEImport::WriteToDocument( bool bSizeColsRows, double nOutputFactor, SvNu { // Merged cells, with SfxItemSet.Put() is faster than // with ScDocument.DoMerge() afterwards ScMergeAttr aMerge( pE->nColOverlap, pE->nRowOverlap ); - rSet.Put( aMerge ); + pAttrItemSet->Put( aMerge ); SCROW nRO = 0; if ( pE->nColOverlap > 1 ) mpDoc->ApplyFlagsTab( nCol+1, nRow, @@ -323,8 +323,8 @@ void ScEEImport::WriteToDocument( bool bSizeColsRows, double nOutputFactor, SvNu } const ScStyleSheet* pStyleSheet = mpDoc->GetPattern( nCol, nRow, nTab )->GetStyleSheet(); - aAttr.SetStyleSheet( const_cast<ScStyleSheet*>(pStyleSheet) ); - mpDoc->SetPattern( nCol, nRow, nTab, aAttr ); + pAttr->SetStyleSheet( const_cast<ScStyleSheet*>(pStyleSheet) ); + auto rAttrItemSet2 = mpDoc->SetPattern( nCol, nRow, nTab, std::move(pAttr) )->GetItemSet(); // Add data if (bSimple) @@ -359,7 +359,7 @@ void ScEEImport::WriteToDocument( bool bSizeColsRows, double nOutputFactor, SvNu bool bTextFormat = false; const SfxPoolItem* pNumFmt = nullptr; - if (rSet.GetItemState(ATTR_VALUE_FORMAT, false, &pNumFmt) == SfxItemState::SET) + if (rAttrItemSet2.GetItemState(ATTR_VALUE_FORMAT, false, &pNumFmt) == SfxItemState::SET) { sal_uInt32 nNumFmt = static_cast<const SfxUInt32Item*>(pNumFmt)->GetValue(); SvNumFormatType nType = pFormatter->GetType(nNumFmt); diff --git a/sc/source/ui/undo/undocell.cxx b/sc/source/ui/undo/undocell.cxx index 55e309c959df..debc9465a8d9 100644 --- a/sc/source/ui/undo/undocell.cxx +++ b/sc/source/ui/undo/undocell.cxx @@ -232,9 +232,9 @@ void ScUndoEnterData::Undo() SfxUInt32Item(ATTR_VALUE_FORMAT, rVal.mnFormat)); else { - ScPatternAttr aPattern(*rDoc.GetPattern(maPos.Col(), maPos.Row(), rVal.mnTab)); - aPattern.GetItemSet().ClearItem( ATTR_VALUE_FORMAT ); - rDoc.SetPattern(maPos.Col(), maPos.Row(), rVal.mnTab, aPattern); + auto pPattern = std::make_unique<ScPatternAttr>(*rDoc.GetPattern(maPos.Col(), maPos.Row(), rVal.mnTab)); + pPattern->GetItemSet().ClearItem( ATTR_VALUE_FORMAT ); + rDoc.SetPattern(maPos.Col(), maPos.Row(), rVal.mnTab, std::move(pPattern)); } pDocShell->PostPaintCell(maPos.Col(), maPos.Row(), rVal.mnTab); } diff --git a/svl/source/items/itempool.cxx b/svl/source/items/itempool.cxx index 67f141dbd16b..bb3ac17f417b 100644 --- a/svl/source/items/itempool.cxx +++ b/svl/source/items/itempool.cxx @@ -27,6 +27,7 @@ #include <sal/log.hxx> #include <svl/SfxBroadcaster.hxx> #include <svl/hint.hxx> +#include <svl/itemset.hxx> #include <poolio.hxx> #include <algorithm> @@ -578,7 +579,7 @@ void SfxItemPool::ResetPoolDefaultItem( sal_uInt16 nWhichId ) } -const SfxPoolItem& SfxItemPool::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich ) +const SfxPoolItem& SfxItemPool::PutImpl( const SfxPoolItem& rItem, sal_uInt16 nWhich, bool bPassingOwnership ) { if ( 0 == nWhich ) nWhich = rItem.Which(); @@ -588,7 +589,7 @@ const SfxPoolItem& SfxItemPool::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich if ( !bSID && !IsInRange(nWhich) ) { if ( pImpl->mpSecondary ) - return pImpl->mpSecondary->Put( rItem, nWhich ); + return pImpl->mpSecondary->PutImpl( rItem, nWhich, bPassingOwnership ); OSL_FAIL( "unknown WhichId - cannot put item" ); } @@ -601,6 +602,8 @@ const SfxPoolItem& SfxItemPool::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich SfxPoolItem *pPoolItem = rItem.Clone(pImpl->mpMaster); pPoolItem->SetWhich(nWhich); AddRef( *pPoolItem ); + if (bPassingOwnership) + delete &rItem; return *pPoolItem; } @@ -623,6 +626,7 @@ const SfxPoolItem& SfxItemPool::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich if (it != rItemArr.end()) { AddRef(rItem); + assert(!bPassingOwnership && "cant be passing ownership and have the item already in the pool"); return rItem; } } @@ -635,6 +639,8 @@ const SfxPoolItem& SfxItemPool::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich { assert(*pFoundItem == rItem); AddRef(*pFoundItem); + if (bPassingOwnership) + delete &rItem; return *pFoundItem; } } @@ -645,6 +651,7 @@ const SfxPoolItem& SfxItemPool::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich if (**itr == rItem) { AddRef(**itr); + assert(!bPassingOwnership && "cant be passing ownership and have the item already in the pool"); return **itr; } } @@ -652,7 +659,14 @@ const SfxPoolItem& SfxItemPool::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich } // 3. not found, so clone to insert into the pointer array. - SfxPoolItem* pNewItem = rItem.Clone(pImpl->mpMaster); + SfxPoolItem* pNewItem; + if (bPassingOwnership) + { + assert(!dynamic_cast<const SfxItemSet*>(&rItem) && "cant pass ownership of SfxItem, they need to be cloned to the master pool"); + pNewItem = const_cast<SfxPoolItem*>(&rItem); + } + else + pNewItem = rItem.Clone(pImpl->mpMaster); pNewItem->SetWhich(nWhich); assert(typeid(rItem) == typeid(*pNewItem) && "SfxItemPool::Put(): unequal types, no Clone() override?"); if (dynamic_cast<const SfxSetItem*>(&rItem) == nullptr) _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits