include/xmloff/xmlstyle.hxx | 7 + sd/qa/unit/SVGExportTests.cxx | 28 +++--- sd/qa/unit/export-tests-ooxml1.cxx | 2 xmloff/source/draw/ximpstyl.cxx | 46 ++++------- xmloff/source/draw/ximpstyl.hxx | 2 xmloff/source/style/xmlstyle.cxx | 154 +++++++++++++++++++++---------------- 6 files changed, 130 insertions(+), 109 deletions(-)
New commits: commit 6c2f827697b898c9d351b79c2dbb5aefc8b70dcc Author: Noel Grandin <[email protected]> AuthorDate: Sun Oct 20 15:07:41 2024 +0200 Commit: Caolán McNamara <[email protected]> CommitDate: Mon Oct 21 17:23:58 2024 +0200 improve style searching in SvXMLStylesContext We can searching without needing a std::map by sorting the style list. Which also allows to do prefix searching. Which we can use to dramatically reduce the number of styles we need to loop through in SdXMLStylesContext::ImpSetGraphicStyles. a similar mega-master-page scenario as reported in tdf#158773 13.5 - 9.1s Needed to adjust some unit tests because the order of iteration through styles is now different, which affects some file output. Change-Id: Ia7240fe520b70839d2519eba1fb70819a3c3bf81 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/175281 Reviewed-by: Caolán McNamara <[email protected]> Tested-by: Jenkins diff --git a/include/xmloff/xmlstyle.hxx b/include/xmloff/xmlstyle.hxx index 995553f58c3f..a507993e20ae 100644 --- a/include/xmloff/xmlstyle.hxx +++ b/include/xmloff/xmlstyle.hxx @@ -26,6 +26,7 @@ #include <sal/types.h> #include <xmloff/xmlictxt.hxx> #include <xmloff/families.hxx> +#include <vector> #include <memory> class SvXMLStylesContext_Impl; @@ -164,6 +165,8 @@ protected: public: + typedef std::vector<SvXMLStyleContext*> StyleIndex; + SvXMLStylesContext( SvXMLImport& rImport, bool bAutomatic = false ); @@ -180,6 +183,10 @@ public: XmlStyleFamily nFamily, const OUString& rName, bool bCreateIndex = false ) const; + std::pair<StyleIndex::const_iterator, StyleIndex::const_iterator> + FindStyleChildContextByPrefix( + XmlStyleFamily nFamily, + const OUString& rNamePrefix) const; static XmlStyleFamily GetFamily( std::u16string_view rFamily ); virtual rtl::Reference < SvXMLImportPropertyMapper > GetImportPropertyMapper( XmlStyleFamily nFamily ) const; diff --git a/sd/qa/unit/SVGExportTests.cxx b/sd/qa/unit/SVGExportTests.cxx index 68d73bee89db..bd800a516db9 100644 --- a/sd/qa/unit/SVGExportTests.cxx +++ b/sd/qa/unit/SVGExportTests.cxx @@ -186,10 +186,10 @@ public: xmlDocUniquePtr svgDoc = parseXml(maTempFile); CPPUNIT_ASSERT(svgDoc); - assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[9] ), "class", u"BackgroundBitmaps"); - assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[9]/SVG_IMAGE ), 1); + assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10] ), "class", u"BackgroundBitmaps"); + assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10]/SVG_IMAGE ), 1); - OUString sImageId = getXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[9]/SVG_IMAGE ), "id"); + OUString sImageId = getXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10]/SVG_IMAGE ), "id"); CPPUNIT_ASSERT_MESSAGE(OString("The exported bitmap has not a valid id: " + sImageId.toUtf8()).getStr(), isValidBitmapId(sImageId)); BitmapChecksum nChecksum = getBitmapChecksumFromId(sImageId); @@ -216,39 +216,39 @@ public: CPPUNIT_ASSERT(svgDoc); // check the bitmap - assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[9] ), "class", u"BackgroundBitmaps"); - assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[9]/SVG_IMAGE ), 1); + assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10] ), "class", u"BackgroundBitmaps"); + assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10]/SVG_IMAGE ), 1); // check the pattern and background rectangle - assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10] ), "class", u"BackgroundPatterns"); - assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10]/SVG_PATTERN ), 1); - assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10]/SVG_PATTERN/SVG_USE ), 1); - assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10]/SVG_G/SVG_RECT ), 1); + assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[11] ), "class", u"BackgroundPatterns"); + assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[11]/SVG_PATTERN ), 1); + assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[11]/SVG_PATTERN/SVG_USE ), 1); + assertXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[11]/SVG_G/SVG_RECT ), 1); // check that <pattern><use> is pointing to the correct <image> - OUString sImageId = getXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[9]/SVG_IMAGE ), "id"); + OUString sImageId = getXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10]/SVG_IMAGE ), "id"); CPPUNIT_ASSERT_MESSAGE(OString("The exported bitmap has not a valid id: " + sImageId.toUtf8()).getStr(), isValidBitmapId(sImageId)); BitmapChecksum nChecksum = getBitmapChecksumFromId(sImageId); CPPUNIT_ASSERT_MESSAGE(OString("The exported bitmap has not a valid checksum: " + sImageId.toUtf8()).getStr(), nChecksum != 0); - OUString sRef = getXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10]/SVG_PATTERN/SVG_USE ), "href"); + OUString sRef = getXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[11]/SVG_PATTERN/SVG_USE ), "href"); CPPUNIT_ASSERT_MESSAGE("The <pattern><use> element has not a valid href attribute: starting '#' not present.", sRef.startsWith("#")); sRef = sRef.copy(1); CPPUNIT_ASSERT_EQUAL_MESSAGE("The href attribute for <pattern><use> does not match the <image> id attribute: ", sImageId, sRef); - OUString sPatternId = getXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10]/SVG_PATTERN ), "id"); + OUString sPatternId = getXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[11]/SVG_PATTERN ), "id"); CPPUNIT_ASSERT_MESSAGE(OString("The exported pattern has not a valid id: " + sPatternId.toUtf8()).getStr(), isValidBackgroundPatternId(sPatternId)); - OUString sFillUrl = getXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10]/SVG_G/SVG_RECT ), "fill"); + OUString sFillUrl = getXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[11]/SVG_G/SVG_RECT ), "fill"); bool bIsUrlFormat = sFillUrl.startsWith("url(#") && sFillUrl.endsWith(")"); CPPUNIT_ASSERT_MESSAGE("The fill attribute for the <rectangle> element has not a url format .", bIsUrlFormat); // remove "url(#" and ")" sFillUrl = sFillUrl.copy(5, sFillUrl.getLength() - 6); CPPUNIT_ASSERT_EQUAL_MESSAGE("The fill url for <rectangle> does not match the <pattern> id attribute: ", sPatternId, sFillUrl); - OUString sBackgroundId = getXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[10]/SVG_G ), "id"); + OUString sBackgroundId = getXPath(svgDoc, SAL_STRINGIFY( /SVG_SVG/SVG_DEFS[11]/SVG_G ), "id"); CPPUNIT_ASSERT_MESSAGE(OString("The exported tiled background has not a valid id: " + sBackgroundId.toUtf8()).getStr(), isValidTiledBackgroundId(sBackgroundId)); // check <use> element that point to the tiled background diff --git a/sd/qa/unit/export-tests-ooxml1.cxx b/sd/qa/unit/export-tests-ooxml1.cxx index 1d6f62a70e62..d497887dafa0 100644 --- a/sd/qa/unit/export-tests-ooxml1.cxx +++ b/sd/qa/unit/export-tests-ooxml1.cxx @@ -485,7 +485,7 @@ CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest1, testN828390_4) { const SvxFontHeightItem* pFontHeight = dynamic_cast<const SvxFontHeightItem*>((*it).pAttr); - if (pFontHeight && (*it).nStart == 18) + if (pFontHeight && (*it).nStart == 19) CPPUNIT_ASSERT_EQUAL_MESSAGE("Font height is wrong", static_cast<sal_uInt32>(1129), pFontHeight->GetHeight()); const SvxFontItem* pFont = dynamic_cast<const SvxFontItem*>((*it).pAttr); diff --git a/xmloff/source/draw/ximpstyl.cxx b/xmloff/source/draw/ximpstyl.cxx index b3b0bc8978d3..ee94f5b3adab 100644 --- a/xmloff/source/draw/ximpstyl.cxx +++ b/xmloff/source/draw/ximpstyl.cxx @@ -1174,7 +1174,7 @@ void SdXMLStylesContext::ImpSetGraphicStyles() const { uno::Reference< container::XNameAccess > xGraphicPageStyles( GetSdImport().GetLocalDocStyleFamilies()->getByName(u"graphics"_ustr), uno::UNO_QUERY_THROW ); - ImpSetGraphicStyles(xGraphicPageStyles, XmlStyleFamily::SD_GRAPHICS_ID, u""); + ImpSetGraphicStyles(xGraphicPageStyles, XmlStyleFamily::SD_GRAPHICS_ID, u""_ustr); } catch( uno::Exception& ) { @@ -1188,7 +1188,7 @@ void SdXMLStylesContext::ImpSetCellStyles() const { uno::Reference< container::XNameAccess > xGraphicPageStyles( GetSdImport().GetLocalDocStyleFamilies()->getByName(u"cell"_ustr), uno::UNO_QUERY_THROW ); - ImpSetGraphicStyles(xGraphicPageStyles, XmlStyleFamily::TABLE_CELL, u""); + ImpSetGraphicStyles(xGraphicPageStyles, XmlStyleFamily::TABLE_CELL, u""_ustr); } catch( uno::Exception& ) { @@ -1230,41 +1230,33 @@ static bool canSkipReset(std::u16string_view rName, const XMLPropStyleContext* p // help function used by ImpSetGraphicStyles() and ImpSetMasterPageStyles() -void SdXMLStylesContext::ImpSetGraphicStyles( uno::Reference< container::XNameAccess > const & xPageStyles, XmlStyleFamily nFamily, std::u16string_view rPrefix) const +void SdXMLStylesContext::ImpSetGraphicStyles( uno::Reference< container::XNameAccess > const & xPageStyles, XmlStyleFamily nFamily, const OUString& rPrefix) const { - sal_Int32 nPrefLen(rPrefix.size()); - - sal_uInt32 a; + sal_Int32 nPrefLen(rPrefix.getLength()); // set defaults - for( a = 0; a < GetStyleCount(); a++) + auto [itStart1, itEnd1] = FindStyleChildContextByPrefix(nFamily, u""_ustr); + for (auto it = itStart1; it != itEnd1; ++it) { - const SvXMLStyleContext* pStyle = GetStyle(a); - - if(nFamily == pStyle->GetFamily() && pStyle->IsDefaultStyle()) + const SvXMLStyleContext* pStyle = *it; + if(pStyle->IsDefaultStyle()) { const_cast<SvXMLStyleContext*>(pStyle)->SetDefaults(); } } // create all styles and set properties - for( a = 0; a < GetStyleCount(); a++) + auto [itStart, itEnd] = FindStyleChildContextByPrefix(nFamily, rPrefix); + for (auto it = itStart; it != itEnd; ++it) { + const SvXMLStyleContext* pStyle = *it; try { - const SvXMLStyleContext* pStyle = GetStyle(a); - if(nFamily == pStyle->GetFamily() && !pStyle->IsDefaultStyle()) + if(!pStyle->IsDefaultStyle()) { OUString aStyleName(pStyle->GetDisplayName()); - if( nPrefLen ) - { - sal_Int32 nStylePrefLen = aStyleName.lastIndexOf( '-' ) + 1; - if( (nPrefLen != nStylePrefLen) || !aStyleName.startsWith(rPrefix) ) - continue; - aStyleName = aStyleName.copy( nPrefLen ); - } XMLPropStyleContext* pPropStyle = dynamic_cast< XMLPropStyleContext* >(const_cast< SvXMLStyleContext* >( pStyle ) ); @@ -1345,21 +1337,17 @@ void SdXMLStylesContext::ImpSetGraphicStyles( uno::Reference< container::XNameAc } // now set parents for all styles (when necessary) - for(a = 0; a < GetStyleCount(); a++) + for (auto it = itStart; it != itEnd; ++it) { - const SvXMLStyleContext* pStyle = GetStyle(a); + const SvXMLStyleContext* pStyle = *it; - if(pStyle && !pStyle->GetDisplayName().isEmpty() && (nFamily == pStyle->GetFamily())) try + if(pStyle->GetDisplayName().isEmpty()) + continue; + try { OUString aStyleName(pStyle->GetDisplayName()); if( nPrefLen ) - { - sal_Int32 nStylePrefLen = aStyleName.lastIndexOf( '-' ) + 1; - if( (nPrefLen != nStylePrefLen) || !aStyleName.startsWith( rPrefix ) ) - continue; - aStyleName = aStyleName.copy( nPrefLen ); - } uno::Reference< style::XStyle > xStyle( xPageStyles->getByName(aStyleName), UNO_QUERY ); if(xStyle.is()) diff --git a/xmloff/source/draw/ximpstyl.hxx b/xmloff/source/draw/ximpstyl.hxx index 1a89cc83bffa..3b0bbe237032 100644 --- a/xmloff/source/draw/ximpstyl.hxx +++ b/xmloff/source/draw/ximpstyl.hxx @@ -180,7 +180,7 @@ class SdXMLStylesContext : public SvXMLStylesContext void ImpSetGraphicStyles() const; void ImpSetCellStyles() const; void ImpSetGraphicStyles( css::uno::Reference< css::container::XNameAccess > const & xPageStyles, - XmlStyleFamily nFamily, std::u16string_view rPrefix) const; + XmlStyleFamily nFamily, const OUString& rPrefix) const; protected: using SvXMLStylesContext::CreateStyleChildContext; diff --git a/xmloff/source/style/xmlstyle.cxx b/xmloff/source/style/xmlstyle.cxx index e09ef37bfa12..ce5918f0ec94 100644 --- a/xmloff/source/style/xmlstyle.cxx +++ b/xmloff/source/style/xmlstyle.cxx @@ -155,68 +155,31 @@ bool SvXMLStyleContext::IsTransient() const } namespace { - -class SvXMLStyleIndex_Impl -{ - OUString sName; - XmlStyleFamily nFamily; - // we deliberately don't use a reference here, to avoid creating a ref-count-cycle - SvXMLStyleContext* mpStyle; - -public: - - SvXMLStyleIndex_Impl( XmlStyleFamily nFam, OUString aName ) : - sName(std::move( aName )), - nFamily( nFam ), - mpStyle(nullptr) - { - } - - SvXMLStyleIndex_Impl( const rtl::Reference<SvXMLStyleContext> &rStl ) : - sName( rStl->GetName() ), - nFamily( rStl->GetFamily() ), - mpStyle ( rStl.get() ) - { - } - - const OUString& GetName() const { return sName; } - XmlStyleFamily GetFamily() const { return nFamily; } - const SvXMLStyleContext *GetStyle() const { return mpStyle; } -}; - -struct SvXMLStyleIndexCmp_Impl +struct StyleVectorCompare { - bool operator()(const SvXMLStyleIndex_Impl& r1, const SvXMLStyleIndex_Impl& r2) const + bool operator()(const SvXMLStyleContext* r1, const SvXMLStyleContext* r2) const { - sal_Int32 nRet; - - if( r1.GetFamily() < r2.GetFamily() ) - nRet = -1; - else if( r1.GetFamily() > r2.GetFamily() ) - nRet = 1; - else - nRet = r1.GetName().compareTo( r2.GetName() ); - - return nRet < 0; + if( r1->GetFamily() < r2->GetFamily() ) + return true; + if( r1->GetFamily() > r2->GetFamily() ) + return false; + return r1->GetName() < r2->GetName(); } }; - } class SvXMLStylesContext_Impl { - typedef std::set<SvXMLStyleIndex_Impl, SvXMLStyleIndexCmp_Impl> IndicesType; - std::vector<rtl::Reference<SvXMLStyleContext>> aStyles; - mutable std::unique_ptr<IndicesType> pIndices; + // it would be better if we could share one vector for the styles and the index, but some code in calc + // is sensitive to having styles re-ordered + mutable SvXMLStylesContext::StyleIndex maStylesIndex; bool bAutomaticStyle; #if OSL_DEBUG_LEVEL > 0 mutable sal_uInt32 m_nIndexCreated; #endif - void FlushIndex() { pIndices.reset(); } - public: explicit SvXMLStylesContext_Impl( bool bAuto ); @@ -233,7 +196,15 @@ public: const SvXMLStyleContext *FindStyleChildContext( XmlStyleFamily nFamily, const OUString& rName, bool bCreateIndex ) const; + + std::pair<SvXMLStylesContext::StyleIndex::const_iterator, SvXMLStylesContext::StyleIndex::const_iterator> + FindStyleChildContextByPrefix( XmlStyleFamily nFamily, + const OUString& rPrefix ) const; + bool IsAutomaticStyle() const { return bAutomaticStyle; } + +private: + void BuildIndex() const; }; SvXMLStylesContext_Impl::SvXMLStylesContext_Impl( bool bAuto ) : @@ -252,12 +223,11 @@ inline void SvXMLStylesContext_Impl::AddStyle( SvXMLStyleContext *pStyle ) #endif aStyles.emplace_back(pStyle ); - FlushIndex(); + maStylesIndex.clear(); } void SvXMLStylesContext_Impl::dispose() { - FlushIndex(); aStyles.clear(); } @@ -267,23 +237,22 @@ const SvXMLStyleContext *SvXMLStylesContext_Impl::FindStyleChildContext( XmlStyl { const SvXMLStyleContext *pStyle = nullptr; - if( !pIndices && bCreateIndex && !aStyles.empty() ) - { - pIndices = std::make_unique<IndicesType>(aStyles.begin(), aStyles.end()); - SAL_WARN_IF(pIndices->size() != aStyles.size(), "xmloff.style", "Here is a duplicate Style"); -#if OSL_DEBUG_LEVEL > 0 - SAL_WARN_IF(0 != m_nIndexCreated, "xmloff.style", - "Performance warning: sdbcx::Index created multiple times"); - ++m_nIndexCreated; -#endif - } + if( maStylesIndex.empty() && bCreateIndex && !aStyles.empty() ) + BuildIndex(); - if( pIndices ) + if( !maStylesIndex.empty() ) { - SvXMLStyleIndex_Impl aIndex( nFamily, rName ); - IndicesType::iterator aFind = pIndices->find(aIndex); - if( aFind != pIndices->end() ) - pStyle = aFind->GetStyle(); + auto it = std::lower_bound(maStylesIndex.begin(), maStylesIndex.end(), true, + [&nFamily, &rName](const SvXMLStyleContext* lhs, bool /*rhs*/) + { + if (lhs->GetFamily() < nFamily) + return true; + if (lhs->GetFamily() > nFamily) + return false; + return lhs->GetName() < rName; + }); + if (it != maStylesIndex.end() && (*it)->GetFamily() == nFamily && (*it)->GetName() == rName) + pStyle = *it; } else { @@ -298,6 +267,55 @@ const SvXMLStyleContext *SvXMLStylesContext_Impl::FindStyleChildContext( XmlStyl return pStyle; } +namespace +{ +struct PrefixProbe +{ + XmlStyleFamily nFamily; + const OUString& rPrefix; + + bool operator()(const SvXMLStyleContext* lhs, bool /*rhs*/) + { + if (lhs->GetFamily() < nFamily) + return true; + if (lhs->GetFamily() > nFamily) + return false; + const OUString& lhsName = lhs->GetName(); + return lhsName.subView(0, std::min(lhsName.getLength(), rPrefix.getLength())) < rPrefix; + } + bool operator()(bool /*lhs*/, const SvXMLStyleContext* rhs) + { + if (nFamily < rhs->GetFamily()) + return true; + if (nFamily > rhs->GetFamily()) + return false; + const OUString& rhsName = rhs->GetName(); + return rPrefix < rhsName.subView(0, std::min(rhsName.getLength(), rPrefix.getLength())); + } +}; +} + +std::pair<SvXMLStylesContext::StyleIndex::const_iterator, SvXMLStylesContext::StyleIndex::const_iterator> +SvXMLStylesContext_Impl::FindStyleChildContextByPrefix( XmlStyleFamily nFamily, + const OUString& rPrefix ) const +{ + if( maStylesIndex.empty() ) + BuildIndex(); + return std::equal_range(maStylesIndex.begin(), maStylesIndex.end(), true, PrefixProbe{nFamily,rPrefix}); +} + +void SvXMLStylesContext_Impl::BuildIndex() const +{ + maStylesIndex.reserve(aStyles.size()); + for (const auto & i : aStyles) + maStylesIndex.push_back(i.get()); + std::sort(maStylesIndex.begin(), maStylesIndex.end(), StyleVectorCompare()); +#if OSL_DEBUG_LEVEL > 0 + SAL_WARN_IF(0 != m_nIndexCreated, "xmloff.style", + "Performance warning: sdbcx::Index created multiple times"); + ++m_nIndexCreated; +#endif +} sal_uInt32 SvXMLStylesContext::GetStyleCount() const { @@ -813,4 +831,12 @@ const SvXMLStyleContext *SvXMLStylesContext::FindStyleChildContext( return mpImpl->FindStyleChildContext( nFamily, rName, bCreateIndex ); } +std::pair<SvXMLStylesContext::StyleIndex::const_iterator, SvXMLStylesContext::StyleIndex::const_iterator> +SvXMLStylesContext::FindStyleChildContextByPrefix( + XmlStyleFamily nFamily, + const OUString& rNamePrefix ) const +{ + return mpImpl->FindStyleChildContextByPrefix( nFamily, rNamePrefix ); +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
