strepsirrhini/office-gobmx
9
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

tdf#164046 style searching improvement breaks Impress layout

Browse source 
regression from
    commit 6c2f827697
    Author: Noel Grandin <noel.grandin@collabora.co.uk>
    Date:   Sun Oct 20 15:07:41 2024 +0200
    improve style searching in SvXMLStylesContext

Two problems - the original code was searching by DisplayName,
not Name, so add another index for DisplayName,
and the prefix matching was not properly matching.

Change-Id: Ifd43bdb89d33067954298115c8700b87c7f93050
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177488
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
This commit is contained in:
Noel Grandin 2024-11-28 16:09:47 +02:00
parent e627686deb
commit b7c6bc0bda
3 changed files with 68 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
include/xmloff/xmlstyle.hxx
View file

@ -184,7 +184,7 @@ public:
const OUString& rName, const OUString& rName,
bool bCreateIndex = false ) const; bool bCreateIndex = false ) const;
std::pair<StyleIndex::const_iterator, StyleIndex::const_iterator> std::pair<StyleIndex::const_iterator, StyleIndex::const_iterator>
FindStyleChildContextByPrefix( FindStyleChildContextByDisplayNamePrefix(
XmlStyleFamily nFamily, XmlStyleFamily nFamily,
const OUString& rNamePrefix) const; const OUString& rNamePrefix) const;
static XmlStyleFamily GetFamily( std::u16string_view rFamily ); static XmlStyleFamily GetFamily( std::u16string_view rFamily );

4
xmloff/source/draw/ximpstyl.cxx
View file

@ -1235,7 +1235,7 @@ void SdXMLStylesContext::ImpSetGraphicStyles( uno::Reference< container::XNameAc
sal_Int32 nPrefLen(rPrefix.getLength()); sal_Int32 nPrefLen(rPrefix.getLength());
// set defaults // set defaults
auto [itStart1, itEnd1] = FindStyleChildContextByPrefix(nFamily, u""_ustr); auto [itStart1, itEnd1] = FindStyleChildContextByDisplayNamePrefix(nFamily, u""_ustr);
for (auto it = itStart1; it != itEnd1; ++it) for (auto it = itStart1; it != itEnd1; ++it)
{ {
const SvXMLStyleContext* pStyle = *it; const SvXMLStyleContext* pStyle = *it;
@ -1246,7 +1246,7 @@ void SdXMLStylesContext::ImpSetGraphicStyles( uno::Reference< container::XNameAc
} }
// create all styles and set properties // create all styles and set properties
auto [itStart, itEnd] = FindStyleChildContextByPrefix(nFamily, rPrefix); auto [itStart, itEnd] = FindStyleChildContextByDisplayNamePrefix(nFamily, rPrefix);
for (auto it = itStart; it != itEnd; ++it) for (auto it = itStart; it != itEnd; ++it)
{ {
const SvXMLStyleContext* pStyle = *it; const SvXMLStyleContext* pStyle = *it;

90
xmloff/source/style/xmlstyle.cxx
View file

@ -153,7 +153,7 @@ bool SvXMLStyleContext::IsTransient() const
} }
namespace { namespace {
struct StyleVectorCompare struct StyleIndexCompareByName
{ {
bool operator()(const SvXMLStyleContext* r1, const SvXMLStyleContext* r2) const bool operator()(const SvXMLStyleContext* r1, const SvXMLStyleContext* r2) const
{ {
@ -164,6 +164,17 @@ struct StyleVectorCompare
return r1->GetName() < r2->GetName(); return r1->GetName() < r2->GetName();
} }
}; };
struct StyleIndexCompareByDisplayName
{
bool operator()(const SvXMLStyleContext* r1, const SvXMLStyleContext* r2) const
{
if( r1->GetFamily() < r2->GetFamily() )
return true;
if( r1->GetFamily() > r2->GetFamily() )
return false;
return r1->GetDisplayName() < r2->GetDisplayName();
}
};
} }
class SvXMLStylesContext_Impl class SvXMLStylesContext_Impl
@ -171,7 +182,8 @@ class SvXMLStylesContext_Impl
std::vector<rtl::Reference<SvXMLStyleContext>> aStyles; std::vector<rtl::Reference<SvXMLStyleContext>> aStyles;
// it would be better if we could share one vector for the styles and the index, but some code in calc // 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 // is sensitive to having styles re-ordered
mutable SvXMLStylesContext::StyleIndex maStylesIndex; mutable SvXMLStylesContext::StyleIndex maStylesIndexByName;
mutable SvXMLStylesContext::StyleIndex maStylesIndexByDisplayName;
bool bAutomaticStyle; bool bAutomaticStyle;
#if OSL_DEBUG_LEVEL > 0 #if OSL_DEBUG_LEVEL > 0
@ -196,13 +208,14 @@ public:
bool bCreateIndex ) const; bool bCreateIndex ) const;
std::pair<SvXMLStylesContext::StyleIndex::const_iterator, SvXMLStylesContext::StyleIndex::const_iterator> std::pair<SvXMLStylesContext::StyleIndex::const_iterator, SvXMLStylesContext::StyleIndex::const_iterator>
FindStyleChildContextByPrefix( XmlStyleFamily nFamily, FindStyleChildContextByDisplayNamePrefix( XmlStyleFamily nFamily,
const OUString& rPrefix ) const; const OUString& rPrefix ) const;
bool IsAutomaticStyle() const { return bAutomaticStyle; } bool IsAutomaticStyle() const { return bAutomaticStyle; }
private: private:
void BuildIndex() const; void BuildNameIndex() const;
void BuildDisplayNameIndex() const;
}; };
SvXMLStylesContext_Impl::SvXMLStylesContext_Impl( bool bAuto ) : SvXMLStylesContext_Impl::SvXMLStylesContext_Impl( bool bAuto ) :
@ -221,7 +234,8 @@ inline void SvXMLStylesContext_Impl::AddStyle( SvXMLStyleContext *pStyle )
#endif #endif
aStyles.emplace_back(pStyle ); aStyles.emplace_back(pStyle );
maStylesIndex.clear(); maStylesIndexByName.clear();
maStylesIndexByDisplayName.clear();
} }
void SvXMLStylesContext_Impl::dispose() void SvXMLStylesContext_Impl::dispose()
@ -235,12 +249,12 @@ const SvXMLStyleContext *SvXMLStylesContext_Impl::FindStyleChildContext( XmlStyl
{ {
const SvXMLStyleContext *pStyle = nullptr; const SvXMLStyleContext *pStyle = nullptr;
if( maStylesIndex.empty() && bCreateIndex && !aStyles.empty() ) if( maStylesIndexByName.empty() && bCreateIndex && !aStyles.empty() )
BuildIndex(); BuildNameIndex();
if( !maStylesIndex.empty() ) if( !maStylesIndexByName.empty() )
{ {
auto it = std::lower_bound(maStylesIndex.begin(), maStylesIndex.end(), true, auto it = std::lower_bound(maStylesIndexByName.begin(), maStylesIndexByName.end(), true,
[&nFamily, &rName](const SvXMLStyleContext* lhs, bool /*rhs*/) [&nFamily, &rName](const SvXMLStyleContext* lhs, bool /*rhs*/)
{ {
if (lhs->GetFamily() < nFamily) if (lhs->GetFamily() < nFamily)
@ -249,7 +263,7 @@ const SvXMLStyleContext *SvXMLStylesContext_Impl::FindStyleChildContext( XmlStyl
return false; return false;
return lhs->GetName() < rName; return lhs->GetName() < rName;
}); });
if (it != maStylesIndex.end() && (*it)->GetFamily() == nFamily && (*it)->GetName() == rName) if (it != maStylesIndexByName.end() && (*it)->GetFamily() == nFamily && (*it)->GetName() == rName)
pStyle = *it; pStyle = *it;
} }
else else
@ -267,7 +281,7 @@ const SvXMLStyleContext *SvXMLStylesContext_Impl::FindStyleChildContext( XmlStyl
namespace namespace
{ {
struct PrefixProbe struct PrefixProbeLowerBound
{ {
XmlStyleFamily nFamily; XmlStyleFamily nFamily;
const OUString& rPrefix; const OUString& rPrefix;
@ -278,36 +292,54 @@ struct PrefixProbe
return true; return true;
if (lhs->GetFamily() > nFamily) if (lhs->GetFamily() > nFamily)
return false; return false;
const OUString& lhsName = lhs->GetName(); return lhs->GetDisplayName() < rPrefix;
return lhsName.subView(0, std::min(lhsName.getLength(), rPrefix.getLength())) < rPrefix;
} }
};
struct PrefixProbeUpperBound
{
XmlStyleFamily nFamily;
const OUString& rPrefix;
bool operator()(bool /*lhs*/, const SvXMLStyleContext* rhs) bool operator()(bool /*lhs*/, const SvXMLStyleContext* rhs)
{ {
if (nFamily < rhs->GetFamily()) if (nFamily < rhs->GetFamily())
return true; return true;
if (nFamily > rhs->GetFamily()) if (nFamily > rhs->GetFamily())
return false; return false;
const OUString& rhsName = rhs->GetName(); std::u16string_view rhsName = rhs->GetDisplayName();
return rPrefix < rhsName.subView(0, std::min(rhsName.getLength(), rPrefix.getLength())); // For the upper bound we want to view the vector's data as if
// every element was truncated to the size of the prefix.
// Then perform a normal match.
rhsName = rhsName.substr(0, rPrefix.getLength());
// compare UP TO the length of the prefix and no farther
if (int cmp = rPrefix.compareTo(rhsName))
return cmp < 0;
// The strings are equal to the length of the prefix so
// behave as if they are equal. That means s1 < s2 == false
return false;
} }
}; };
} }
std::pair<SvXMLStylesContext::StyleIndex::const_iterator, SvXMLStylesContext::StyleIndex::const_iterator> std::pair<SvXMLStylesContext::StyleIndex::const_iterator, SvXMLStylesContext::StyleIndex::const_iterator>
SvXMLStylesContext_Impl::FindStyleChildContextByPrefix( XmlStyleFamily nFamily, SvXMLStylesContext_Impl::FindStyleChildContextByDisplayNamePrefix( XmlStyleFamily nFamily,
const OUString& rPrefix ) const const OUString& rPrefix ) const
{ {
if( maStylesIndex.empty() ) if( maStylesIndexByDisplayName.empty() )
BuildIndex(); BuildDisplayNameIndex();
return std::equal_range(maStylesIndex.begin(), maStylesIndex.end(), true, PrefixProbe{nFamily,rPrefix}); auto itStart = std::lower_bound(maStylesIndexByDisplayName.begin(), maStylesIndexByDisplayName.end(), true, PrefixProbeLowerBound{nFamily,rPrefix});
auto itEnd = std::upper_bound(itStart, maStylesIndexByDisplayName.end(), true, PrefixProbeUpperBound{nFamily,rPrefix});
return {itStart, itEnd};
} }
void SvXMLStylesContext_Impl::BuildIndex() const void SvXMLStylesContext_Impl::BuildNameIndex() const
{ {
maStylesIndex.reserve(aStyles.size()); maStylesIndexByName.reserve(aStyles.size());
for (const auto & i : aStyles) for (const auto & i : aStyles)
maStylesIndex.push_back(i.get()); maStylesIndexByName.push_back(i.get());
std::sort(maStylesIndex.begin(), maStylesIndex.end(), StyleVectorCompare()); std::sort(maStylesIndexByName.begin(), maStylesIndexByName.end(), StyleIndexCompareByName());
#if OSL_DEBUG_LEVEL > 0 #if OSL_DEBUG_LEVEL > 0
SAL_WARN_IF(0 != m_nIndexCreated, "xmloff.style", SAL_WARN_IF(0 != m_nIndexCreated, "xmloff.style",
"Performance warning: sdbcx::Index created multiple times"); "Performance warning: sdbcx::Index created multiple times");
@ -315,6 +347,14 @@ void SvXMLStylesContext_Impl::BuildIndex() const
#endif #endif
} }
void SvXMLStylesContext_Impl::BuildDisplayNameIndex() const
{
maStylesIndexByDisplayName.reserve(aStyles.size());
for (const auto & i : aStyles)
maStylesIndexByDisplayName.push_back(i.get());
std::sort(maStylesIndexByDisplayName.begin(), maStylesIndexByDisplayName.end(), StyleIndexCompareByDisplayName());
}
sal_uInt32 SvXMLStylesContext::GetStyleCount() const sal_uInt32 SvXMLStylesContext::GetStyleCount() const
{ {
return mpImpl->GetStyleCount(); return mpImpl->GetStyleCount();
@ -830,11 +870,11 @@ const SvXMLStyleContext *SvXMLStylesContext::FindStyleChildContext(
} }
std::pair<SvXMLStylesContext::StyleIndex::const_iterator, SvXMLStylesContext::StyleIndex::const_iterator> std::pair<SvXMLStylesContext::StyleIndex::const_iterator, SvXMLStylesContext::StyleIndex::const_iterator>
SvXMLStylesContext::FindStyleChildContextByPrefix( SvXMLStylesContext::FindStyleChildContextByDisplayNamePrefix(
XmlStyleFamily nFamily, XmlStyleFamily nFamily,
const OUString& rNamePrefix ) const const OUString& rNamePrefix ) const
{ {
return mpImpl->FindStyleChildContextByPrefix( nFamily, rNamePrefix ); return mpImpl->FindStyleChildContextByDisplayNamePrefix( nFamily, rNamePrefix );
} }
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ /* vim:set shiftwidth=4 softtabstop=4 expandtab: */