From 2b4cb63a4450aff4582994ca6ac701287da61ddd Mon Sep 17 00:00:00 2001 From: "Armin Le Grand (allotropia)" Date: Sun, 19 Nov 2023 17:48:31 +0100 Subject: [PATCH] Cleanup some ScPatternAttr specific stuff Removed some special stuff in the ItemSet/Item tooling regarding ScPatternAttr. There are still quite a view (grep for isExceptionalSCItem), but all these are identified and isolated. Only for that Item (of over 500) are these exceptions, so that raelly does not fit into the Item schemata in any way. Not even the pool default of ScPatternAttr is without trouble. It is the only and last Item that needs to be 'pooled' in the sense to avoid copies on any price. That is OK, but should not be done in the ItemSet schemata. In short: It should not be an Item at all. It should be changed and renamed to something describing it's role (CellAttributes..?) and get helped/assisted by something called CellAttributeHelper at SC's model or Pool. It should not even be derived from SetItem, it could just contain an shared_ptr of SfxItemSet (allows more and better optimizations - think about Clone() and op==). In parallel, all these hacks in the ItemSet/Item stuff could be removed, making that faster and easier. Also quite some usages of DirectPutItemInPool could be cleaned-up, this only exists since there *is* no defined place to hold that data (coud be CellAttributeHelper in the future). Putting Items directly to the pool (and in most cases never remove them again) is just nonsense and another hint that all this does not fit to the Item/ItemSet schema at all. This is now - after hard isolation of problems and have all working - doable. It may be one of the next things to do, but there are other candidates, too. Doing this would mostly only help SC... Found another hack that uses DirectPutItemInPool and *never* removes any Item again, see comments framelinkarray.cxx And another one: PoolItemTest::testPool() explicitely tests DirectPutItemInPool stuff - which makes no sense long-term, but for now keep it and make it work by marking the slots used as _bNeedsPoolRegistration == true Have now overhauled the framelinkarray stuff to work without DirectPutItemInPool and Cell not being a SfxPoolItem. That will be much less complex and use much less calls/checks. Since this is the data structure created for every calc repaint that should get faster, too. Also for now and memory loss security I added code to DirectPutItemInPool to behave the same as with the unchanged implCreateItemEntry: register Items so that the garbage collection still is used. This will/can be removed when all usages of DirectPutItemInPool is cleaned up. Directly registering in DirectPutItemInPoolImpl is more tricky than thought, but a good thing to do. Looking at that I saw that tryRegisterSfxPoolItem is not used anymore, so rearranged some stuff. Change-Id: If07762b0a25e2739027f81872441772dc82a25d9 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159685 Tested-by: Jenkins Reviewed-by: Armin Le Grand --- include/svl/itempool.hxx | 14 +--- include/svl/itemset.hxx | 4 +- include/svl/poolitem.hxx | 8 +- sc/source/core/data/patattr.cxx | 8 +- svl/qa/unit/items/test_itempool.cxx | 2 +- svl/source/items/itempool.cxx | 55 ++++--------- svl/source/items/itemset.cxx | 40 ++++------ svl/source/items/poolitem.cxx | 2 +- svx/source/dialog/framelink.cxx | 4 + svx/source/dialog/framelinkarray.cxx | 115 +++++++++++++++++---------- 10 files changed, 123 insertions(+), 129 deletions(-) diff --git a/include/svl/itempool.hxx b/include/svl/itempool.hxx index 9bdb5d9a3504a..f5949f1700c80 100644 --- a/include/svl/itempool.hxx +++ b/include/svl/itempool.hxx @@ -74,7 +74,7 @@ class SVL_DLLPUBLIC SfxItemPool : public salhelper::SimpleReferenceObject friend class SfxAllItemSet; // allow ItemSetTooling to access - friend SfxPoolItem const* implCreateItemEntry(SfxItemPool&, SfxPoolItem const*, sal_uInt16, bool, bool); + friend SfxPoolItem const* implCreateItemEntry(SfxItemPool&, SfxPoolItem const*, sal_uInt16, bool); friend void implCleanupItemEntry(SfxItemPool&, SfxPoolItem const*); // unit testing @@ -230,16 +230,8 @@ public: static bool IsSlot(sal_uInt16 nId) { return nId && nId > SFX_WHICH_MAX; } - // this method tries to register an Item at this Pool. If this - // is done depends on the SfxItemInfo-flag _bNeedsPoolRegistration - // which needs to be set for Items that are accessed using - // GetItemSurrogates, else the Item will not be returned/accessed - void tryRegisterSfxPoolItem(const SfxPoolItem& rItem, bool bPoolDirect); - - // this method will register the Item at this Pool, no matter what. - // It is needed for all calls that directly register Items at the - // Pool, so the DirectPutItemInPool-methods - void doRegisterSfxPoolItem(const SfxPoolItem& rItem); + // This method will try to register the Item at this Pool. + void registerSfxPoolItem(const SfxPoolItem& rItem); // this method will unregister an Item from this Pool void unregisterSfxPoolItem(const SfxPoolItem& rItem); diff --git a/include/svl/itemset.hxx b/include/svl/itemset.hxx index 06e82b87e054c..998830c6542a5 100644 --- a/include/svl/itemset.hxx +++ b/include/svl/itemset.hxx @@ -37,7 +37,7 @@ SVL_DLLPUBLIC size_t getUsedSfxItemSetCount(); #endif // ItemSet/ItemPool helpers -SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool, SfxPoolItem const* pSource, sal_uInt16 nWhich, bool bPassingOwnership, bool bPoolDirect); +SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool, SfxPoolItem const* pSource, sal_uInt16 nWhich, bool bPassingOwnership); void implCleanupItemEntry(SfxItemPool& rPool, SfxPoolItem const* pSource); class SAL_WARN_UNUSED SVL_DLLPUBLIC SfxItemSet @@ -46,7 +46,7 @@ class SAL_WARN_UNUSED SVL_DLLPUBLIC SfxItemSet friend class SfxWhichIter; // allow ItemSetTooling to access - friend SfxPoolItem const* implCreateItemEntry(SfxItemPool&, SfxPoolItem const*, sal_uInt16, bool, bool); + friend SfxPoolItem const* implCreateItemEntry(SfxItemPool&, SfxPoolItem const*, sal_uInt16, bool); friend void implCleanupItemEntry(SfxItemPool&, SfxPoolItem const*); SfxItemPool* m_pPool; ///< pool that stores the items diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx index b434302a2f857..d5499351bd1e0 100644 --- a/include/svl/poolitem.hxx +++ b/include/svl/poolitem.hxx @@ -115,7 +115,7 @@ class SVL_DLLPUBLIC SfxPoolItem friend class SfxItemSet; // allow ItemSetTooling to access - friend SfxPoolItem const* implCreateItemEntry(SfxItemPool&, SfxPoolItem const*, sal_uInt16, bool, bool); + friend SfxPoolItem const* implCreateItemEntry(SfxItemPool&, SfxPoolItem const*, sal_uInt16, bool); friend void implCleanupItemEntry(SfxItemPool&, SfxPoolItem const*); mutable sal_uInt32 m_nRefCount; @@ -136,7 +136,7 @@ class SVL_DLLPUBLIC SfxPoolItem bool m_bStaticDefault : 1; // bit 2 bool m_bPoolDefault : 1; // bit 3 bool m_bRegisteredAtPool : 1; // bit 4 - bool m_bNewItemCallback : 1; // bit 5 + bool m_bExceptionalSCItem : 1; // bit 5 bool m_bIsSetItem : 1; // bit 6 protected: @@ -158,7 +158,7 @@ protected: void setStaticDefault() { m_bStaticDefault = true; } void setPoolDefault() { m_bPoolDefault = true; } void setRegisteredAtPool(bool bNew) { m_bRegisteredAtPool = bNew; } - void setNewItemCallback() { m_bNewItemCallback = true; } + void setExceptionalSCItem() { m_bExceptionalSCItem = true; } void setIsSetItem() { m_bIsSetItem = true; } public: @@ -178,7 +178,7 @@ public: bool isStaticDefault() const { return m_bStaticDefault; } bool isPoolDefault() const { return m_bPoolDefault; } bool isRegisteredAtPool() const { return m_bRegisteredAtPool; } - bool isNewItemCallback() const { return m_bNewItemCallback; } + bool isExceptionalSCItem() const { return m_bExceptionalSCItem; } bool isSetItem() const { return m_bIsSetItem; } // version that allows nullptrs diff --git a/sc/source/core/data/patattr.cxx b/sc/source/core/data/patattr.cxx index 7fe65bc82ba94..4bb79828da133 100644 --- a/sc/source/core/data/patattr.cxx +++ b/sc/source/core/data/patattr.cxx @@ -76,7 +76,7 @@ ScPatternAttr::ScPatternAttr( SfxItemSet&& pItemSet, const OUString& rStyleName pStyle ( nullptr ), mnPAKey(0) { - setNewItemCallback(); + setExceptionalSCItem(); // We need to ensure that ScPatternAttr is using the correct WhichRange, // see comments in commit message. This does transfers the items with @@ -90,7 +90,7 @@ ScPatternAttr::ScPatternAttr( SfxItemSet&& pItemSet ) pStyle ( nullptr ), mnPAKey(0) { - setNewItemCallback(); + setExceptionalSCItem(); // We need to ensure that ScPatternAttr is using the correct WhichRange, // see comments in commit message. This does transfers the items with @@ -104,7 +104,7 @@ ScPatternAttr::ScPatternAttr( SfxItemPool* pItemPool ) pStyle ( nullptr ), mnPAKey(0) { - setNewItemCallback(); + setExceptionalSCItem(); } ScPatternAttr::ScPatternAttr( const ScPatternAttr& rPatternAttr ) @@ -113,7 +113,7 @@ ScPatternAttr::ScPatternAttr( const ScPatternAttr& rPatternAttr ) pStyle ( rPatternAttr.pStyle ), mnPAKey(rPatternAttr.mnPAKey) { - setNewItemCallback(); + setExceptionalSCItem(); } ScPatternAttr* ScPatternAttr::Clone( SfxItemPool *pPool ) const diff --git a/svl/qa/unit/items/test_itempool.cxx b/svl/qa/unit/items/test_itempool.cxx index c8e313bb61e50..2cb751d4fd77a 100644 --- a/svl/qa/unit/items/test_itempool.cxx +++ b/svl/qa/unit/items/test_itempool.cxx @@ -37,7 +37,7 @@ void PoolItemTest::testPool() SfxItemInfo const aItems[] = { // _nSID, _bNeedsPoolRegistration, _bShareable - { 4, false, true }, + { 4, true, true }, { 3, true, false /* test NeedsPoolRegistration */ }, { 2, false, false }, { 1, true, false /* test NeedsPoolRegistration */} diff --git a/svl/source/items/itempool.cxx b/svl/source/items/itempool.cxx index 0d3275471eac3..f119bc85304f2 100644 --- a/svl/source/items/itempool.cxx +++ b/svl/source/items/itempool.cxx @@ -801,8 +801,19 @@ const SfxPoolItem& SfxItemPool::DirectPutItemInPoolImpl(const SfxPoolItem& rItem nRemainingDirectlyPooledSfxPoolItemCount++; #endif + // CAUTION: Do not register the problematic pool default + if (rItem.isExceptionalSCItem() && GetMasterPool()->newItem_UseDirect(rItem)) + return rItem; + // make sure to use 'master'-pool, that's the one used by SfxItemSets - return *implCreateItemEntry(*GetMasterPool(), &rItem, nWhich, bPassingOwnership, true); + const SfxPoolItem* pRetval(implCreateItemEntry(*GetMasterPool(), &rItem, nWhich, bPassingOwnership)); + + // For the moment, as long as DirectPutItemInPoolImpl is used, make sure that + // the changes in implCreateItemEntry do not change anything, that would + // risc memory leaks by not (ab)using the garbage collector aspect of the pool. + registerSfxPoolItem(*pRetval); + + return *pRetval; } void SfxItemPool::DirectRemoveItemFromPool(const SfxPoolItem& rItem) @@ -1058,7 +1069,7 @@ sal_uInt16 SfxItemPool::GetTrueSlotId( sal_uInt16 nWhich ) const return pItemInfos[nWhich - pImpl->mnStart]._nSID; } -void SfxItemPool::tryRegisterSfxPoolItem(const SfxPoolItem& rItem, bool bPoolDirect) +void SfxItemPool::registerSfxPoolItem(const SfxPoolItem& rItem) { assert(rItem.Which() != 0); @@ -1067,7 +1078,7 @@ void SfxItemPool::tryRegisterSfxPoolItem(const SfxPoolItem& rItem, bool bPoolDir return; if (rItem.isRegisteredAtPool()) - // already registered, done (should not happen) + // already registered, done return; if (!IsInRange(rItem.Which())) @@ -1075,49 +1086,13 @@ void SfxItemPool::tryRegisterSfxPoolItem(const SfxPoolItem& rItem, bool bPoolDir // get to the right pool if (pImpl->mpSecondary) { - pImpl->mpSecondary->tryRegisterSfxPoolItem(rItem, bPoolDirect); + pImpl->mpSecondary->registerSfxPoolItem(rItem); return; } return; } - // get index (must exist due to checks above) - const sal_uInt16 nIndex(rItem.Which() - pImpl->mnStart); - bool bRegisterItem(bPoolDirect); - - if (!bRegisterItem) - { - if (g_bItemClassicMode) - { - // classic mode: register in general *all* items - // ...but only shareable ones: for non-shareable registering for re-use - // makes no sense - bRegisterItem = Shareable_Impl(nIndex); - } - else - { - // experimental mode: only register items that are defined to be registered - bRegisterItem = NeedsPoolRegistration_Impl(nIndex); - } - } - - if (bRegisterItem) - doRegisterSfxPoolItem(rItem); -} - -void SfxItemPool::doRegisterSfxPoolItem(const SfxPoolItem& rItem) -{ - assert(rItem.Which() != 0); - - if (IsSlot(rItem.Which())) - // do not register SlotItems - return; - - if (rItem.isRegisteredAtPool()) - // already registered, done - return; - if (nullptr == ppRegisteredSfxPoolItems) // on-demand allocate array of registeredSfxPoolItems and init to nullptr ppRegisteredSfxPoolItems = new registeredSfxPoolItems*[GetSize_Impl()]{}; diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx index 6cf4e706e17db..abb2644228283 100644 --- a/svl/source/items/itemset.cxx +++ b/svl/source/items/itemset.cxx @@ -128,7 +128,7 @@ SfxItemSet::SfxItemSet(SfxItemPool& pool, WhichRangesContainer wids) assert(svl::detail::validRanges2(m_pWhichRanges)); } -SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool, SfxPoolItem const* pSource, sal_uInt16 nWhich, bool bPassingOwnership, bool bPoolDirect) +SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool, SfxPoolItem const* pSource, sal_uInt16 nWhich, bool bPassingOwnership) { if (nullptr == pSource) // SfxItemState::UNKNOWN aka current default (nullptr) @@ -140,7 +140,7 @@ SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool, SfxPoolItem const* pS // just use pSource which equals INVALID_POOL_ITEM return pSource; - if (pSource->isNewItemCallback() && rPool.GetMasterPool()->newItem_UseDirect(*pSource)) + if (pSource->isExceptionalSCItem() && rPool.GetMasterPool()->newItem_UseDirect(*pSource)) // exceptional handling for *some* items, see SC // (do not copy item: use directly, it is a pool default) return pSource; @@ -240,22 +240,14 @@ SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool, SfxPoolItem const* pS return pSource; } - // classic mode: try finding already existing item - // NOTE: bPoolDirect currently required due to DirectPutItemInPool and the - // self-handled Item ScPatternAttr/ATTR_PATTERN in SC, else e.g. - // testIteratorsDefPattern will fail in line 1306 + // g_bItemClassicMode: try finding already existing item // NOTE: the UnitTest testIteratorsDefPattern claims that that Item "can be // edited by the user" which explains why it breaks so many rules for Items, // it behaves like an alien. That Item in the SC Pool claims to be a // 'StaticDefault' and gets changed (..?) - // NOTE: despite 1st thinking that this can be limited to ScPatternAttr/ - // ATTR_PATTERN it also has to be applied to the range - // [ATTR_PATTERN_START, ATTR_PATTERN_END] *used* by ATTR_PATTERN, plus - // it, so it's [100 .. 155] and [156] in SC. For now, just use bPoolDirect. - // This needs to be cleaned-up somehow anyways // only do this if classic mode or required (calls from Pool::Direct*) - while(g_bItemClassicMode || bPoolDirect) + while(g_bItemClassicMode || pSource->isExceptionalSCItem()) { if (!pTargetPool->Shareable_Impl(nIndex)) // not shareable, so no need to search for identical item @@ -308,29 +300,31 @@ SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool, SfxPoolItem const* pS // Unfortunately e,g, SC does 'special' things for some new Items, // so we need to give the opportunity for this. To limit this to - // the needed cases, use m_bNewItemCallback flag at item - if (pSource->isNewItemCallback()) + // the needed cases, use m_bExceptionalSCItem flag at item + if (pSource->isExceptionalSCItem()) pMasterPool->newItem_Callback(*pSource); // try to register @Pool (only needed if not yet registered) if (!pSource->isRegisteredAtPool()) { - if (!bPoolDirect) // re-use bPoolDirect + bool bRegisterAtPool(pSource->isExceptionalSCItem()); + + if (!bRegisterAtPool) { if (g_bItemClassicMode) { // in classic mode register only/all shareable items - bPoolDirect = pTargetPool->Shareable_Impl(nIndex); + bRegisterAtPool = pTargetPool->Shareable_Impl(nIndex); } else { // in new mode register only/all items marked as need to be registered - bPoolDirect = pTargetPool->NeedsPoolRegistration_Impl(nIndex); + bRegisterAtPool = pTargetPool->NeedsPoolRegistration_Impl(nIndex); } } - if (bPoolDirect) - pTargetPool->doRegisterSfxPoolItem(*pSource); + if (bRegisterAtPool) + pTargetPool->registerSfxPoolItem(*pSource); } return pSource; @@ -346,7 +340,7 @@ void implCleanupItemEntry(SfxItemPool& rPool, SfxPoolItem const* pSource) // nothing to do for invalid item entries return; - if (pSource->isNewItemCallback() && rPool.GetMasterPool()->newItem_UseDirect(*pSource)) + if (pSource->isExceptionalSCItem() && rPool.GetMasterPool()->newItem_UseDirect(*pSource)) // exceptional handling for *some* items, see SC // do not delete Item, it is a pool default return; @@ -421,7 +415,7 @@ SfxItemSet::SfxItemSet( const SfxItemSet& rASet ) for (const auto& rSource : rASet) { - *ppDst = implCreateItemEntry(*GetPool(), rSource, 0, false, false); + *ppDst = implCreateItemEntry(*GetPool(), rSource, 0, false); ppDst++; } @@ -702,7 +696,7 @@ const SfxPoolItem* SfxItemSet::PutImpl(const SfxPoolItem& rItem, sal_uInt16 nWhi } // prepare new entry - SfxPoolItem const* pNew(implCreateItemEntry(*GetPool(), &rItem, nWhich, bPassingOwnership, false)); + SfxPoolItem const* pNew(implCreateItemEntry(*GetPool(), &rItem, nWhich, bPassingOwnership)); // Notification-Callback if(m_aCallback) @@ -1311,7 +1305,7 @@ void SfxItemSet::MergeItem_Impl(const SfxPoolItem **ppFnd1, const SfxPoolItem *p else if ( pFnd2 && bIgnoreDefaults ) // Decision table: default, set, doesn't matter, sal_True - *ppFnd1 = implCreateItemEntry(*GetPool(), pFnd2, 0, false, false); + *ppFnd1 = implCreateItemEntry(*GetPool(), pFnd2, 0, false); // *ppFnd1 = &GetPool()->Put( *pFnd2 ); if ( *ppFnd1 ) diff --git a/svl/source/items/poolitem.cxx b/svl/source/items/poolitem.cxx index 49990079367c0..bf86b4ab740ef 100644 --- a/svl/source/items/poolitem.cxx +++ b/svl/source/items/poolitem.cxx @@ -499,7 +499,7 @@ SfxPoolItem::SfxPoolItem(sal_uInt16 const nWhich) , m_bStaticDefault(false) , m_bPoolDefault(false) , m_bRegisteredAtPool(false) - , m_bNewItemCallback(false) + , m_bExceptionalSCItem(false) , m_bIsSetItem(false) #ifdef DBG_UTIL , m_bDeleted(false) diff --git a/svx/source/dialog/framelink.cxx b/svx/source/dialog/framelink.cxx index 77098bd3e80d6..fbc5ee1d55336 100644 --- a/svx/source/dialog/framelink.cxx +++ b/svx/source/dialog/framelink.cxx @@ -178,6 +178,10 @@ Style& Style::MirrorSelf() bool Style::operator==( const Style& rOther) const { + if (this == &rOther) + // ptr compare (same instance) + return true; + return (Prim() == rOther.Prim() && Dist() == rOther.Dist() && Secn() == rOther.Secn() diff --git a/svx/source/dialog/framelinkarray.cxx b/svx/source/dialog/framelinkarray.cxx index c5efc9f2b4226..afe6a2d7dd51f 100644 --- a/svx/source/dialog/framelinkarray.cxx +++ b/svx/source/dialog/framelinkarray.cxx @@ -24,14 +24,12 @@ #include #include #include -#include #include #include #include #include #include #include -// #include //#define OPTICAL_CHECK_CLIPRANGE_FOR_MERGED_CELL #ifdef OPTICAL_CHECK_CLIPRANGE_FOR_MERGED_CELL @@ -43,7 +41,7 @@ namespace svx::frame { namespace { -class Cell final : public SfxPoolItem +class Cell final { private: Style maLeft; @@ -72,8 +70,7 @@ class Cell final : public SfxPoolItem explicit Cell(); explicit Cell(const Cell&) = default; - virtual bool operator==( const SfxPoolItem& ) const override; - virtual Cell* Clone( SfxItemPool *pPool = nullptr ) const override; + bool operator==( const Cell& ) const; void SetStyleLeft(const Style& rStyle) { maLeft = rStyle; } void SetStyleRight(const Style& rStyle) { maRight = rStyle; } @@ -201,7 +198,6 @@ basegfx::B2DHomMatrix Cell::CreateCoordinateSystemMergedCell( } Cell::Cell() : - SfxPoolItem(10), mnAddLeft( 0 ), mnAddRight( 0 ), mnAddTop( 0 ), @@ -213,16 +209,12 @@ Cell::Cell() : { } -Cell* Cell::Clone(SfxItemPool* /*pPool*/) const +bool Cell::operator==(const Cell& rOther) const { - return new Cell(*this); -} + if (this == &rOther) + // ptr compare (same instance) + return true; -bool Cell::operator==(const SfxPoolItem& rItem) const -{ - if (!SfxPoolItem::operator==(rItem)) - return false; - const Cell& rOther = static_cast(rItem); return maLeft == rOther.maLeft && maRight == rOther.maRight && maTop == rOther.maTop @@ -259,31 +251,12 @@ static void lclRecalcCoordVec( std::vector& rCoords, const std::vecto } } -static void lclSetMergedRange( SfxItemPool& rPool, CellVec& rCells, sal_Int32 nWidth, sal_Int32 nFirstCol, sal_Int32 nFirstRow, sal_Int32 nLastCol, sal_Int32 nLastRow ) -{ - for( sal_Int32 nCol = nFirstCol; nCol <= nLastCol; ++nCol ) - { - for( sal_Int32 nRow = nFirstRow; nRow <= nLastRow; ++nRow ) - { - const Cell* pCell = rCells[ nRow * nWidth + nCol ]; - Cell aTempCell(*pCell); - aTempCell.mbOverlapX = nCol > nFirstCol; - aTempCell.mbOverlapY = nRow > nFirstRow; - rCells[ nRow * nWidth + nCol ] = &rPool.DirectPutItemInPool(aTempCell); - } - } - Cell aTempCell(*rCells[ nFirstRow * nWidth + nFirstCol ]); - rCells[ nFirstRow * nWidth + nFirstCol ] = &rPool.DirectPutItemInPool(aTempCell); -} - - const Style OBJ_STYLE_NONE; const Cell OBJ_CELL_NONE; struct ArrayImpl { - // used to reduce the memory consumption of cells - rtl::Reference mxPool; + std::unordered_set maRegisteredCells; CellVec maCells; std::vector maWidths; std::vector maHeights; @@ -300,6 +273,7 @@ struct ArrayImpl bool mbMayHaveCellRotation; explicit ArrayImpl( sal_Int32 nWidth, sal_Int32 nHeight ); + ~ArrayImpl(); bool IsValidPos( sal_Int32 nCol, sal_Int32 nRow ) const { return (nCol < mnWidth) && (nRow < mnHeight); } @@ -334,15 +308,29 @@ struct ArrayImpl sal_Int32 GetRowPosition( sal_Int32 nRow ) const; bool HasCellRotation() const; + + Cell* createOrFind(const Cell& rCell); }; -const SfxItemInfo maItemInfos[] +static void lclSetMergedRange( ArrayImpl& rImpl, CellVec& rCells, sal_Int32 nWidth, sal_Int32 nFirstCol, sal_Int32 nFirstRow, sal_Int32 nLastCol, sal_Int32 nLastRow ) { - // _nSID, _bNeedsPoolRegistration, _bShareable - {0, false, true } -}; + for( sal_Int32 nCol = nFirstCol; nCol <= nLastCol; ++nCol ) + { + for( sal_Int32 nRow = nFirstRow; nRow <= nLastRow; ++nRow ) + { + const Cell* pCell = rCells[ nRow * nWidth + nCol ]; + Cell aTempCell(*pCell); + aTempCell.mbOverlapX = nCol > nFirstCol; + aTempCell.mbOverlapY = nRow > nFirstRow; + rCells[ nRow * nWidth + nCol ] = rImpl.createOrFind(aTempCell); + } + } + Cell aTempCell(*rCells[ nFirstRow * nWidth + nFirstCol ]); + rCells[ nFirstRow * nWidth + nFirstCol ] = rImpl.createOrFind(aTempCell); +} + ArrayImpl::ArrayImpl( sal_Int32 nWidth, sal_Int32 nHeight ) : - mxPool(new SfxItemPool("Mine", 10, 10, maItemInfos)), + maRegisteredCells(), mnWidth( nWidth ), mnHeight( nHeight ), mnFirstClipCol( 0 ), @@ -353,7 +341,7 @@ ArrayImpl::ArrayImpl( sal_Int32 nWidth, sal_Int32 nHeight ) : mbYCoordsDirty( false ), mbMayHaveCellRotation( false ) { - const Cell* pDefaultCell = &mxPool->DirectPutItemInPool(Cell()); + const Cell* pDefaultCell = createOrFind(Cell()); // default-construct all vectors maCells.resize( mnWidth * mnHeight, pDefaultCell ); maWidths.resize( mnWidth, 0 ); @@ -362,6 +350,23 @@ ArrayImpl::ArrayImpl( sal_Int32 nWidth, sal_Int32 nHeight ) : maYCoords.resize( mnHeight + 1, 0 ); } +ArrayImpl::~ArrayImpl() +{ + for (auto* pCell : maRegisteredCells) + delete pCell; +} + +Cell* ArrayImpl::createOrFind(const Cell& rCell) +{ + for (auto* pCell : maRegisteredCells) + if (*pCell == rCell) + return pCell; + + Cell* pRetval(new Cell(rCell)); + maRegisteredCells.insert(pRetval); + return pRetval; +} + const Cell& ArrayImpl::GetCell( sal_Int32 nCol, sal_Int32 nRow ) const { return IsValidPos( nCol, nRow ) ? *maCells[ GetIndex( nCol, nRow ) ] : OBJ_CELL_NONE; @@ -370,7 +375,7 @@ const Cell& ArrayImpl::GetCell( sal_Int32 nCol, sal_Int32 nRow ) const void ArrayImpl::PutCell( sal_Int32 nCol, sal_Int32 nRow, const Cell & rCell ) { if (IsValidPos( nCol, nRow )) - maCells[ GetIndex( nCol, nRow ) ] = &mxPool->DirectPutItemInPool(rCell); + maCells[ GetIndex( nCol, nRow ) ] = createOrFind(rCell); } sal_Int32 ArrayImpl::GetMergedFirstCol( sal_Int32 nCol, sal_Int32 nRow ) const @@ -599,6 +604,8 @@ void Array::SetCellStyleLeft( sal_Int32 nCol, sal_Int32 nRow, const Style& rStyl { DBG_FRAME_CHECK_COLROW( nCol, nRow, "SetCellStyleLeft" ); Cell aTempCell(CELL(nCol, nRow)); + if (aTempCell.GetStyleLeft() == rStyle) + return; aTempCell.SetStyleLeft(rStyle); PUTCELL( nCol, nRow, aTempCell ); } @@ -607,6 +614,8 @@ void Array::SetCellStyleRight( sal_Int32 nCol, sal_Int32 nRow, const Style& rSty { DBG_FRAME_CHECK_COLROW( nCol, nRow, "SetCellStyleRight" ); Cell aTempCell(CELL(nCol, nRow)); + if (aTempCell.GetStyleRight() == rStyle) + return; aTempCell.SetStyleRight(rStyle); PUTCELL( nCol, nRow, aTempCell ); } @@ -615,6 +624,8 @@ void Array::SetCellStyleTop( sal_Int32 nCol, sal_Int32 nRow, const Style& rStyle { DBG_FRAME_CHECK_COLROW( nCol, nRow, "SetCellStyleTop" ); Cell aTempCell(CELL(nCol, nRow)); + if (aTempCell.GetStyleTop() == rStyle) + return; aTempCell.SetStyleTop(rStyle); PUTCELL( nCol, nRow, aTempCell ); } @@ -623,6 +634,8 @@ void Array::SetCellStyleBottom( sal_Int32 nCol, sal_Int32 nRow, const Style& rSt { DBG_FRAME_CHECK_COLROW( nCol, nRow, "SetCellStyleBottom" ); Cell aTempCell(CELL(nCol, nRow)); + if (aTempCell.GetStyleBottom() == rStyle) + return; aTempCell.SetStyleBottom(rStyle); PUTCELL( nCol, nRow, aTempCell ); } @@ -631,6 +644,8 @@ void Array::SetCellStyleTLBR( sal_Int32 nCol, sal_Int32 nRow, const Style& rStyl { DBG_FRAME_CHECK_COLROW( nCol, nRow, "SetCellStyleTLBR" ); Cell aTempCell(CELL(nCol, nRow)); + if (aTempCell.GetStyleTLBR() == rStyle) + return; aTempCell.SetStyleTLBR(rStyle); PUTCELL( nCol, nRow, aTempCell ); } @@ -639,6 +654,8 @@ void Array::SetCellStyleBLTR( sal_Int32 nCol, sal_Int32 nRow, const Style& rStyl { DBG_FRAME_CHECK_COLROW( nCol, nRow, "SetCellStyleBLTR" ); Cell aTempCell(CELL(nCol, nRow)); + if (aTempCell.GetStyleBLTR() == rStyle) + return; aTempCell.SetStyleBLTR(rStyle); PUTCELL( nCol, nRow, aTempCell ); } @@ -647,6 +664,8 @@ void Array::SetCellStyleDiag( sal_Int32 nCol, sal_Int32 nRow, const Style& rTLBR { DBG_FRAME_CHECK_COLROW( nCol, nRow, "SetCellStyleDiag" ); Cell aTempCell(CELL(nCol, nRow)); + if (aTempCell.GetStyleTLBR() == rTLBR && aTempCell.GetStyleBLTR() == rBLTR) + return; aTempCell.SetStyleTLBR(rTLBR); aTempCell.SetStyleBLTR(rBLTR); PUTCELL( nCol, nRow, aTempCell ); @@ -684,6 +703,8 @@ void Array::SetCellRotation(sal_Int32 nCol, sal_Int32 nRow, SvxRotateMode eRotMo { DBG_FRAME_CHECK_COLROW(nCol, nRow, "SetCellRotation"); Cell aTempCell(CELL(nCol, nRow)); + if (aTempCell.meRotMode == eRotMode && aTempCell.mfOrientation == fOrientation) + return; aTempCell.meRotMode = eRotMode; aTempCell.mfOrientation = fOrientation; PUTCELL( nCol, nRow, aTempCell ); @@ -852,7 +873,7 @@ void Array::SetMergedRange( sal_Int32 nFirstCol, sal_Int32 nFirstRow, sal_Int32 } #endif if( mxImpl->IsValidPos( nFirstCol, nFirstRow ) && mxImpl->IsValidPos( nLastCol, nLastRow ) ) - lclSetMergedRange( *mxImpl->mxPool, mxImpl->maCells, mxImpl->mnWidth, nFirstCol, nFirstRow, nLastCol, nLastRow ); + lclSetMergedRange( *mxImpl, mxImpl->maCells, mxImpl->mnWidth, nFirstCol, nFirstRow, nLastCol, nLastRow ); } void Array::SetAddMergedLeftSize( sal_Int32 nCol, sal_Int32 nRow, sal_Int32 nAddSize ) @@ -862,6 +883,8 @@ void Array::SetAddMergedLeftSize( sal_Int32 nCol, sal_Int32 nRow, sal_Int32 nAdd for( MergedCellIterator aIt( *this, nCol, nRow ); aIt.Is(); ++aIt ) { Cell aTempCell(CELL(aIt.Col(), aIt.Row())); + if (aTempCell.mnAddLeft == nAddSize) + return; aTempCell.mnAddLeft = nAddSize; PUTCELL( nCol, nRow, aTempCell ); } @@ -874,6 +897,8 @@ void Array::SetAddMergedRightSize( sal_Int32 nCol, sal_Int32 nRow, sal_Int32 nAd for( MergedCellIterator aIt( *this, nCol, nRow ); aIt.Is(); ++aIt ) { Cell aTempCell(CELL(aIt.Col(), aIt.Row())); + if (aTempCell.mnAddRight == nAddSize) + return; aTempCell.mnAddRight = nAddSize; PUTCELL( nCol, nRow, aTempCell ); } @@ -886,6 +911,8 @@ void Array::SetAddMergedTopSize( sal_Int32 nCol, sal_Int32 nRow, sal_Int32 nAddS for( MergedCellIterator aIt( *this, nCol, nRow ); aIt.Is(); ++aIt ) { Cell aTempCell(CELL(aIt.Col(), aIt.Row())); + if (aTempCell.mnAddTop == nAddSize) + return; aTempCell.mnAddTop = nAddSize; PUTCELL( nCol, nRow, aTempCell ); } @@ -898,6 +925,8 @@ void Array::SetAddMergedBottomSize( sal_Int32 nCol, sal_Int32 nRow, sal_Int32 nA for( MergedCellIterator aIt( *this, nCol, nRow ); aIt.Is(); ++aIt ) { Cell aTempCell(CELL(aIt.Col(), aIt.Row())); + if (aTempCell.mnAddBottom == nAddSize) + return; aTempCell.mnAddBottom = nAddSize; PUTCELL( nCol, nRow, aTempCell ); } @@ -1063,7 +1092,7 @@ void Array::MirrorSelfX() { Cell aTempCell(CELL(mxImpl->GetMirrorCol( nCol ), nRow)); aTempCell.MirrorSelfX(); - aNewCells.push_back( &mxImpl->mxPool->DirectPutItemInPool(aTempCell) ); + aNewCells.push_back( mxImpl->createOrFind(aTempCell) ); } } mxImpl->maCells.swap( aNewCells );