Wallpaper: hide color or blur filling options for full filling mode
ClosedPublic

Authored by guoyunhe on Oct 24 2017, 9:32 AM.

Details

Summary

Related to patch https://phabricator.kde.org/D7047 . Pointed out by @ngraham

Make a new option called "Edge filling". Users can choose either "Blur image" or "Solid color". They are only visible when the image filling mode is "Center" or "Scale and keep ratio".

When it is "Center":

When it is "Tile":

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
guoyunhe created this revision.Oct 24 2017, 9:32 AM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 24 2017, 9:32 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidedmundson added inline comments.
wallpapers/image/imagepackage/contents/ui/config.qml
161

This used to be the case, then it was changed for some obscure use case with transparaent images.

guoyunhe edited the summary of this revision. (Show Details)Oct 24 2017, 9:37 AM
guoyunhe added a reviewer: ngraham.
guoyunhe added a subscriber: ngraham.
guoyunhe added inline comments.Oct 24 2017, 9:48 AM
wallpapers/image/imagepackage/contents/ui/config.qml
161

Ah, I have never thought about this. Is this use case common? I have never seen people use transparent image for wallpaper. Usually it is some photos or JPG images downloaded from internet.

This brings another problem. Blur image edge filling doesn't work well with transparent images. For transparent image you always need a background color. I am lost...

If we say, we only support opaque image, the solution is much simpler.

ngraham added inline comments.Oct 24 2017, 1:02 PM
wallpapers/image/imagepackage/contents/ui/config.qml
161

I suppose we could also show the radio buttons if we detect that the image has transparency. But yeah, it would be nice if we can only support opaque images. It also hadn't occurred to me that anyone would use an image with transparency, and I don't know if I've ever seen such a thing in the wild. But of course users can be surprising! :)

Can we change "Edge filling" to "Background"?

Can we change "Edge filling" to "Background"?

I don't know which word is better because I am not a native English speaker. But yeah, I can change it.

guoyunhe updated this revision to Diff 21239.Oct 24 2017, 1:43 PM
  • Wallpaper: change label name
guoyunhe updated this revision to Diff 21240.Oct 24 2017, 1:45 PM
  • Wallpaper: change variable name from edgefilling to background

Here's how I would do it:

Background: * Blur
            * Solid color: [color chooser]
guoyunhe updated this revision to Diff 21241.Oct 24 2017, 1:53 PM
  • Wallpaper: change label text of blur option

Done.

Here's how I would do it:

Background: * Blur
            * Solid color: [color chooser]

Great, thanks! Much better. Any chance you can upload new screenshots?

Great, thanks! Much better. Any chance you can upload new screenshots?

Big +1 from me, if we can resolve the potential issue with images that have transparency.

broulik added a subscriber: broulik.Nov 4 2017, 1:25 AM

You need to adjust the config.xml for that setting to persist.

Also, it seems you have pushed an unfinished version of that feature [1] without approval.

[1] https://cgit.kde.org/plasma-workspace.git/commit/?id=9923cde1024dc86ab14c200bf745dde308fc9d3f

ngraham edited the summary of this revision. (Show Details)Nov 4 2017, 8:53 PM

You need to adjust the config.xml for that setting to persist.

Also, it seems you have pushed an unfinished version of that feature [1] without approval.

[1] https://cgit.kde.org/plasma-workspace.git/commit/?id=9923cde1024dc86ab14c200bf745dde308fc9d3f

It was approved in another Diff https://phabricator.kde.org/D7047 . But I forgot to use arc land to commit...

Heh I may be the one who used transparent images - the idea being that you could, in theory set colour of an entire wallpaper based on icon colour using the correct kind of transparent image (essentially making darker parts less transparent meaning the lighter the area the brighter the underlying colour, creating a monochromatic image). To be quite frank as an idea it demanded slightly more work than was feasible, so I support this.

A neat idea, but yeah, I think it probably wouldn't even occur to most people to try to use a transparent image! If you approve, would you mind formally accepting it? I'm willing to triage and handle any bugs that come in saying "Hey, you broke my transparent wallpaper!"

jensreuterberg accepted this revision.Nov 12 2017, 10:09 AM

Sure thing - I worry that there may be some technical difficulties even though I, with my limited technical skillset, can't foresee any. (Waiting nervously for a dev to kick down my door and kill me :) )

This revision is now accepted and ready to land.Nov 12 2017, 10:09 AM
ngraham accepted this revision.Nov 12 2017, 2:32 PM

I'm inclined to accept this since it handles the common case better than what we have right now, and the uncommon case of transparent images is going to be a nightmare no matter what.

This revision was automatically updated to reflect the committed changes.

From what I can tell the wallpaper itself doesn't respect this so we still end up loading the blurry image on the wallpaper, no?