D18005: Include only the largest size for each wallpaper

This revision was automatically updated to reflect the committed changes. Closed by commit R131:c39e8d7a1003: Include only the largest size for each wallpaper (authored by ngraham). REPOSITORY R131 Plasma Wallpapers CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18005?vs=49285&id=492

D18005: Include only the largest size for each wallpaper

ngraham added a comment. In D18005#391626 , @filipf wrote: > For D18078 it would be best to wait until a save cache is implemented, right? I believe so. Once we have that, we can remove these compatibi

D18005: Include only the largest size for each wallpaper

filipf added a comment. For D18078 it would be best to wait until a save cache is implemented, right? BTW, can you **add summer_1am** to `CMakeLists.txt` as a part of this diff? This poor wallpaper has been in the repo for over 3 years but was never b

D18005: Include only the largest size for each wallpaper

ngraham updated this revision to Diff 49285. ngraham added a comment. Really add symlinks REPOSITORY R131 Plasma Wallpapers CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18005?vs=49283&id=49285 BRANCH arcpatch-D18005 REVISION DETAIL https://phabricator.kde.org/D18005 AFFEC

D18005: Include only the largest size for each wallpaper

ngraham updated this revision to Diff 49283. ngraham added a comment. Add compatibility symlinks for images shipped in prior releases REPOSITORY R131 Plasma Wallpapers CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18005?vs=48779&id=49283 BRANCH arcpatch-D18005 REVISION DETAIL

D18005: Include only the largest size for each wallpaper

davidedmundson added a comment. > This is no longer a technical constraint that we face. I don't think this was ever a thing. - Ok, lets do it with the symlink thing. REPOSITORY R131 Plasma Wallpapers BRANCH keep-only-largest-size (branched from master) REVISION DETAIL

D18005: Include only the largest size for each wallpaper

ngraham added a comment. In D18005#389553 , @davidedmundson wrote: > If we're going to save it, we could save the post-scaled version which would nullify the increased loading cost argument...and it would be a prerequisite for having a single w

D18005: Include only the largest size for each wallpaper

davidedmundson added a comment. > Could Plasma always copy the used wallpaper to some location and link to that? "Could it" yes. "Does it" no. If we're going to save it, we could save the post-scaled version which would nullify the increased loading cost argument...and it would

D18005: Include only the largest size for each wallpaper

ngraham added a comment. Does @cfeck's idea to turn the smaller sizes into symlinks make sense as a transitional step to preserve the user experience for people who are current users of these wallpapers? REPOSITORY R131 Plasma Wallpapers BRANCH keep-only-largest-size (branched from mast

D18005: Include only the largest size for each wallpaper

davidedmundson added a comment. > any ideas on this front? >> [Containments][11][Wallpaper][org.kde.image][General] >> Image=file:///home/opt/kde5/share/wallpapers/FallenLeaf/contents/images/1280x1024.jpg Apparently we do store by full path which is quite a good argument for

D18005: Include only the largest size for each wallpaper

filipf added a comment. Could Plasma always copy the used wallpaper to some location and link to that? This is actually an annoyance irrespective of this discussion, if you move your wallpaper on the disk you lose the wallpaper. REPOSITORY R131 Plasma Wallpapers BRANCH keep-only-largest

D18005: Include only the largest size for each wallpaper

GB_2 added a comment. @cfeck had a good idea: In D18005#387519 , @cfeck wrote: > Maybe symlinking all to the largest version? REPOSITORY R131 Plasma Wallpapers BRANCH keep-only-largest-size (branched from master) REVISION DETAIL http

D18005: Include only the largest size for each wallpaper

ngraham added a subscriber: mart. ngraham added a comment. So it sounds like our only technical blocker is that we don't want people who are using a wallpaper whose small sizes disappear on disk to get an ugly black background. @davidedmundson or @mart, any ideas on this front? REPOSITORY

D18005: Include only the largest size for each wallpaper

filipf added a subscriber: rooty. filipf added a comment. It's a total non-issue. @rooty and I have been shifting through loads of wallpapers in the past month or so and it hasn't even occurred to us that this (image editor vs. Plasma cropping) might even be a thing. The difference is minor

D18005: Include only the largest size for each wallpaper

ngraham added a comment. FWIW I agree that the image editor scaling vs Plasma scaling is a non-issue. Even in the worst-case scenario, the only distinguishable differences are when you deliberately do a flip-between-them comparison. For most, the differences are negligible and non-regressive

D18005: Include only the largest size for each wallpaper

ngraham added a comment. In D18005#388044 , @rikmills wrote: > Instead Plasma should perhaps recognise that the image has gone, and seek and apply whatever single larger size now exists. In D18005#388048

D18005: Include only the largest size for each wallpaper

filipf added a comment. In D18005#387607 , @davidedmundson wrote: > - Our run-time resizes will look worse than gimp/inkscape doing it. Letting Plasma scale this 3972x2979 wallpaper . F653332

D18005: Include only the largest size for each wallpaper

davidedmundson added a comment. > Perhaps we should alert packagers that for the 5.15 release, they shouldn't remove these files from users' machines when they disappear from the repo. That's not do-able. Why does it result in a black screen? If we are storing the image by path rath

D18005: Include only the largest size for each wallpaper

rikmills added a comment. In D18005#387324 , @ngraham wrote: > However, just removing files from the repo doesn't actually remove them from users' machines; their packaging system does that. Perhaps we should alert packagers that for the 5.15 re

D18005: Include only the largest size for each wallpaper

filipf added a comment. I've booted up Debian 8, KDE SC version 4.14.2. The wallpapers scaled nicely there so I understand your curiosity. Taking a look at this list (https://packages.debian.org/de/jessie/all/kde-wallpapers/filelist) I notice three things: 1. some of the wallpapers

D18005: Include only the largest size for each wallpaper

davidedmundson added a comment. > @filipf That looks about right! It's not. That's only within 5.x. This code has been around since at least ede349f8f2db9930c96ab4e56f9440ba82c24cb7 on kde-workspace which w

D18005: Include only the largest size for each wallpaper

ngraham added a comment. @filipf That looks about right! @davidedmundson Reading through that bug report, it looks like we do some magic to avoid storing huge pixmaps in memory, so is it true that this change would avoid increasing users' memory consumption? I was thinking that on future

D18005: Include only the largest size for each wallpaper

filipf added a comment. Couldn't it be this? https://bugs.kde.org/show_bug.cgi?id=338506 REPOSITORY R131 Plasma Wallpapers BRANCH keep-only-largest-size (branched from master) REVISION DETAIL https://phabricator.kde.org/D18005 To: ngraham, #vdg, #plasma, ndavis Cc: filipf, davidedmund

D18005: Include only the largest size for each wallpaper

davidedmundson added a comment. I can list bugs that you might have seen in 5.x. I can't name things in 4.x. which is when it was introduced. Its before my plasma time. REPOSITORY R131 Plasma Wallpapers BRANCH keep-only-largest-size (branched from master) REVISION DETAIL https:/

D18005: Include only the largest size for each wallpaper

filipf added a comment. In D18005#387738 , @davidedmundson wrote: > > In short, images were blurry when scaled and cropped. > > I want the "In long" version. QImage resizing hasn't changed, and with the source size set that's what we'll be

D18005: Include only the largest size for each wallpaper

davidedmundson added a comment. > In short, images were blurry when scaled and cropped. I want the "In long" version. QImage resizing hasn't changed, and with the source size set that's what we'll be using. REPOSITORY R131 Plasma Wallpapers BRANCH keep-only-largest-size (branched f

D18005: Include only the largest size for each wallpaper

filipf added a comment. In D18005#387607 , @davidedmundson wrote: > What previous technical constraint are you referring to? See D18006#387372 . In short, images were blurry when scaled and crop

D18005: Include only the largest size for each wallpaper

davidedmundson added a comment. What previous technical constraint are you referring to? And we've had the multiple resolutions since 4.x so it dates back a long way. It's about a few things: - CPU usage to load an image is a sizable amount of our loading. Especially for JPEG im

D18005: Include only the largest size for each wallpaper

cfeck added a comment. Maybe symlinking all to the largest version? REPOSITORY R131 Plasma Wallpapers BRANCH keep-only-largest-size (branched from master) REVISION DETAIL https://phabricator.kde.org/D18005 To: ngraham, #vdg, #plasma, ndavis Cc: cfeck, ndavis, plasma-devel, kvanton, jr

D18005: Include only the largest size for each wallpaper

ngraham added a comment. Feel free to accept, but I really want some input from a #plasma person on this whole idea before it lands. They have been silent on the matter so far but now that there are patches, I think it's important that we get the ful

D18005: Include only the largest size for each wallpaper

ndavis added a comment. Should I accept now or should I wait until that gets sorted out? REPOSITORY R131 Plasma Wallpapers REVISION DETAIL https://phabricator.kde.org/D18005 To: ngraham, #vdg, #plasma Cc: ndavis, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai

D18005: Include only the largest size for each wallpaper

ndavis added a comment. In D18005#387324 , @ngraham wrote: > However, just removing files from the repo doesn't actually remove them from users' machines; their packaging system does that. Perhaps we should alert packagers that for the 5.15 rele

D18005: Include only the largest size for each wallpaper

ngraham added a comment. However, just removing files from the repo doesn't actually remove them from users' machines; their packaging system does that. Perhaps we should alert packagers that for the 5.15 release, they shouldn't remove these files from users' machines when they disappear fro

D18005: Include only the largest size for each wallpaper

ngraham added a comment. Just tested that out and the results aren't great: you get a black background instead of the wallpaper until you go to the wallpaper chooser and re-choose it. :/ REPOSITORY R131 Plasma Wallpapers REVISION DETAIL https://phabricator.kde.org/D18005 To: ngraham, #

D18005: Include only the largest size for each wallpaper

ngraham added a comment. Probably a question best asked in D18006 REPOSITORY R131 Plasma Wallpapers REVISION DETAIL https://phabricator.kde.org/D18005 To: ngraham, #vdg, #plasma Cc: ndavis, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBo

D18005: Include only the largest size for each wallpaper

ndavis added a comment. What happens if my wallpaper is set to the 1080p version of the default wallpaper and then that version is removed with only the highest resolution version left? REPOSITORY R131 Plasma Wallpapers REVISION DETAIL https://phabricator.kde.org/D18005 To: ngraham, #v

D18005: Include only the largest size for each wallpaper

ngraham created this revision. ngraham added reviewers: VDG, Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ngraham requested review of this revision. REVISION SUMMARY In this patch, we remove all the versions of each wallpaper that are not the largest size. He