Re: changing icon sizes no longer emits signal

2020-04-30 Thread Benjamin Port
On dimanche 26 avril 2020 14:24:27 CEST David Faure wrote:
> On Saturday, April 25, 2020 4:54:43 PM CEST Friedrich W. H. Kossebau wrote:
> > Am Samstag, 25. April 2020, 15:10:37 CEST schrieb Martin Koller:
> > > Hi,
> > > 
> > > in liquidshell I'm using
> > > 
> > >   connect(KIconLoader::global(),
> > >   ::iconLoaderSettingsChanged,
> > > 
> > > this, ::adjustIconSize);
> > > 
> > > to be informed whenever the icon sizes get changed in systemsettings.
> > > I don't know since when but at least in version
> > > 
> > > Operating System: openSUSE Tumbleweed 20200419
> > > KDE Plasma Version: 5.18.4
> > > KDE Frameworks Version: 5.69.0
> > > Qt Version: 5.14.1
> > > 
> > > I no longer receive this signal.
> > 
> > I can confirm that changing icon sizes in Plasma Systemsettings and
> > clicking apply seems to have no effect also for all other places which
> > would adapt to those values, like KXmlGui-driven toolbars (which also by
> > a quick check seems to connect to that signal). The changed sizes are
> > only picked up by newly started instances of programs.
> > I am also on openSUSE TW with same versions. Also not sure when I saw this
> > working the last time.
> > 
> > Given the API dox of KIconLoader does not note that signal is deprecated,
> > I
> > would assume this signal is still to be used (and main.cpp of the icon kcm
> > seems to call KIconLoader::emitChange(...), which should trigger the
> > signal
> > via D-Bus in all client side, from a peek view at code path.
> 
> The call to KIconLoader::emitChange is deeply nested in KDE4 compat stuff,
> with possible exit paths before.
> 
> A patch like the one attached seems to help, but someone who knows the KCM
> better (or has time to dig) should make this conditional on the user
> actually changing icon sizes, and only emit for the groups that have
> changed.

Solved by this one in theory, if you can confirm.

https://phabricator.kde.org/D29285

-- 
Benjamin Port - enioka Haute Couture
06 38 83 18 16




Re: changing icon sizes no longer emits signal

2020-04-27 Thread Kai Uwe Broulik
(That's why confirmation prompts are pointless, one gets so used to just 
acknowledge them all that that I sent this mail mid-sentece :)


...so perhaps we should just change it to always emit the dbus signal 
manually when something changed, rather than it being nested inside 
exportToKDE4().


So, basically like David's patch, except maybe just with the explicit 
DBus call, so we can get rid of the emitChange call, which I think might 
be kdelibs4support material?


Cheers
Kai Uwe



Re: changing icon sizes no longer emits signal

2020-04-27 Thread Kai Uwe Broulik

Hi,


A patch like the one attached seems to help, but someone who knows the KCM
better (or has time to dig) should make this conditional on the user actually
changing icon sizes, and only emit for the groups that have changed.


Patch makes sense, though I wonder why we do both emitChange and a 
manual DBus call. From what I gather emitChange does the same.


Plasma-Integration also connects to that signal to update icon sizes.

It looks like a flaw in the previous code, where it would always run 
exportToKDE4() when anything was changed, which would then emit the 
signal, which unbeknownst to me, was also still being used :)


In Plasma 5.17 it used to be:
> if (m_selectedThemeDirty || m_iconSizesDirty || m_revertIconEffects) {
> exportToKDE4();
> }

So perha



Re: changing icon sizes no longer emits signal

2020-04-26 Thread David Faure
On Saturday, April 25, 2020 4:54:43 PM CEST Friedrich W. H. Kossebau wrote:
> Am Samstag, 25. April 2020, 15:10:37 CEST schrieb Martin Koller:
> > Hi,
> > 
> > in liquidshell I'm using
> > 
> >   connect(KIconLoader::global(), ::iconLoaderSettingsChanged,
> > 
> > this, ::adjustIconSize);
> > 
> > to be informed whenever the icon sizes get changed in systemsettings.
> > I don't know since when but at least in version
> > 
> > Operating System: openSUSE Tumbleweed 20200419
> > KDE Plasma Version: 5.18.4
> > KDE Frameworks Version: 5.69.0
> > Qt Version: 5.14.1
> > 
> > I no longer receive this signal.
> 
> I can confirm that changing icon sizes in Plasma Systemsettings and clicking
> apply seems to have no effect also for all other places which would adapt
> to those values, like KXmlGui-driven toolbars (which also by a quick check
> seems to connect to that signal). The changed sizes are only picked up by
> newly started instances of programs.
> I am also on openSUSE TW with same versions. Also not sure when I saw this
> working the last time.
> 
> Given the API dox of KIconLoader does not note that signal is deprecated, I
> would assume this signal is still to be used (and main.cpp of the icon kcm
> seems to call KIconLoader::emitChange(...), which should trigger the signal
> via D-Bus in all client side, from a peek view at code path.

The call to KIconLoader::emitChange is deeply nested in KDE4 compat stuff, 
with possible exit paths before.

A patch like the one attached seems to help, but someone who knows the KCM 
better (or has time to dig) should make this conditional on the user actually 
changing icon sizes, and only emit for the groups that have changed.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5
diff --git i/kcms/icons/main.cpp w/kcms/icons/main.cpp
index 172d9bc18..4ab2f5150 100644
--- i/kcms/icons/main.cpp
+++ w/kcms/icons/main.cpp
@@ -148,7 +148,10 @@ void IconModule::save()
 
 processPendingDeletions();
 
-KIconLoader::global()->newIconLoader();
+// TODO do this only for the group(s) where icon sizes have changed
+for (int i = 0; i < KIconLoader::LastGroup; i++) {
+KIconLoader::emitChange(KIconLoader::Group(i));
+}
 }
 
 bool IconModule::isSaveNeeded() const
@@ -288,8 +291,6 @@ void IconModule::exportToKDE4()
 
 //message kde4 apps that icon theme is changed
 for (int i = 0; i < KIconLoader::LastGroup; i++) {
-KIconLoader::emitChange(KIconLoader::Group(i));
-
 QDBusMessage message = QDBusMessage::createSignal(QStringLiteral("/KGlobalSettings"),
   QStringLiteral("org.kde.KGlobalSettings"),
   QStringLiteral("notifyChange"));


Re: changing icon sizes no longer emits signal

2020-04-25 Thread Friedrich W. H. Kossebau
Am Samstag, 25. April 2020, 15:10:37 CEST schrieb Martin Koller:
> Hi,
> 
> in liquidshell I'm using
> 
>   connect(KIconLoader::global(), ::iconLoaderSettingsChanged,
> this, ::adjustIconSize);
> 
> to be informed whenever the icon sizes get changed in systemsettings.
> I don't know since when but at least in version
> 
> Operating System: openSUSE Tumbleweed 20200419
> KDE Plasma Version: 5.18.4
> KDE Frameworks Version: 5.69.0
> Qt Version: 5.14.1
> 
> I no longer receive this signal.

I can confirm that changing icon sizes in Plasma Systemsettings and clicking 
apply seems to have no effect also for all other places which would adapt to 
those values, like KXmlGui-driven toolbars (which also by a quick check seems 
to connect to that signal). The changed sizes are only picked up by newly 
started instances of programs.
I am also on openSUSE TW with same versions. Also not sure when I saw this 
working the last time.

Given the API dox of KIconLoader does not note that signal is deprecated, I 
would assume this signal is still to be used (and main.cpp of the icon kcm 
seems to call KIconLoader::emitChange(...), which should trigger the signal 
via D-Bus in all client side, from a peek view at code path.

Please file a bug, to Plasma Systemsettings I would do. Or debug if you can, 
some developer karma waiting to be picked up for who does :)

Cheers
Friedrich




changing icon sizes no longer emits signal

2020-04-25 Thread Martin Koller
Hi,

in liquidshell I'm using

  connect(KIconLoader::global(), ::iconLoaderSettingsChanged, this, 
::adjustIconSize);

to be informed whenever the icon sizes get changed in systemsettings.
I don't know since when but at least in version

Operating System: openSUSE Tumbleweed 20200419
KDE Plasma Version: 5.18.4
KDE Frameworks Version: 5.69.0
Qt Version: 5.14.1

I no longer receive this signal.
Does it have to do with the changed UI for changing the icon sizes
or something else ?

-- 
Best regards/Schöne Grüße

Martin
A: Because it breaks the logical sequence of discussion
Q: Why is top posting bad?

()  ascii ribbon campaign - against html e-mail 
/\- against proprietary attachments

Frühstück, Geschenkideen, Accessoires, Kulinarisches: www.lillehus.at