Cache single image wallpapers locally
Changes PlannedPublic

Authored by ngraham on Jan 16 2020, 11:27 PM.

Details

Summary

Right now, if the user sets a wallpaper and it disappears on disk, the next time Plasma
is restarted, the wallpaper will be reset to the default, which is probably not what
the user expected.

This patch caches the wallpaper locally every time the user specifically changes it.
Thereafter, if the path to the original wallpaper becomes invalid, Plasma will fall
back to reading the wallpaper from the cache.

BUG: 399905
FIXED-IN; 5.18.0

Test Plan
  • Set wallpaper to some image on disk
  • Delete or rename that file
  • Restart plasmashell
  • User-specified image is retained, rather than being replaced with the default wallpaper
  • Delete the cache file too
  • Restart plasmashell
  • Wallpaper is replaced with the default wallpaper as a fallback

No changes to slideshows or other wallpaper plugins.

Diff Detail

Repository
R120 Plasma Workspace
Branch
wallpaper-cache (branched from Plasma/5.18)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 21257
Build 21275: arc lint + arc unit
ngraham created this revision.Jan 16 2020, 11:27 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 16 2020, 11:27 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Jan 16 2020, 11:27 PM

Note that I found the code here very complicated so it's possible I may have done this in a sub-optimal way.

ngraham edited the test plan for this revision. (Show Details)Jan 16 2020, 11:31 PM

This doesn't appear to be containment-aware (e.g. multi-screen, activities, etc).
How does the settings page handle this scenario?

Good point, I totally forgot to consider that. I have no idea how it works and will need to look into it.

davidedmundson requested changes to this revision.Jan 19 2020, 6:28 PM
davidedmundson added a subscriber: davidedmundson.

This doesn't appear to be containment-aware (e.g. multi-screen, activities, etc).

As Kai said, changing status just to clean up the queue.

I was also under the impression that we wanted to perform crop and resize in the cached image to solve your other feature of only shipping 1 size with the distro.

This revision now requires changes to proceed.Jan 19 2020, 6:28 PM
trmdi added a subscriber: trmdi.Jan 20 2020, 2:03 PM
trmdi added inline comments.
wallpapers/image/image.cpp
503
ngraham planned changes to this revision.Jan 23 2020, 9:14 PM
trmdi added inline comments.Jan 24 2020, 2:16 AM
wallpapers/image/image.cpp
503

Sorry, I meant QDir::mkpath is needed for KIO::CopyJob, not here, otherwise CopyJob will fail when the destination directory does not exist.

clel added a subscriber: clel.Jun 23 2020, 1:45 PM

This doesn't appear to be containment-aware (e.g. multi-screen, activities, etc).

As Kai said, changing status just to clean up the queue.

I was also under the impression that we wanted to perform crop and resize in the cached image to solve your other feature of only shipping 1 size with the distro.

Just adding when it is planned to generate a device-(or srceen) optimized wallpaper in the future, one could also think about storing those image(s) in a special location for the current wallpaper and use them always as default. Might come with other problems, though.