editeng/source/items/textitem.cxx | 4 +-
include/editeng/sizeitem.hxx | 9 +++-
include/svl/cintitem.hxx | 8 ++--
include/svl/custritm.hxx | 2 -
include/svl/eitem.hxx | 4 +-
include/svl/flagitem.hxx | 2 -
include/svl/intitem.hxx | 2 -
include/svl/poolitem.hxx | 7 +--
include/svl/ptitem.hxx | 2 -
sw/inc/fmtfsize.hxx | 21 +++++++---
sw/inc/fmtornt.hxx | 23 ++++++++---
sw/source/core/layout/atrfrm.cxx | 75 ++++++++++++++++++++++++++++++++++++++
12 files changed, 126 insertions(+), 33 deletions(-)
New commits:
commit 912c55537b11331e030d915e5857bccf182c9087
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Thu Jun 27 12:55:04 2024 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Fri Jun 28 09:18:04 2024 +0200
item-specific instance managers for SwFormatFrameSize and SwFormatVertOrient
and add some asserts to catch if we modify these things when they inside
a pool, which now that they are in hash-based container, will cause
trouble.
Change-Id: I2779f25cbcf056fbf71e417731d9b1f09ae33dd6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169629
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
Tested-by: Jenkins
diff --git a/include/editeng/sizeitem.hxx b/include/editeng/sizeitem.hxx
index 2247c76d9e79..62ba4f1fba51 100644
--- a/include/editeng/sizeitem.hxx
+++ b/include/editeng/sizeitem.hxx
@@ -56,12 +56,15 @@ public:
virtual bool HasMetrics() const override;
const Size& GetSize() const { return m_aSize; }
- void SetSize(const Size& rSize) { m_aSize = rSize; }
+ void SetSize(const Size& rSize)
+ { assert( !isPooled() && "SetValue() with pooled item" ); m_aSize = rSize;
}
tools::Long GetWidth() const { return m_aSize.getWidth(); }
tools::Long GetHeight() const { return m_aSize.getHeight(); }
- void SetWidth(tools::Long n) { m_aSize.setWidth(n); }
- void SetHeight(tools::Long n) { m_aSize.setHeight(n); }
+ void SetWidth(tools::Long n)
+ { assert( !isPooled() && "SetValue() with pooled item" );
m_aSize.setWidth(n); }
+ void SetHeight(tools::Long n)
+ { assert( !isPooled() && "SetValue() with pooled item" );
m_aSize.setHeight(n); }
};
#endif
diff --git a/sw/inc/fmtfsize.hxx b/sw/inc/fmtfsize.hxx
index 66aea1e78e50..7b70411f666d 100644
--- a/sw/inc/fmtfsize.hxx
+++ b/sw/inc/fmtfsize.hxx
@@ -87,10 +87,12 @@ public:
virtual bool PutValue( const css::uno::Any& rVal, sal_uInt8 nMemberId )
override;
SwFrameSize GetHeightSizeType() const { return m_eFrameHeightType; }
- void SetHeightSizeType( SwFrameSize eSize ) { m_eFrameHeightType = eSize; }
+ void SetHeightSizeType( SwFrameSize eSize )
+ { assert( !isPooled() && "SetValue() with pooled item" );
m_eFrameHeightType = eSize; }
SwFrameSize GetWidthSizeType() const { return m_eFrameWidthType; }
- void SetWidthSizeType( SwFrameSize eSize ) { m_eFrameWidthType = eSize; }
+ void SetWidthSizeType( SwFrameSize eSize )
+ { assert( !isPooled() && "SetValue() with pooled item" );
m_eFrameWidthType = eSize; }
enum PercentFlags { SYNCED = 0xff };
//0xff is reserved to indicate height is synced to width
@@ -99,12 +101,19 @@ public:
//0xff is reserved to indicate width is synced to height
sal_uInt8 GetWidthPercent() const { return m_nWidthPercent; }
sal_Int16 GetWidthPercentRelation() const { return
m_eWidthPercentRelation; }
- void SetHeightPercent( sal_uInt8 n ) { m_nHeightPercent = n; }
- void SetHeightPercentRelation ( sal_Int16 n ) {
m_eHeightPercentRelation = n; }
- void SetWidthPercent ( sal_uInt8 n ) { m_nWidthPercent = n; }
- void SetWidthPercentRelation ( sal_Int16 n ) { m_eWidthPercentRelation
= n; }
+ void SetHeightPercent( sal_uInt8 n )
+ { assert( !isPooled() && "SetValue() with pooled item" ); m_nHeightPercent
= n; }
+ void SetHeightPercentRelation ( sal_Int16 n )
+ { assert( !isPooled() && "SetValue() with pooled item" );
m_eHeightPercentRelation = n; }
+ void SetWidthPercent ( sal_uInt8 n )
+ { assert( !isPooled() && "SetValue() with pooled item" ); m_nWidthPercent
= n; }
+ void SetWidthPercentRelation ( sal_Int16 n )
+ { assert( !isPooled() && "SetValue() with pooled item" );
m_eWidthPercentRelation = n; }
void dumpAsXml(xmlTextWriterPtr pWriter) const override;
+
+protected:
+ virtual ItemInstanceManager* getItemInstanceManager() const override;
};
inline const SwFormatFrameSize &SwAttrSet::GetFrameSize(bool bInP) const
diff --git a/sw/inc/fmtornt.hxx b/sw/inc/fmtornt.hxx
index 0a993243cf7a..644576b6fc7f 100644
--- a/sw/inc/fmtornt.hxx
+++ b/sw/inc/fmtornt.hxx
@@ -56,13 +56,19 @@ public:
sal_Int16 GetVertOrient() const { return m_eOrient; }
sal_Int16 GetRelationOrient() const { return m_eRelation; }
- void SetVertOrient( sal_Int16 eNew ) { m_eOrient = eNew; }
- void SetRelationOrient( sal_Int16 eNew ) { m_eRelation = eNew; }
+ void SetVertOrient( sal_Int16 eNew )
+ { assert( !isPooled() && "SetValue() with pooled item" ); m_eOrient =
eNew; }
+ void SetRelationOrient( sal_Int16 eNew )
+ { assert( !isPooled() && "SetValue() with pooled item" ); m_eRelation =
eNew; }
SwTwips GetPos() const { return m_nYPos; }
- void SetPos( SwTwips nNew ) { m_nYPos = nNew; }
+ void SetPos( SwTwips nNew )
+ { assert( !isPooled() && "SetValue() with pooled item" ); m_nYPos = nNew; }
void dumpAsXml(xmlTextWriterPtr pWriter) const override;
+
+protected:
+ virtual ItemInstanceManager* getItemInstanceManager() const override;
};
/// Defines the horizontal position of a fly frame.
@@ -93,16 +99,19 @@ public:
sal_Int16 GetHoriOrient() const { return m_eOrient; }
sal_Int16 GetRelationOrient() const { return m_eRelation; }
- void SetHoriOrient( sal_Int16 eNew ) { m_eOrient = eNew; }
- void SetRelationOrient( sal_Int16 eNew ) { m_eRelation = eNew; }
+ void SetHoriOrient( sal_Int16 eNew ) { assert( !isPooled() && "SetValue()
with pooled item" ); m_eOrient = eNew; }
+ void SetRelationOrient( sal_Int16 eNew ) { assert( !isPooled() &&
"SetValue() with pooled item" ); m_eRelation = eNew; }
SwTwips GetPos() const { return m_nXPos; }
- void SetPos( SwTwips nNew ) { m_nXPos = nNew; }
+ void SetPos( SwTwips nNew ) { assert( !isPooled() && "SetValue() with
pooled item" ); m_nXPos = nNew; }
bool IsPosToggle() const { return m_bPosToggle; }
- void SetPosToggle( bool bNew ) { m_bPosToggle = bNew; }
+ void SetPosToggle( bool bNew ) { assert( !isPooled() && "SetValue() with
pooled item" ); m_bPosToggle = bNew; }
void dumpAsXml(xmlTextWriterPtr pWriter) const override;
+
+protected:
+ virtual ItemInstanceManager* getItemInstanceManager() const override;
};
inline const SwFormatVertOrient &SwAttrSet::GetVertOrient(bool bInP) const
diff --git a/sw/source/core/layout/atrfrm.cxx b/sw/source/core/layout/atrfrm.cxx
index c3534b19534a..8c44f2ac893b 100644
--- a/sw/source/core/layout/atrfrm.cxx
+++ b/sw/source/core/layout/atrfrm.cxx
@@ -216,6 +216,34 @@ static void lcl_DelHFFormat( SwClient *pToRemove,
SwFrameFormat *pFormat )
delete pFormat;
}
+namespace
+{
+ class SwFormatFrameSizeInstanceManager : public
TypeSpecificItemInstanceManager<SwFormatFrameSize>
+ {
+ protected:
+ virtual size_t hashCode(const SfxPoolItem& rItem) const override
+ {
+ auto const & rFormatItem = static_cast<const
SwFormatFrameSize&>(rItem);
+ std::size_t seed(0);
+ o3tl::hash_combine(seed, rFormatItem.GetHeightSizeType());
+ o3tl::hash_combine(seed, rFormatItem.GetWidthSizeType());
+ o3tl::hash_combine(seed, rFormatItem.GetWidthPercent());
+ o3tl::hash_combine(seed, rFormatItem.GetWidthPercentRelation());
+ o3tl::hash_combine(seed, rFormatItem.GetHeightPercent());
+ o3tl::hash_combine(seed, rFormatItem.GetHeightPercentRelation());
+ o3tl::hash_combine(seed, rFormatItem.GetSize().Width());
+ o3tl::hash_combine(seed, rFormatItem.GetSize().Height());
+ return seed;
+ }
+ };
+}
+
+ItemInstanceManager* SwFormatFrameSize::getItemInstanceManager() const
+{
+ static SwFormatFrameSizeInstanceManager aInstanceManager;
+ return &aInstanceManager;
+}
+
void SwFormatFrameSize::ScaleMetrics(tools::Long lMult, tools::Long lDiv) {
// Don't inherit the SvxSizeItem override (might or might not be relevant;
added "just in case"
// when changing SwFormatFrameSize to derive from SvxSizeItem instead of
directly from
@@ -1375,6 +1403,29 @@ void SwFormatSurround::dumpAsXml(xmlTextWriterPtr
pWriter) const
(void)xmlTextWriterEndElement(pWriter);
}
+namespace
+{
+ class SwFormatVertOrientInstanceManager : public
TypeSpecificItemInstanceManager<SwFormatVertOrient>
+ {
+ protected:
+ virtual size_t hashCode(const SfxPoolItem& rItem) const override
+ {
+ auto const & rFormatItem = static_cast<const
SwFormatVertOrient&>(rItem);
+ std::size_t seed(0);
+ o3tl::hash_combine(seed, rFormatItem.GetPos());
+ o3tl::hash_combine(seed, rFormatItem.GetVertOrient());
+ o3tl::hash_combine(seed, rFormatItem.GetRelationOrient());
+ return seed;
+ }
+ };
+}
+
+ItemInstanceManager* SwFormatVertOrient::getItemInstanceManager() const
+{
+ static SwFormatVertOrientInstanceManager aInstanceManager;
+ return &aInstanceManager;
+}
+
// Partially implemented inline in hxx
SwFormatVertOrient::SwFormatVertOrient( SwTwips nY, sal_Int16 eVert,
sal_Int16 eRel )
@@ -1467,6 +1518,30 @@ void SwFormatVertOrient::dumpAsXml(xmlTextWriterPtr
pWriter) const
(void)xmlTextWriterEndElement(pWriter);
}
+namespace
+{
+ class SwFormatHoriOrientInstanceManager : public
TypeSpecificItemInstanceManager<SwFormatHoriOrient>
+ {
+ protected:
+ virtual size_t hashCode(const SfxPoolItem& rItem) const override
+ {
+ auto const & rFormatItem = static_cast<const
SwFormatHoriOrient&>(rItem);
+ std::size_t seed(0);
+ o3tl::hash_combine(seed, rFormatItem.GetPos());
+ o3tl::hash_combine(seed, rFormatItem.GetHoriOrient());
+ o3tl::hash_combine(seed, rFormatItem.GetRelationOrient());
+ o3tl::hash_combine(seed, rFormatItem.IsPosToggle());
+ return seed;
+ }
+ };
+}
+
+ItemInstanceManager* SwFormatHoriOrient::getItemInstanceManager() const
+{
+ static SwFormatHoriOrientInstanceManager aInstanceManager;
+ return &aInstanceManager;
+}
+
// Partially implemented inline in hxx
SwFormatHoriOrient::SwFormatHoriOrient( SwTwips nX, sal_Int16 eHori,
sal_Int16 eRel, bool bPos )
commit cb3c65fb706cb1c7c9224222fd16875e924a9759
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Thu Jun 27 19:24:28 2024 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Fri Jun 28 09:17:52 2024 +0200
assert when SfxObjectItems are modified while in a pool
which has always been a problem, but becomes a more obvious problem when
I implement some upcoming optimisations
Change-Id: I8b0368b0b8e9a726c71d241841afeed3876281d9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169657
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
Tested-by: Jenkins
diff --git a/editeng/source/items/textitem.cxx
b/editeng/source/items/textitem.cxx
index 2bdfdd6890d9..34ca99d8fcd6 100644
--- a/editeng/source/items/textitem.cxx
+++ b/editeng/source/items/textitem.cxx
@@ -997,7 +997,7 @@ bool SvxFontHeightItem::HasMetrics() const
void SvxFontHeightItem::SetHeight( sal_uInt32 nNewHeight, const sal_uInt16
nNewProp,
MapUnit eUnit )
{
- DBG_ASSERT( GetRefCount() == 0, "SetValue() with pooled item" );
+ assert( !isPooled() && "SetHeight() with pooled item" );
ASSERT_CHANGE_REFCOUNTED_ITEM;
if( MapUnit::MapRelative != eUnit )
@@ -1015,7 +1015,7 @@ void SvxFontHeightItem::SetHeight( sal_uInt32 nNewHeight,
const sal_uInt16 nNewP
void SvxFontHeightItem::SetHeight( sal_uInt32 nNewHeight, sal_uInt16 nNewProp,
MapUnit eMetric, MapUnit eCoreMetric )
{
- DBG_ASSERT( GetRefCount() == 0, "SetValue() with pooled item" );
+ assert( !isPooled() && "SetValue() with pooled item" );
ASSERT_CHANGE_REFCOUNTED_ITEM;
if( MapUnit::MapRelative != eMetric )
diff --git a/include/svl/cintitem.hxx b/include/svl/cintitem.hxx
index 0b3da790d1ed..eef40f333240 100644
--- a/include/svl/cintitem.hxx
+++ b/include/svl/cintitem.hxx
@@ -56,7 +56,7 @@ public:
inline void CntByteItem::SetValue(sal_uInt8 nTheValue)
{
- DBG_ASSERT(GetRefCount() == 0, "CntByteItem::SetValue(): Pooled item");
+ assert( !isPooled() && "SetValue() with pooled item" );
m_nValue = nTheValue;
}
@@ -93,7 +93,7 @@ public:
inline void CntUInt16Item::SetValue(sal_uInt16 nTheValue)
{
- DBG_ASSERT(GetRefCount() == 0, "CntUInt16Item::SetValue(): Pooled item");
+ assert( !isPooled() && "SetValue() with pooled item" );
m_nValue = nTheValue;
}
@@ -130,7 +130,7 @@ public:
inline void CntInt32Item::SetValue(sal_Int32 nTheValue)
{
- DBG_ASSERT(GetRefCount() == 0, "CntInt32Item::SetValue(): Pooled item");
+ assert( !isPooled() && "SetValue() with pooled item" );
m_nValue = nTheValue;
}
@@ -167,7 +167,7 @@ public:
inline void CntUInt32Item::SetValue(sal_uInt32 nTheValue)
{
- DBG_ASSERT(GetRefCount() == 0, "CntUInt32Item::SetValue(): Pooled item");
+ assert( !isPooled() && "SetValue() with pooled item" );
m_nValue = nTheValue;
}
diff --git a/include/svl/custritm.hxx b/include/svl/custritm.hxx
index 0251df2ca182..3e2d23eb8ee4 100644
--- a/include/svl/custritm.hxx
+++ b/include/svl/custritm.hxx
@@ -60,7 +60,7 @@ public:
inline void CntUnencodedStringItem::SetValue(const OUString & rTheValue)
{
- assert(GetRefCount() == 0 && "cannot modify name of pooled item");
+ assert( !isPooled() && "SetValue() with pooled item" );
m_aValue = rTheValue;
}
diff --git a/include/svl/eitem.hxx b/include/svl/eitem.hxx
index 8e95bed00881..a75a329ba146 100644
--- a/include/svl/eitem.hxx
+++ b/include/svl/eitem.hxx
@@ -22,7 +22,7 @@
#include <svl/svldllapi.h>
#include <svl/cenumitm.hxx>
-
+#include <cassert>
template<typename EnumT>
class SAL_DLLPUBLIC_RTTI SfxEnumItem : public SfxEnumItemInterface
@@ -43,7 +43,7 @@ public:
void SetValue(EnumT nTheValue)
{
- assert(GetRefCount() == 0 && "SfxEnumItem::SetValue(): Pooled item");
+ assert( !isPooled() && "SetValue() with pooled item" );
m_nValue = nTheValue;
}
diff --git a/include/svl/flagitem.hxx b/include/svl/flagitem.hxx
index 76226cb1aa80..221416986593 100644
--- a/include/svl/flagitem.hxx
+++ b/include/svl/flagitem.hxx
@@ -46,7 +46,7 @@ public:
const IntlWrapper& ) const override;
sal_uInt16 GetValue() const { return nVal; }
void SetValue( sal_uInt16 nNewVal ) {
- DBG_ASSERT( GetRefCount() == 0, "SetValue()
with pooled item" );
+ assert( !isPooled() && "SetValue() with
pooled item" );
nVal = nNewVal;
}
bool GetFlag( sal_uInt8 nFlag ) const { return (nVal &
( 1<<nFlag)); }
diff --git a/include/svl/intitem.hxx b/include/svl/intitem.hxx
index f189388e748e..77cb540335a7 100644
--- a/include/svl/intitem.hxx
+++ b/include/svl/intitem.hxx
@@ -75,7 +75,7 @@ public:
inline void SfxInt16Item::SetValue(sal_Int16 nTheValue)
{
- DBG_ASSERT(GetRefCount() == 0, "SfxInt16Item::SetValue(); Pooled item");
+ assert( !isPooled() && "SetValue() with pooled item" );
m_nValue = nTheValue;
}
diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx
index 1cb667b5c90c..b1fc8280736c 100644
--- a/include/svl/poolitem.hxx
+++ b/include/svl/poolitem.hxx
@@ -503,6 +503,8 @@ public:
bool isDynamicDefault() const { return m_bDynamicDefault; }
bool isSetItem() const { return m_bIsSetItem; }
bool isShareable() const { return m_bShareable; }
+ bool isPooled() const { return GetRefCount() > 0; }
+
// version that allows nullptrs
static bool areSame(const SfxPoolItem* pItem1, const SfxPoolItem* pItem2);
@@ -707,11 +709,6 @@ inline bool IsDefaultItem( const SfxPoolItem *pItem )
return pItem && (pItem->isStaticDefault() || pItem->isDynamicDefault());
}
-inline bool IsPooledItem( const SfxPoolItem *pItem )
-{
- return pItem && pItem->GetRefCount() > 0;
-}
-
SVL_DLLPUBLIC extern SfxPoolItem const * const INVALID_POOL_ITEM;
SVL_DLLPUBLIC extern SfxPoolItem const * const DISABLED_POOL_ITEM;
diff --git a/include/svl/ptitem.hxx b/include/svl/ptitem.hxx
index 2ceb39410695..bb41ea916f45 100644
--- a/include/svl/ptitem.hxx
+++ b/include/svl/ptitem.hxx
@@ -47,7 +47,7 @@ public:
const Point& GetValue() const { return aVal; }
void SetValue( const Point& rNewVal ) {
- DBG_ASSERT( GetRefCount() == 0, "SetValue()
with pooled item" );
+ assert( !isPooled() && "SetValue() with
pooled item" );
aVal = rNewVal;
}
