include/svl/itemset.hxx | 24 +++++++++++++++++++-----
svl/qa/unit/items/stylepool.cxx | 16 ++++++++++++++++
svl/source/items/itemset.cxx | 33 +++++++++++++++++++++++++++++++++
3 files changed, 68 insertions(+), 5 deletions(-)
New commits:
commit 4bcdbddf9a29d27c5246825826765377ef832800
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Wed Dec 6 09:56:55 2023 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Mon Dec 18 20:10:29 2023 +0100
add copy constructor for SfxItemSetFixed
which flushes out the fact that, previously, when
SfxItemSetFixed was being copied, we were not
actually taking advantage of the "internal" memory,
and were actually allocating a separate block of memory,
like a "normal" SfxItemSet.
Change-Id: I6a8073c58b464d53bfd2a54cf1dd27a3f2cb3df7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160377
Tested-by: Noel Grandin <noel.gran...@collabora.co.uk>
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/include/svl/itemset.hxx b/include/svl/itemset.hxx
index 599afc7ab5bd..ffd7e7ac8caa 100644
--- a/include/svl/itemset.hxx
+++ b/include/svl/itemset.hxx
@@ -120,6 +120,8 @@ protected:
SfxItemSet( SfxItemPool&, SfxAllItemSetFlag );
/** special constructor for SfxItemSetFixed */
SfxItemSet( SfxItemPool&, WhichRangesContainer&& ranges, SfxPoolItem const
** ppItems, sal_uInt16 nTotalCount );
+ /** special constructor for SfxItemSetFixed copy constructor */
+ SfxItemSet( const SfxItemSet& rOther, SfxPoolItem const ** ppMyItems );
public:
SfxItemSet( const SfxItemSet& );
@@ -145,6 +147,8 @@ public:
sal_uInt16 Count() const { return m_nCount; }
sal_uInt16 TotalCount() const { return m_nTotalCount; }
+ bool IsItemsFixed() const { return m_bItemsFixed; }
+
const SfxPoolItem& Get( sal_uInt16 nWhich, bool bSrchInParent =
true ) const;
template<class T>
const T& Get( TypedWhichId<T> nWhich, bool
bSrchInParent = true ) const
@@ -333,17 +337,27 @@ static constexpr sal_uInt16 CountRanges1()
return nCapacity;
}}
+// Split out the array because we need it to be initialised before we call
+// the SfxItemSet constructor
+template<sal_uInt16... WIDs>
+struct SfxItemSetFixedStorage
+{
+ static constexpr sal_uInt16 NITEMS = svl::detail::CountRanges1<WIDs...>();
+ const SfxPoolItem* m_aItems[NITEMS] {};
+};
+
// Allocate the items array inside the object, to reduce allocation cost.
//
template<sal_uInt16... WIDs>
-class SfxItemSetFixed : public SfxItemSet
+class SfxItemSetFixed : public SfxItemSetFixedStorage<WIDs...>, public
SfxItemSet
{
public:
SfxItemSetFixed( SfxItemPool& rPool)
- : SfxItemSet(rPool, WhichRangesContainer(svl::Items_t<WIDs...>{}),
m_aItems, NITEMS) {}
-private:
- static constexpr sal_uInt16 NITEMS = svl::detail::CountRanges1<WIDs...>();
- const SfxPoolItem* m_aItems[NITEMS] = {};
+ : SfxItemSet(rPool, WhichRangesContainer(svl::Items_t<WIDs...>{}),
+ SfxItemSetFixedStorage<WIDs...>::m_aItems,
+ SfxItemSetFixedStorage<WIDs...>::NITEMS) {}
+ SfxItemSetFixed( const SfxItemSetFixed<WIDs...>& rOther )
+ : SfxItemSet(rOther, SfxItemSetFixedStorage<WIDs...>::m_aItems) {}
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/svl/qa/unit/items/stylepool.cxx b/svl/qa/unit/items/stylepool.cxx
index d852dd29a24c..454db1c2faf4 100644
--- a/svl/qa/unit/items/stylepool.cxx
+++ b/svl/qa/unit/items/stylepool.cxx
@@ -79,6 +79,22 @@ CPPUNIT_TEST_FIXTURE(StylePoolTest, testIterationOrder)
CPPUNIT_ASSERT(!pIter->getNext());
}
}
+
+CPPUNIT_TEST_FIXTURE(StylePoolTest, testFixedItemSet)
+{
+ SfxStringItem aDefault1(1);
+ std::vector<SfxPoolItem*> aDefaults{ &aDefault1 };
+ SfxItemInfo const aItems[] = { // _nSID, _bNeedsPoolRegistration,
_bShareable
+ { 2, false, false }
+ };
+ rtl::Reference<SfxItemPool> pPool = new SfxItemPool("test", 1, 1, aItems);
+ pPool->SetDefaults(&aDefaults);
+
+ SfxItemSetFixed<1, 2> aItemSet1(*pPool);
+
+ SfxItemSetFixed<1, 2> aItemSet2(aItemSet1); // test copy constructor
+ assert(aItemSet2.IsItemsFixed());
+}
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx
index 102cf4d2bef8..0648a63da8f2 100644
--- a/svl/source/items/itemset.cxx
+++ b/svl/source/items/itemset.cxx
@@ -169,6 +169,39 @@ SfxItemSet::SfxItemSet( SfxItemPool& rPool,
WhichRangesContainer&& ranges, SfxPo
assert(svl::detail::validRanges2(m_pWhichRanges));
}
+/** special constructor for SfxItemSetFixed copy constructor */
+SfxItemSet::SfxItemSet( const SfxItemSet& rOther,
+ SfxPoolItem const ** ppMyItems )
+ : m_pPool(rOther.m_pPool)
+ , m_pParent(rOther.m_pParent)
+ , m_nCount(rOther.m_nCount)
+ , m_nTotalCount(rOther.m_nTotalCount)
+ , m_bItemsFixed(true)
+ , m_ppItems(ppMyItems)
+ , m_pWhichRanges(rOther.m_pWhichRanges)
+ , m_aCallback(rOther.m_aCallback)
+{
+#ifdef DBG_UTIL
+ nAllocatedSfxItemSetCount++;
+ nUsedSfxItemSetCount++;
+#endif
+ assert(ppMyItems);
+ assert(m_pWhichRanges.size() > 0);
+ assert(svl::detail::validRanges2(m_pWhichRanges));
+
+ if (0 == rOther.Count())
+ return;
+
+ // Copy attributes
+ SfxPoolItem const** ppDst(m_ppItems);
+
+ for (const auto& rSource : rOther)
+ {
+ *ppDst = implCreateItemEntry(*GetPool(), rSource, 0, false);
+ ppDst++;
+ }
+}
+
SfxItemSet::SfxItemSet(SfxItemPool& pool, WhichRangesContainer wids)
: m_pPool(&pool)
, m_pParent(nullptr)
