[effects] Split the Fade effect
ClosedPublic

Authored by zzag on Nov 12 2018, 10:08 AM.

Details

Summary

Currently, we have three effects that can be used to animate the
appearing of toplevel windows(fade, glide, scale) and one can enable
all three of them, which seems to be wrong. It doesn't make sense to have
glide and scale effect enabled, for example.

We couldn't put all three effects into an exclusive group before because
the fade effect animates not only toplevel windows but also popups. So,
if all three effects are in an exclusive group and you enable glide effect,
for example, then tooltips and other popups won't be faded in/out.

This patch splits the fade effect into two: the first effect (called Fade)
animates toplevel windows and the other one (called Fading Popups) animates
popup windows.

Test Plan

Have been using the Fading Popups effect in combination with the Scale
effect for a couple of days. Haven't noticed any significant differences between
the new combination (Fading Popups + Scale) and the old combination
(Fade + Scale).

Diff Detail

Repository
R108 KWin
Branch
split-fade-effect
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4862
Build 4880: arc lint + arc unit
zzag created this revision.Nov 12 2018, 10:08 AM
Restricted Application added a project: KWin. · View Herald TranscriptNov 12 2018, 10:08 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Nov 12 2018, 10:08 AM
zzag edited the test plan for this revision. (Show Details)Nov 12 2018, 10:15 AM
zzag edited the summary of this revision. (Show Details)Nov 12 2018, 10:24 AM
zzag edited the test plan for this revision. (Show Details)Nov 12 2018, 10:29 AM

Very cool, +1 for putting these effects into an exclusive group.

Have we given any thought to how to better present the exclusive groups in the KCM? Right now it's not as clear as it could be.

abetts added a subscriber: abetts.Nov 12 2018, 3:02 PM

Maybe another group could be "Transitions", or "Appear"

zzag added a reviewer: VDG.Nov 12 2018, 3:31 PM

Have we given any thought to how to better present the exclusive groups in the KCM? Right now it's not as clear as it could be.

As far as I know, no. I'm open for suggestions.

zzag added a comment.Nov 12 2018, 6:56 PM

I was thinking about creating a whole new category for open/close animations.

Very cool, +1 for putting these effects into an exclusive group.

Have we given any thought to how to better present the exclusive groups in the KCM? Right now it's not as clear as it could be.

As I spent quite some time on that: it's difficult to present this in a good way and I implemented the best solution I found. If we want to present it better we might have to completely rethink the effects kcm, but we also did that several times. The big problem is that it's exclusive group which could also contain none. That's from a UI perspective really difficult.

graesslin accepted this revision.Nov 13 2018, 8:37 AM

Reasoning sounds sensible, code looks good to me

This revision is now accepted and ready to land.Nov 13 2018, 8:37 AM

Very cool, +1 for putting these effects into an exclusive group.

Have we given any thought to how to better present the exclusive groups in the KCM? Right now it's not as clear as it could be.

As I spent quite some time on that: it's difficult to present this in a good way and I implemented the best solution I found. If we want to present it better we might have to completely rethink the effects kcm, but we also did that several times. The big problem is that it's exclusive group which could also contain none. That's from a UI perspective really difficult.

Adding a simple "None" entry for exclusive groups that need one seems like it could work. Also, something as simple as more whitespace before the exclusive group's header and after the last entry might help. Right now the exclusive groups kind of blend into the other non-exclusive entries.

This revision was automatically updated to reflect the committed changes.