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 <Armin.Le.Grand@me.com>

10 files changed, 123 insertions, 129 deletions

diff --git a/include/svl/itempool.hxx b/include/svl/itempool.hxx
index 9bdb5d9a3504..f5949f1700c8 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 06e82b87e054..998830c6542a 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 b434302a2f85..d5499351bd1e 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 7fe65bc82ba9..4bb79828da13 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 c8e313bb61e5..2cb751d4fd77 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 0d3275471eac..f119bc85304f 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 6cf4e706e17d..abb264422828 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 49990079367c..bf86b4ab740e 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 77098bd3e80d..fbc5ee1d5533 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 c5efc9f2b422..afe6a2d7dd51 100644
--- a/svx/source/dialog/framelinkarray.cxx
+++ b/svx/source/dialog/framelinkarray.cxx
@@ -24,14 +24,12 @@
 #include <set>
 #include <unordered_set>
 #include <algorithm>
-#include <svl/itempool.hxx>
 #include <tools/debug.hxx>
 #include <tools/gen.hxx>
 #include <vcl/canvastools.hxx>
 #include <svx/sdr/primitive2d/sdrframeborderprimitive2d.hxx>
 #include <basegfx/matrix/b2dhommatrixtools.hxx>
 #include <basegfx/polygon/b2dpolygonclipper.hxx>
-// #include <basegfx/numeric/ftools.hxx>
 
 //#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 @@ public:
     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<const Cell&>(rItem);
     return maLeft == rOther.maLeft
         && maRight == rOther.maRight
         && maTop == rOther.maTop
@@ -259,31 +251,12 @@ static void lclRecalcCoordVec( std::vector<sal_Int32>& 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<SfxItemPool> mxPool;
+    std::unordered_set<Cell*> maRegisteredCells;
     CellVec             maCells;
     std::vector<sal_Int32>   maWidths;
     std::vector<sal_Int32>   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 );