[kwinrules] Added option to enable and disable blur
Needs RevisionPublic

Authored by mkulinski on Jul 10 2018, 12:47 PM.

Details

Reviewers
davidedmundson
Group Reviewers
KWin
Summary

This add blur to Rules and RulesWidget

Test Plan

I don't know if including KWindowEffects in rules.c is a good idea, maybe adding it to AbstractClient is better?

Video:

Diff Detail

Repository
R108 KWin
Lint
Lint Skipped
Unit
Unit Tests Skipped
mkulinski created this revision.Jul 10 2018, 12:47 PM
mkulinski created this object with visibility "All Users".
Restricted Application added a subscriber: kwin. · View Herald TranscriptJul 10 2018, 12:47 PM
mkulinski requested review of this revision.Jul 10 2018, 12:47 PM
cfeck added a reviewer: zzag.Jul 10 2018, 12:54 PM
cfeck added a subscriber: cfeck.

The Summary is the text committed to the change history, so please do not ask questions there, but describe your change shortly.

mkulinski updated this revision to Diff 37507.Jul 10 2018, 12:57 PM

Fixed formatting.

mkulinski edited the summary of this revision. (Show Details)Jul 10 2018, 1:04 PM
mkulinski edited the test plan for this revision. (Show Details)
zzag removed a reviewer: zzag.Jul 10 2018, 2:14 PM
zzag added a subscriber: graesslin.

Using KWindowEffects inside of KWin is wrong. Also, it would work only on X11. I think it would be better to add blurBehind(or something like that) property to toplevel and use it later in the blur effect.

Anyway, I suggest to wait for @graesslin.

zzag changed the visibility from "All Users" to "Public (No Login Required)".Jul 10 2018, 2:15 PM

Adding support for rule in effects is not trivial. We have had this idea for years and never found a suitable and implementable solution. Hacking this in for one effect is not a solution. We need a general solution. And that is not trivial as many areas in KWin are lacking:

  • effects need to define sensible rules
  • rules ui must be able to load the rules for effects
  • a way to notify from rules to effects

Using KWindowEffects inside KWin is an absolute no-go. This is an API intended to be used to inform KWin not to be used by KWin. The behavior is pretty much undefined.

@graesslin, after looking for couple of hours on kwin, kde-workspace and some others repositories i come with idea:

  • KWin::Rules will use list of effects(maybe special X-KDE-ServiceTypes) to generate map/vector of all available rules
  • every Rule will provide/implement:
    • "unique" identifier, for example: blur; that is going to be used to identify it in KCM and KWin::Rules
    • KCM(just the row or whole tab bar) for kwinrules.
    • special interface which is going to be used by KWin::Rules with virtual methods like: void read(KConfigGroup&), void write(KConfigGroup&), void apply(Rules*), apply(Client*), void discard(Rules*), etc

The only thing that's left is:

  • a way to notify from rules to effects

but i will come with idea how to solve that after implementing the things described above.

Please let me know if you have a question or problem with that idea.

PS. I think that is not the best place to discuss about that.

I would turn it around: first think about how to pass the information to the effects. Without that it doesn't make much sense to define which rules exists. Especially have a look how KWin core uses the rules. The common pattern is: something changes, then check in rules the value and use that as the true new value. That pattern is btw. not used in the code of this patch. But for having the rules work correctly and in all circumstances this is extremely important.

So in case of blur it would be that the blur effect needs to check the rules whenever a window gets added or a window changes in a way that the blur behind needs to be re-evaluated. If this setup works or we have a plan how it works then we can look into how to define it. Otherwise we might end up with a system which is not suited for rules. That said: I wouldn't know whether your suggestion would work as I don't know what's really needed to make effects support rules.

davidedmundson requested changes to this revision.Sep 28 2018, 11:32 AM
davidedmundson added a subscriber: davidedmundson.

Marking as request changes as per MF's comments

This revision now requires changes to proceed.Sep 28 2018, 11:32 AM