SUMMARY When setting a desktop containment to use the slideshow plugin, when the slideshow changes the wallpaper image to a new image, it used to be that the plugin will update the configuration file ~/.config/plasma-org.kde.plasma.desktop-appletsrc with the new image URL stored in the entry `[Containments][ID][Wallpaper][org.kde.slideshow][General]` under the key `Image`. Also the DBUS API would answer the call `org.kde.plasmashell /PlasmaShell org.kde.plasmashell.wallpaper <SCREENNUM>` and will list the current wallpaper image. In Plasma 6 this no longer works. STEPS TO REPRODUCE 1. Set the wallpaper to use the image slideshow plugin, with a few images available 2. List the current wallpaper image through the DBUS API: `qdbus org.kde.plasmashell /PlasmaShell org.kde.PlasmaShell.wallpaper 0` (assuming the primary screen) 3. Right click the desktop and choose "Next Wallpaper" 4. List the current wallpaper image again through the DBUS API OBSERVED RESULT The output from the DBUS API call remains unchanged EXPECTED RESULT The "Image" value in the DBUS API result should change to reflect the new image. SOFTWARE/OS VERSIONS Operating System: KDE neon Testing Edition KDE Plasma Version: 6.0.0 KDE Frameworks Version: 6.0.0 Qt Version: 6.6.2 Kernel Version: 6.5.0-17-generic (64-bit) Graphics Platform: Wayland Processors: 20 × 12th Gen Intel® Core™ i7-12700H Memory: 31.0 GiB of RAM Graphics Processor: Mesa Intel® Graphics ADDITIONAL INFORMATION I tried to figure out the problem on my own, and I have verified that the code in plasma-workspace/wallpapers/image/plugin/imagebackend.cpp does correctly call `ImageBackend::saveCurrentWallpaper()`, which does correctly calls the QML `WallpaperItem.imageView.imageWallpaper.writeImageConfig()`, and that correctly updates the `Image` field in the config map. But that change isn't propagated where I expect it to be updated and at this point I don't know where else to look. This may be related to the change that fixed bug #461490 but I am unsure how.
KConfigPropertyMapPrivate::writeConfigValue doesn't sync the config immediately
(In reply to Fushan Wen from comment #1) > KConfigPropertyMapPrivate::writeConfigValue doesn't sync the config > immediately Can it? Should it? I understand performance reasons because disk IO sth sth, but: 1. Will it ever sync to the files? Is there an external (DBUS) API to request the sync? I failed to find one and failed to wait long enough for the sync to occur spontaneously, including when plasmashell is closing. 2. What about the data in the DBUS `org.kde.PlasmaShell.wallpaper response? it shouldn't be a performance issue to have that data available in memory for a DBUS request. I tried to do this horrible thing: diff --git a/wallpapers/image/plugin/imagebackend.cpp b/wallpapers/image/plugin/imagebackend.cpp index e8d51ba2bf..2ab21e1f54 100644 --- a/wallpapers/image/plugin/imagebackend.cpp +++ b/wallpapers/image/plugin/imagebackend.cpp @@ -23,6 +23,7 @@ #include <QStandardPaths> #include <KLocalizedString> +#include <KConfigPropertyMap> #include "model/imageproxymodel.h" #include "slidefiltermodel.h" @@ -189,6 +190,7 @@ void ImageBackend::saveCurrentWallpaper() } QMetaObject::invokeMethod(this, "writeImageConfig", Qt::QueuedConnection, Q_ARG(QString, m_image.toString())); + static_cast<KConfigPropertyMap*>(m_configMap.data())->writeConfig(); } QAbstractItemModel *ImageBackend::slideFilterModel() const And now it updates the configuration - but with the previous wallpaper image?!? (i.e. if showing a.jpeg, then saveCurrentWallpaper() is called to save "b.jpeg", after that the configuration DBUS and file will show "a.jpeg")
This works better: diff --git a/wallpapers/image/plugin/imagebackend.cpp b/wallpapers/image/plugin/imagebackend.cpp index e8d51ba2bf..754fb7077b 100644 --- a/wallpapers/image/plugin/imagebackend.cpp +++ b/wallpapers/image/plugin/imagebackend.cpp @@ -21,10 +21,11 @@ #include <QMimeDatabase> #include <QScreen> #include <QStandardPaths> #include <KLocalizedString> +#include <KConfigPropertyMap> #include "model/imageproxymodel.h" #include "slidefiltermodel.h" #include "slidemodel.h" @@ -356,10 +357,13 @@ void ImageBackend::setConfigMap(QQmlPropertyMap *configMap) connect(m_configMap, &QQmlPropertyMap::valueChanged, this, [this](const QString &key, const QVariant & /* value */) { if (key == QStringLiteral("Image")) { Q_EMIT configMapChanged(); } + if (!m_configMap.isNull()) { + static_cast<KConfigPropertyMap*>(m_configMap.data())->writeConfig(); + } }); saveCurrentWallpaper(); } Still horrible, but at least gets the job done. As a general concept - if the API was sane - would that be acceptable?
Depends on how much we want to prioritize avoiding writes to the config file, I guess. If the user does something silly like change their wallpaper every 5 seconds, we could end up doing a lot of writes. So it depends at least partially on how much we care about this use case.
(In reply to Nate Graham from comment #4) > Depends on how much we want to prioritize avoiding writes to the config > file, I guess. If the user does something silly like change their wallpaper > every 5 seconds, we could end up doing a lot of writes. Is writing the config file every 5 seconds an issue? The config file is synced for a lot of other things - such as moving widgets. When someone is setting up a new widget on their desktop, the config file will be updated multiple times during that process. If the user does RMB -> next wallpaper, again and again - it is user interaction and updating the config file in response seems like an appropriate measure. > So it depends at least partially on how much we care about this use case. Well, obviously _I_ care about this use case 😅 I have a lot of wallpapers and a bunch of automation tools that help me manage them and they interact with the plasma shell using DBus and shell scripting - I'm not actually reading the config file, but both DBus and shell scripting use the same mechanism that - at least with the current code AFAIK - won't update unless we do `KConfigPropertyMap::writeConfig()`. Some of those tools are presented as desktop widgets, so when the wallpaper changes (due to either timer or actually doing "Next wallpaper") I'd expect the widget to reflect the current wallpaper.
Sure, that's why I marked it as CONFIRMED. :) My point is that we need to consider as a part of this how to avoid excessive config file writes. In the worst-case scenario where the user has the wallpaper changing every 5 seconds and keeps the system on 24/7, that's over 17,000 writes daily--far more than the writes that take place when you move widgets around. So either we decide we care about optimizing that, or we decide we don't and tell people "don't change your wallpaper every 5 seconds". Maybe we should even reduce the minimum wallpaper transition time to 1 minute.
(In reply to Nate Graham from comment #6) > In the worst-case scenario where the user has the wallpaper changing every 5 > seconds and keeps the system on 24/7, that's over 17,000 writes daily--far > more than the writes that take place when you move widgets around. I can add a simple heuristic that has a minimum time between writes and a burst budget, so with - lets say 1 minute and burst 10 - would let you make 10 writes quickly but on average not more than 1 write a minute. Shouldn't be more than a dozen LoC. > So either we decide we care about optimizing that, or we decide we don't and > tell people "don't change your wallpaper every 5 seconds". Maybe we should > even reduce the minimum wallpaper transition time to 1 minute. On MS-Windows you can't set the slideshow to more frequent than 1 minute, but most other desktops let you go down to 1 second - and I kind of don't want to degrade the current capabilities.
Thinking about it a bit more - as I've mentioned, I don't actually care about the contents of the on-disk file, I just want the DBus APIs (direct responses and shell scripting) to see the changes. I wonder if there's another way to sync the changes to they are visible to the external APIs without actually opening files and moving bits to storage.
Your plan sounds great!
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/3987
I pushed an MR with what I was doing and I would appreciate any feedback.
Git commit 37d6e6b5043cd2a44e19d668b113c8d2ebeedb73 by Fushan Wen. Committed on 09/03/2024 at 02:01. Pushed by fusionfuture into branch 'master'. shell: read wallpaper config from plugin instead of local file The local config is not always up-to-date. FIXED-IN: 6.0 M +11 -15 shell/shellcorona.cpp https://invent.kde.org/plasma/plasma-workspace/-/commit/37d6e6b5043cd2a44e19d668b113c8d2ebeedb73
Git commit c940d214b794d0ca4d822aa40cb64199101e76cf by Fushan Wen. Committed on 09/03/2024 at 02:22. Pushed by fusionfuture into branch 'Plasma/6.0'. shell: read wallpaper config from plugin instead of local file The local config is not always up-to-date. FIXED-IN: 6.0 (cherry picked from commit 37d6e6b5043cd2a44e19d668b113c8d2ebeedb73) M +11 -15 shell/shellcorona.cpp https://invent.kde.org/plasma/plasma-workspace/-/commit/c940d214b794d0ca4d822aa40cb64199101e76cf
Thank you Fushan Wen. With the current version of plasma-workspace in Neon testing, I can see that the D-Bus API response for `org.kde.plasmashell /PlasmaShell org.kde.plasmashell.wallpaper` does indeed return the current wallpaper. My remaining concern if with the desktop scripting API (https://develop.kde.org/docs/plasma/scripting/). In Plasma 5 I could run something like this (using the interactive console, or the D-Bus API `org.kde.PlasmaShell.evaluateScript`): for (let d of desktopsForActivity('114ebbc8-f947-493d-bc5e-3762eab23c9a')) { if (d.screen != 1) continue; if (d.wallpaperPlugin != 'org.kde.slideshow') continue; // for (let h in d) // print("Arg: " + h + " => " + d[h] + "\n"); d.currentConfigGroup = ['Wallpaper',d.wallpaperPlugin,'General']; print(d.readConfig('Image') + "\n"); } And that would print the current wallpaper image selected by the slideshow. With Plasma 6, without force syncing the configuration files (as the slideshow plugin hack was doing), this scripting code prints the file name of some very old selected image (I think from the last log out, but I'm not sure). I wonder if there's a way to get the desktop scripting API to also see the configuration change.
My current workaround - with the bug fix - is to extract the image file using the command `qdbus org.kde.plasmashell /PlasmaShell org.kde.PlasmaShell.wallpaper 1 | awk '$1=="Image:"{print$2}' | sed s,file://,,` The main problem with that is that it isn't activity aware - so I can only retrieve the current image file name for the current activity. It is OK for *most* of what I'm doing, but I think a solution that fixes desktop scripting will be a good idea.
That indicates there is a deeper problem in plasmashell