sfx2/source/appl/appserv.cxx | 1
svtools/source/config/colorcfg.cxx | 51 ++++++++++++++++++++++++++++++-------
2 files changed, 42 insertions(+), 10 deletions(-)
New commits:
commit 4e04e96334a841865914c1629202912e343c4cb2
Author: Skyler Grey <skyler.g...@collabora.com>
AuthorDate: Tue Jul 30 10:17:31 2024 +0000
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Wed Aug 28 09:41:18 2024 +0200
fix(invert): Avoid spurious LOK invalidations
Using the same mechanism as with theme changes, we can avoid LOK getting
invalidations if we have an invert-background change that doesn't apply.
The trouble is that this method of inverting the background causes *lots
of* properties to change, so there's no single "If we inverted the
background" to check...
To get there, I've checked if all of the following are true
- We didn't change the color scheme
- We didn't have any new colors after this change
- All of the properties we were changing should have been within this
color scheme
While they don't necessarily mean "there was a background inversion",
they do mean "something changed in your theme but no action is needed
from you" - which should only be a background inversion - and if we
added anything else that could fit in that category, it should also
avoid LOK invalidations
Change-Id: Idb680d5241db7879d9be834268ab616848c1f165
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172505
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
diff --git a/sfx2/source/appl/appserv.cxx b/sfx2/source/appl/appserv.cxx
index 603b1b90648f..5d94b1d5786f 100644
--- a/sfx2/source/appl/appserv.cxx
+++ b/sfx2/source/appl/appserv.cxx
@@ -754,7 +754,6 @@ void SfxApplication::MiscExec_Impl( SfxRequest& rReq )
}
svtools::ColorConfigValue aValue;
- aValue.bIsVisible = true;
if(aNewTheme == "Dark")
aValue.nColor = aDefDarkColor;
diff --git a/svtools/source/config/colorcfg.cxx
b/svtools/source/config/colorcfg.cxx
index 46b796e5e226..699dfde0f3aa 100644
--- a/svtools/source/config/colorcfg.cxx
+++ b/svtools/source/config/colorcfg.cxx
@@ -255,25 +255,58 @@ void ColorConfig_Impl::Load(const OUString& rScheme)
void ColorConfig_Impl::Notify(const uno::Sequence<OUString>& rProperties)
{
- const bool bOnlyChangingCurrentColorScheme = rProperties.getLength() == 1
&& rProperties[0] == "CurrentColorScheme";
const OUString sOldLoadedScheme = m_sLoadedScheme;
- //loading via notification always uses the default setting
+ ColorConfigValue aOldConfigValues[ColorConfigEntryCount];
+ std::copy( m_aConfigValues, m_aConfigValues + ColorConfigEntryCount,
aOldConfigValues );
+
+ // loading via notification always uses the default setting
Load(OUString());
+ const bool bNoColorSchemeChange = sOldLoadedScheme == m_sLoadedScheme;
+
// If the name of the scheme hasn't changed, then there is no change to the
// global color scheme name, but Kit deliberately only changed the then
// current document when it last changed, so there are typically a mixture
// of documents with the original 'light' color scheme and the last changed
// color scheme 'dark'. Kit then tries to set the color scheme again to the
// last changed color scheme 'dark' to try and update a 'light' document
- // that had opted out of the last change to 'dark'. So tag such an apparent
- // null change attempt with 'OnlyCurrentDocumentColorScheme' to allow it to
- // go through, but identify what that change is for, so the other color
- // config listeners for whom it doesn't matter, can ignore it as an
- // optimization.
- const bool bOnlyCurrentDocumentColorScheme =
bOnlyChangingCurrentColorScheme && sOldLoadedScheme == m_sLoadedScheme &&
-
comphelper::LibreOfficeKit::isActive();
+ // that had opted out of the last change to 'dark'...
+ const bool bEmptyColorSchemeNotify =
+ rProperties.getLength() == 1
+ && rProperties[0] == "CurrentColorScheme"
+ && bNoColorSchemeChange;
+
+ // ...We can get into a similar situation with inverted backgrounds, for
+ // similar reasons, so even if we are only changing the current color
scheme
+ // we need to make sure that something actually changed...
+ bool bNoConfigChange = true;
+ for (int i = 0; i < ColorConfigEntryCount; ++i) {
+ if (aOldConfigValues[i] != m_aConfigValues[i]) {
+ bNoConfigChange = false;
+ break;
+ }
+ }
+
+ // ...and if something from a different color scheme changes, our config
+ // values wouldn't change anyway, so we need to make sure that if something
+ // changed it was this color scheme...
+ const OUString sCurrentSchemePropertyPrefix =
"ColorSchemes/org.openoffice.Office.UI:ColorScheme['" + m_sLoadedScheme + "']/";
+ bool bOnlyCurrentSchemeChanges = true;
+ for (int i = 0; i < rProperties.getLength(); ++i) {
+ if (!rProperties[i].startsWith(sCurrentSchemePropertyPrefix)) {
+ bOnlyCurrentSchemeChanges = false;
+ break;
+ }
+ }
+
+ bool bEmptyCurrentSchemeNotify = bNoColorSchemeChange && bNoConfigChange
&& bOnlyCurrentSchemeChanges;
+
+ // ...We can tag apparent null change attempts with
+ // 'OnlyCurrentDocumentColorScheme' to allow them to go through, but
+ // identify what that change is for, so the other color config listeners
for
+ // whom it doesn't matter, can ignore it as an optimization.
+ const bool bOnlyCurrentDocumentColorScheme = (bEmptyColorSchemeNotify ||
bEmptyCurrentSchemeNotify) && comphelper::LibreOfficeKit::isActive();
NotifyListeners(bOnlyCurrentDocumentColorScheme ?
ConfigurationHints::OnlyCurrentDocumentColorScheme : ConfigurationHints::NONE);
}
