Added option to blur background on active window
Needs ReviewPublic

Authored by niccolove on Jan 19 2020, 7:11 PM.

Details

Reviewers
None
Group Reviewers
Plasma
Summary

Create an option, off by default, that blurs the wallpaper when it detecs an open window.
Currently it broke the transition from one wallpaper to another.
(Changes planned, sorry for whitespace)

Diff Detail

Repository
R120 Plasma Workspace
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 21707
Build 21725: arc lint + arc unit
niccolove created this revision.Jan 19 2020, 7:11 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 19 2020, 7:11 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
niccolove requested review of this revision.Jan 19 2020, 7:11 PM
niccolove edited the summary of this revision. (Show Details)Jan 19 2020, 7:15 PM

What's the use case?

niccolove added a comment.EditedJan 19 2020, 7:20 PM

I've tried to ask some users, and I received generally positive feedback on the idea, as many considered it pretty. There's a plugin that does this (with many positive reviews on the store), but I think it's not a good idea to fork very similar codebases, and this patch would make it much more discoverable by default. I just think it would be a pretty feature that users might like. I love it myself. It makes the eye focus on the active window rather than the background.
Example:


That's look interesting but indeed it should be a KWin feature, say it has effect dim inactive this should be blur inactive then wallpaper and any other inactive window will be blurred.

niccolove added a comment.EditedJan 19 2020, 7:29 PM

Uhmm, I didn't think about this that way :-) but this is specifically to blur just the wallpaper, as you might want to get information from other windows. At that point, it would be weird for it to be a kwin effect, as it's much more related to just the wallpaper, and the user would expect it to be in the wallpaper settings.

There are legitimate questions regarding whether or not we should add this feature, and if this is the right place for it (a KWin effect intuitively seems more appropriate to me, if we upstream it). However aside from that, is this your code? The absence of a license header is therefore suspicious. Is this code taken straight from the plugin? If so, that's a big no-no; you need the author's permission to take their code and put it here.

Yes, I *DID* take some code from Zren plugin and wrote some other. My intention was absolutely not to steal his code, I want to be clear about that: I had opened an issue on his repo asking him to upstream the code, but then I thought that it was probably simple enough that I could figure out how it could be implemented in master based on his code, and get back to him with the patch directly to make the whole process easier for him. I did not do that yet because this patch is missing some stuff, my intention was not to already submit the patch for landing, I tried to make that clear in the summary because I couldn't find the phab button to flag it as 'not done'. I opened the diff because I had already worked on it for 4/5 hours and did not want to keep changes on local, and keeping the open diff helps me remember what I'm working on, plus I wanted to know if there were objections to implementing the feature before spending more time on it.

I now realize that submitting an half-finished task based on the zren code without much explanation gave the wrong impression, I'm sorry about this, and I hope that the explanation above clarifies it. My bad: I was really tired, in a hurry when submitting, and I had spent the whole afternoon on this (even reusing parts of zren's code, yes) so I just wanted to stop and study. Next time I'll be more careful.

This said (IF ZREN WILL GIVE PERMISSION TO USE HIS CODE, of course!) I don't like the idea of doing a kwin effect instead because:

  • Since this directly affects just the wallpaper /desktop, I think that an user would expect this to be in the wallpaper/desktop configuration page. It is just a tickbox that can go inline with the 'Get new wallpapers...' but to the left, so it uses no space and does not clutter the UI
  • Having it in the kwin effects section, along many many other effects, would bring down drastically the discoverability of the option, which is a bit of a pity since it's the kind of thing that you don't usually look for, but might enjoy once you've seen it.

Also, less important but I'm not sure at all I can manage to write a desktop effect about this, especially since (afaik) it uses js plugins instead of QML, and I'm not sure how the blur could be implemented there.

Zren added a subscriber: Zren.Jan 19 2020, 10:54 PM

I don't care if Inactive Blur is upstreamed or not as all my code is GPL. I didn't upstream it since I thought the feature was niche enough I shouldn't burden others with it's maintenance.

This doesn't blur the desktop widgets, only the wallpaper, which looks weird. I had never thought about blurring the root "desktop" window using KWin. That's an interesting idea!

mart added a subscriber: mart.Jan 20 2020, 10:04 AM

I'm not sure about this.
this looks like a nice playful addon i would expect from the KDE store, but not something in the default at all.
As usual, this actually shows the need to have a nice framework for a "base" image wallpaper that then can be expanded in many ways (this blurring thig, various download from internet etc) that can then be provided from the store.

niccolove updated this revision to Diff 74439.Jan 27 2020, 3:21 PM

Set all proper leftMargins to smallSpacing

Sorry, forgot I was on a arc feature already! Forget the last commit