editeng/source/uno/unoipset.cxx | 81 +++++++++++++++++-------------------- helpcontent2 | 2 include/editeng/unoipset.hxx | 31 ++++++++++---- include/svx/unoshape.hxx | 1 oox/source/drawingml/shape.cxx | 8 --- sd/qa/unit/export-tests-ooxml1.cxx | 6 +- sd/source/ui/unoidl/unopback.cxx | 10 ++-- sd/source/ui/unoidl/unopback.hxx | 2 svx/source/unodraw/unoshape.cxx | 18 ++++---- 9 files changed, 82 insertions(+), 77 deletions(-)
New commits: commit c6f25506b02fbd2a087b7e790283921bf8550206 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Wed Aug 25 08:42:39 2021 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Wed Aug 25 18:11:43 2021 +0200 i#114206 sd: unshare shape properties for the same type before insertion Regression from commit 9bd99c08e8662becdd5ac8b47ef6f953c14e65fc (CWS-TOOLING: integrate CWS os128, 2009-06-03), the problem was that the SvxItemPropertySet was both used to store a property map (information about UNO properties) and also as a descriptor for a not yet inserted shape. The above commit improved performance by sharing a single instance of an SvxItemPropertySet for the same shape types: this works for the property map, but doing the same for property values is problematic. In practice, this eliminates the need for a workaround in oox/, the user-visible problem was that loading a document with smartart, then loading a document with group shapes (but without smartart) and saving it would copy information from the first, closed document (!) to the second document. Just removing the oox/ workaround would make make -C oox -sr CppunitTest_oox_drawingml CPPUNIT_TEST_NAME="testGroupShapeSmartArt testTdf131082" fail, unsharing the descriptor piece makes it pass again. Change-Id: Icdff2108ad0da18ce0ade081b1938dd74bc0ae90 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120996 Reviewed-by: Miklos Vajna <vmik...@collabora.com> Tested-by: Jenkins diff --git a/editeng/source/uno/unoipset.cxx b/editeng/source/uno/unoipset.cxx index 3f524805f4ed..da068755ce8d 100644 --- a/editeng/source/uno/unoipset.cxx +++ b/editeng/source/uno/unoipset.cxx @@ -31,14 +31,6 @@ using namespace ::com::sun::star; -struct SvxIDPropertyCombine -{ - sal_uInt16 nWID; - sal_uInt8 memberId; - uno::Any aAny; -}; - - SvxItemPropertySet::SvxItemPropertySet( const SfxItemPropertyMapEntry* pMap, SfxItemPool& rItemPool ) : m_aPropertyMap( pMap ), mrItemPool( rItemPool ) @@ -48,35 +40,6 @@ SvxItemPropertySet::SvxItemPropertySet( const SfxItemPropertyMapEntry* pMap, Sfx SvxItemPropertySet::~SvxItemPropertySet() { - ClearAllUsrAny(); -} - - -uno::Any* SvxItemPropertySet::GetUsrAnyForID(SfxItemPropertyMapEntry const & entry) const -{ - for (auto const & rActual : aCombineList) - { - if( rActual.nWID == entry.nWID && rActual.memberId == entry.nMemberId ) - return const_cast<uno::Any*>(&rActual.aAny); - } - return nullptr; -} - - -void SvxItemPropertySet::AddUsrAnyForID( - const uno::Any& rAny, SfxItemPropertyMapEntry const & entry) -{ - SvxIDPropertyCombine aNew; - aNew.nWID = entry.nWID; - aNew.memberId = entry.nMemberId; - aNew.aAny = rAny; - aCombineList.push_back( std::move(aNew) ); -} - - -void SvxItemPropertySet::ClearAllUsrAny() -{ - aCombineList.clear(); } @@ -184,10 +147,10 @@ void SvxItemPropertySet::setPropertyValue( const SfxItemPropertyMapEntry* pMap, } -uno::Any SvxItemPropertySet::getPropertyValue( const SfxItemPropertyMapEntry* pMap ) const +uno::Any SvxItemPropertySet::getPropertyValue( const SfxItemPropertyMapEntry* pMap, SvxItemPropertySetUsrAnys& rAnys ) const { // Already entered a value? Then finish quickly - uno::Any* pUsrAny = GetUsrAnyForID(*pMap); + uno::Any* pUsrAny = rAnys.GetUsrAnyForID(*pMap); if(pUsrAny) return *pUsrAny; @@ -213,7 +176,7 @@ uno::Any SvxItemPropertySet::getPropertyValue( const SfxItemPropertyMapEntry* pM if(eState >= SfxItemState::DEFAULT && pItem) { pItem->QueryValue( aVal, nMemberId ); - const_cast<SvxItemPropertySet*>(this)->AddUsrAnyForID(aVal, *pMap); + rAnys.AddUsrAnyForID(aVal, *pMap); } } @@ -236,11 +199,11 @@ uno::Any SvxItemPropertySet::getPropertyValue( const SfxItemPropertyMapEntry* pM } -void SvxItemPropertySet::setPropertyValue( const SfxItemPropertyMapEntry* pMap, const uno::Any& rVal ) const +void SvxItemPropertySet::setPropertyValue( const SfxItemPropertyMapEntry* pMap, const uno::Any& rVal, SvxItemPropertySetUsrAnys& rAnys ) { - uno::Any* pUsrAny = GetUsrAnyForID(*pMap); + uno::Any* pUsrAny = rAnys.GetUsrAnyForID(*pMap); if(!pUsrAny) - const_cast<SvxItemPropertySet*>(this)->AddUsrAnyForID(rVal, *pMap); + rAnys.AddUsrAnyForID(rVal, *pMap); else *pUsrAny = rVal; } @@ -335,4 +298,36 @@ void SvxUnoConvertFromMM( const MapUnit eDestinationMapUnit, uno::Any & rMetric } } +SvxItemPropertySetUsrAnys::SvxItemPropertySetUsrAnys() = default; + +SvxItemPropertySetUsrAnys::~SvxItemPropertySetUsrAnys() +{ + ClearAllUsrAny(); +} + +uno::Any* SvxItemPropertySetUsrAnys::GetUsrAnyForID(SfxItemPropertyMapEntry const & entry) const +{ + for (auto const & rActual : aCombineList) + { + if( rActual.nWID == entry.nWID && rActual.memberId == entry.nMemberId ) + return const_cast<uno::Any*>(&rActual.aAny); + } + return nullptr; +} + +void SvxItemPropertySetUsrAnys::AddUsrAnyForID( + const uno::Any& rAny, SfxItemPropertyMapEntry const & entry) +{ + SvxIDPropertyCombine aNew; + aNew.nWID = entry.nWID; + aNew.memberId = entry.nMemberId; + aNew.aAny = rAny; + aCombineList.push_back( std::move(aNew) ); +} + +void SvxItemPropertySetUsrAnys::ClearAllUsrAny() +{ + aCombineList.clear(); +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/include/editeng/unoipset.hxx b/include/editeng/unoipset.hxx index 2c079a06b37f..b4ea97f6cb28 100644 --- a/include/editeng/unoipset.hxx +++ b/include/editeng/unoipset.hxx @@ -28,13 +28,12 @@ namespace com::sun::star::beans { class XPropertySetInfo; } class SfxItemSet; -struct SvxIDPropertyCombine; +class SvxItemPropertySetUsrAnys; class EDITENG_DLLPUBLIC SvxItemPropertySet { SfxItemPropertyMap m_aPropertyMap; mutable css::uno::Reference<css::beans::XPropertySetInfo> m_xInfo; - ::std::vector< SvxIDPropertyCombine > aCombineList; SfxItemPool& mrItemPool; public: @@ -49,17 +48,33 @@ public: static void setPropertyValue( const SfxItemPropertyMapEntry* pMap, const css::uno::Any& rVal, SfxItemSet& rSet, bool bDontConvertNegativeValues ); // Methods that use Any instead - css::uno::Any getPropertyValue( const SfxItemPropertyMapEntry* pMap ) const; - void setPropertyValue( const SfxItemPropertyMapEntry* pMap, const css::uno::Any& rVal ) const; + css::uno::Any getPropertyValue( const SfxItemPropertyMapEntry* pMap, SvxItemPropertySetUsrAnys& rAnys ) const; + static void setPropertyValue( const SfxItemPropertyMapEntry* pMap, const css::uno::Any& rVal, SvxItemPropertySetUsrAnys& rAnys ); + + css::uno::Reference< css::beans::XPropertySetInfo > const & getPropertySetInfo() const; + const SfxItemPropertyMap& getPropertyMap() const { return m_aPropertyMap;} + const SfxItemPropertyMapEntry* getPropertyMapEntry(std::u16string_view rName) const; +}; + +struct SvxIDPropertyCombine +{ + sal_uInt16 nWID; + sal_uInt8 memberId; + css::uno::Any aAny; +}; + +class EDITENG_DLLPUBLIC SvxItemPropertySetUsrAnys +{ + ::std::vector< SvxIDPropertyCombine > aCombineList; + +public: + SvxItemPropertySetUsrAnys(); + ~SvxItemPropertySetUsrAnys(); bool AreThereOwnUsrAnys() const { return ! aCombineList.empty(); } css::uno::Any* GetUsrAnyForID(SfxItemPropertyMapEntry const & entry) const; void AddUsrAnyForID(const css::uno::Any& rAny, SfxItemPropertyMapEntry const & entry); void ClearAllUsrAny(); - - css::uno::Reference< css::beans::XPropertySetInfo > const & getPropertySetInfo() const; - const SfxItemPropertyMap& getPropertyMap() const { return m_aPropertyMap;} - const SfxItemPropertyMapEntry* getPropertyMapEntry(std::u16string_view rName) const; }; /** converts the given any with a metric to 100th/mm if needed */ diff --git a/include/svx/unoshape.hxx b/include/svx/unoshape.hxx index 2dc2f5c59843..3d3d5bea3a9b 100644 --- a/include/svx/unoshape.hxx +++ b/include/svx/unoshape.hxx @@ -125,6 +125,7 @@ protected: friend class SdXShape; const SvxItemPropertySet* mpPropSet; + SvxItemPropertySetUsrAnys maUrsAnys; const SfxItemPropertyMapEntry* maPropMapEntries; private: diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx index b4079c7b6831..fe38fa6b371d 100644 --- a/oox/source/drawingml/shape.cxx +++ b/oox/source/drawingml/shape.cxx @@ -980,14 +980,6 @@ Reference< XShape > const & Shape::createAndInsert( if ( !mxShape.is() ) { mxShape.set( xServiceFact->createInstance( aServiceName ), UNO_QUERY_THROW ); - if (aServiceName == "com.sun.star.drawing.GroupShape") - { - // TODO why is this necessary? A newly created group shape should have an empty - // grab-bag. - uno::Reference<beans::XPropertySet> xPropertySet(mxShape, uno::UNO_QUERY); - beans::PropertyValues aVals; - xPropertySet->setPropertyValue("InteropGrabBag", uno::makeAny(aVals)); - } } Reference< XPropertySet > xSet( mxShape, UNO_QUERY ); diff --git a/sd/qa/unit/export-tests-ooxml1.cxx b/sd/qa/unit/export-tests-ooxml1.cxx index b191cc62ee76..e2a3f04d2467 100644 --- a/sd/qa/unit/export-tests-ooxml1.cxx +++ b/sd/qa/unit/export-tests-ooxml1.cxx @@ -371,7 +371,7 @@ void SdOOXMLExportTest1::testBnc870233_2() { const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(0)); CPPUNIT_ASSERT(pObjGroup); - const SdrTextObj *pObj = dynamic_cast<SdrTextObj *>(pObjGroup->GetSubList()->GetObj(0)); + const SdrTextObj *pObj = dynamic_cast<SdrTextObj *>(pObjGroup->GetSubList()->GetObj(1)); checkFontAttributes<Color, SvxColorItem>(pObj, Color(0x0000ff), EE_CHAR_COLOR); } @@ -379,7 +379,7 @@ void SdOOXMLExportTest1::testBnc870233_2() { const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(1)); CPPUNIT_ASSERT(pObjGroup); - const SdrTextObj *pObj = dynamic_cast<SdrTextObj *>(pObjGroup->GetSubList()->GetObj(0)); + const SdrTextObj *pObj = dynamic_cast<SdrTextObj *>(pObjGroup->GetSubList()->GetObj(1)); checkFontAttributes<Color, SvxColorItem>( pObj, Color(0x1F497D), EE_CHAR_COLOR ); } @@ -387,7 +387,7 @@ void SdOOXMLExportTest1::testBnc870233_2() { const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(2)); CPPUNIT_ASSERT(pObjGroup); - const SdrTextObj *pObj = dynamic_cast<SdrTextObj *>(pObjGroup->GetSubList()->GetObj(0)); + const SdrTextObj *pObj = dynamic_cast<SdrTextObj *>(pObjGroup->GetSubList()->GetObj(1)); checkFontAttributes<Color, SvxColorItem>(pObj, Color(0xffffff), EE_CHAR_COLOR); } diff --git a/sd/source/ui/unoidl/unopback.cxx b/sd/source/ui/unoidl/unopback.cxx index 646f6db0cb44..5f3911edaf0b 100644 --- a/sd/source/ui/unoidl/unopback.cxx +++ b/sd/source/ui/unoidl/unopback.cxx @@ -98,11 +98,11 @@ void SdUnoPageBackground::fillItemSet( SdDrawDocument* pDoc, SfxItemSet& rSet ) mpSet = std::make_unique<SfxItemSet>( *rSet.GetPool(), svl::Items<XATTR_FILL_FIRST, XATTR_FILL_LAST> ); - if( mpPropSet->AreThereOwnUsrAnys() ) + if( maUsrAnys.AreThereOwnUsrAnys() ) { for( const auto pProp : mpPropSet->getPropertyMap().getPropertyEntries() ) { - uno::Any* pAny = mpPropSet->GetUsrAnyForID( *pProp ); + uno::Any* pAny = maUsrAnys.GetUsrAnyForID( *pProp ); if( pAny ) { const OUString & aPropertyName = pProp->aName; @@ -235,7 +235,7 @@ void SAL_CALL SdUnoPageBackground::setPropertyValue( const OUString& aPropertyNa else { if(pEntry->nWID) - mpPropSet->setPropertyValue( pEntry, aValue ); + SvxItemPropertySet::setPropertyValue( pEntry, aValue, maUsrAnys ); } } @@ -284,7 +284,7 @@ uno::Any SAL_CALL SdUnoPageBackground::getPropertyValue( const OUString& Propert else { if(pEntry->nWID) - aAny = mpPropSet->getPropertyValue( pEntry ); + aAny = mpPropSet->getPropertyValue( pEntry, maUsrAnys ); } return aAny; } @@ -333,7 +333,7 @@ beans::PropertyState SAL_CALL SdUnoPageBackground::getPropertyState( const OUStr } else { - if( nullptr == mpPropSet->GetUsrAnyForID(*pEntry) ) + if( nullptr == maUsrAnys.GetUsrAnyForID(*pEntry) ) return beans::PropertyState_DEFAULT_VALUE; else return beans::PropertyState_DIRECT_VALUE; diff --git a/sd/source/ui/unoidl/unopback.hxx b/sd/source/ui/unoidl/unopback.hxx index cf7bc235773c..c70cc2fea4eb 100644 --- a/sd/source/ui/unoidl/unopback.hxx +++ b/sd/source/ui/unoidl/unopback.hxx @@ -31,6 +31,7 @@ #include <comphelper/servicehelper.hxx> #include <cppuhelper/implbase.hxx> +#include <editeng/unoipset.hxx> class SdDrawDocument; class SdrModel; @@ -48,6 +49,7 @@ class SdUnoPageBackground final : public ::cppu::WeakImplHelper< public SfxListener { const SvxItemPropertySet* mpPropSet; + SvxItemPropertySetUsrAnys maUsrAnys; std::unique_ptr<SfxItemSet> mpSet; SdrModel* mpDoc; diff --git a/svx/source/unodraw/unoshape.cxx b/svx/source/unodraw/unoshape.cxx index a6ba615358ed..83e2eeb5304c 100644 --- a/svx/source/unodraw/unoshape.cxx +++ b/svx/source/unodraw/unoshape.cxx @@ -580,10 +580,10 @@ void SvxShape::ForceMetricTo100th_mm(basegfx::B2DHomMatrix& rB2DHomMatrix) const } } -static void SvxItemPropertySet_ObtainSettingsFromPropertySet(const SvxItemPropertySet& rPropSet, +static void SvxItemPropertySet_ObtainSettingsFromPropertySet(const SvxItemPropertySet& rPropSet, SvxItemPropertySetUsrAnys& rAnys, SfxItemSet& rSet, const uno::Reference< beans::XPropertySet >& xSet, const SfxItemPropertyMap* pMap ) { - if(!rPropSet.AreThereOwnUsrAnys()) + if(!rAnys.AreThereOwnUsrAnys()) return; const SfxItemPropertyMap& rSrc = rPropSet.getPropertyMap(); @@ -593,7 +593,7 @@ static void SvxItemPropertySet_ObtainSettingsFromPropertySet(const SvxItemProper const sal_uInt16 nWID = pSrcProp->nWID; if(SfxItemPool::IsWhich(nWID) && (nWID < OWN_ATTR_VALUE_START || nWID > OWN_ATTR_VALUE_END) - && rPropSet.GetUsrAnyForID(*pSrcProp)) + && rAnys.GetUsrAnyForID(*pSrcProp)) rSet.Put(rSet.GetPool()->GetDefaultItem(nWID)); } @@ -601,7 +601,7 @@ static void SvxItemPropertySet_ObtainSettingsFromPropertySet(const SvxItemProper { if(pSrcProp->nWID) { - uno::Any* pUsrAny = rPropSet.GetUsrAnyForID(*pSrcProp); + uno::Any* pUsrAny = rAnys.GetUsrAnyForID(*pSrcProp); if(pUsrAny) { // search for equivalent entry in pDst @@ -623,18 +623,18 @@ static void SvxItemPropertySet_ObtainSettingsFromPropertySet(const SvxItemProper } } } - const_cast< SvxItemPropertySet& >(rPropSet).ClearAllUsrAny(); + rAnys.ClearAllUsrAny(); } void SvxShape::ObtainSettingsFromPropertySet(const SvxItemPropertySet& rPropSet) { DBG_TESTSOLARMUTEX(); - if(HasSdrObject() && rPropSet.AreThereOwnUsrAnys()) + if(HasSdrObject() && maUrsAnys.AreThereOwnUsrAnys()) { SfxItemSet aSet( GetSdrObject()->getSdrModelFromSdrObject().GetItemPool(), svl::Items<SDRATTR_START, SDRATTR_END>); Reference< beans::XPropertySet > xShape(this); - SvxItemPropertySet_ObtainSettingsFromPropertySet(rPropSet, aSet, xShape, &mpPropSet->getPropertyMap() ); + SvxItemPropertySet_ObtainSettingsFromPropertySet(rPropSet, maUrsAnys, aSet, xShape, &mpPropSet->getPropertyMap() ); GetSdrObject()->SetMergedItemSetAndBroadcast(aSet); @@ -1591,7 +1591,7 @@ void SvxShape::_setPropertyValue( const OUString& rPropertyName, const uno::Any& // support additional properties that we don't know here we // silently store *all* properties, even if they may be not // supported after creation. - mpPropSet->setPropertyValue( pMap, rVal ); + SvxItemPropertySet::setPropertyValue( pMap, rVal, maUrsAnys ); } return; @@ -1762,7 +1762,7 @@ uno::Any SvxShape::_getPropertyValue( const OUString& PropertyName ) if(pMap && pMap->nWID) // FixMe: see setPropertyValue - aAny = mpPropSet->getPropertyValue( pMap ); + aAny = mpPropSet->getPropertyValue( pMap, maUrsAnys ); } return aAny; commit 8befed1f019061cd0b0ef06f6b717f5bc77afdee Author: Olivier Hallot <olivier.hal...@libreoffice.org> AuthorDate: Wed Aug 25 13:11:33 2021 -0300 Commit: Gerrit Code Review <ger...@gerrit.libreoffice.org> CommitDate: Wed Aug 25 18:11:33 2021 +0200 Update git submodules * Update helpcontent2 from branch 'master' to a8089daeffa32ec9177187617833f592f2ffb64e - tdf#137084 (part) Update Writer Form menu Help page Change-Id: I875d33c7470d7e8ae4ac09b4266411ff3f94920e Reviewed-on: https://gerrit.libreoffice.org/c/help/+/121040 Tested-by: Jenkins Reviewed-by: Olivier Hallot <olivier.hal...@libreoffice.org> diff --git a/helpcontent2 b/helpcontent2 index 7d86aa20fd83..a8089daeffa3 160000 --- a/helpcontent2 +++ b/helpcontent2 @@ -1 +1 @@ -Subproject commit 7d86aa20fd836dd1b94dbe50d2ffb4b203ea8c1e +Subproject commit a8089daeffa32ec9177187617833f592f2ffb64e