[effects] Add Scale effect
ClosedPublic

Authored by zzag on Jun 10 2018, 11:05 AM.

Details

Summary

The new effect scales windows as they appear and disappear.

As the the most of window animation effects, it is a monolithic effect,
i.e., if you enable scale effect, it will animate *both* the appearing and
disappearing.

The main difference between the Scale effect and the Scale in effect is
that the Scale in effect only animates windows as they appear. There is
no corresponding "the Scale out" effect, which is odd. Other points that
differentiate the Scale effect from the Scale in effect:

  • it is more subtle;
  • it doesn't animate the log out screen;
  • it doesn't conflict with the Fade effect, etc.

... and overall, the Scale effect supersedes the Scale in effect.

Demos

Window open animation.

Window close animation.

KCM.

Test Plan
  • Enabled this effect
  • Opened/closed System Settings

Diff Detail

Repository
R108 KWin
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 1292
Build 1309: arc lint + arc unit
zzag created this revision.Jun 10 2018, 11:05 AM
Restricted Application added a project: KWin. · View Herald TranscriptJun 10 2018, 11:05 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Jun 10 2018, 11:05 AM
zzag edited the summary of this revision. (Show Details)Jun 10 2018, 11:05 AM
zzag edited the summary of this revision. (Show Details)Jun 10 2018, 11:32 AM
fvogt added a subscriber: fvogt.Jun 10 2018, 11:52 AM

IMO it would be more user friendly to have the values in the configuration dialog displayed as inverted percentage values. That way 0% means disabled and 100% is the maximum.

Replacing ScaleIn with an effect that does both in and out seems very sensible.
Having a blacklist seems very sensible (I don't like that it's hardcoded anywhere, but that already exists so it's a separate topic)

However, scale in was acheived as a scripted effect whereas this isn't. It's a step backwards from our general trend, is there a reason for that?

Could we re-organize the preferences windows to avoid group boxes with single elements? Let's try to mimic the style used in Plasma and new System Settings KCMs that has labels on the left:

          Duration: ______
 Window open scale: _______
Window close scale: _______
zzag added a comment.EditedJun 10 2018, 1:07 PM

Replacing ScaleIn with an effect that does both in and out seems very sensible.
Having a blacklist seems very sensible (I don't like that it's hardcoded anywhere, but that already exists so it's a separate topic)

However, scale in was acheived as a scripted effect whereas this isn't. It's a step backwards from our general trend, is there a reason for that?

Missing API.

There are missing things in scripted effect API*:

  • checking/setting fullscreen effect**
  • grabbing windows

which are crucial to implement this effect.

I tried to implement them but in the end of day all what I had was "Hmm, is this a right way to do this? Isn't it messed up? Does it fit JS?". So, on the next day, I've implemented grabbing of windows (with "f*ck it" in a mind) D13153, which I still don't like (there are issues with emitting of windowDataChanged signal, the whole implementation is not quite intuitive because it relies on isGrabbed, shouldn't animate check grab key, etc). Same with fullscreen effects.

I just don't want to waste yet another day(and get nothing in the end) trying to figure out good APIs for scripted effects so the effect that I want to implement works correctly. If someone implement those APIs, I will rewrite this effect in JavaScript. I hope you'll understand my motives to write this effect in C++.

* that's not the whole list
** this would also fix a bug (bug 321201)

zzag added a comment.EditedJun 10 2018, 1:42 PM

IMO it would be more user friendly to have the values in the configuration dialog displayed as inverted percentage values. That way 0% means disabled and 100% is the maximum.

Well, right now it's possible to have scale > 1. How could we achieve something like that with inverted percentage values?

zzag updated this revision to Diff 35954.Jun 10 2018, 3:24 PM

Don't use QGroupBox

zzag edited the summary of this revision. (Show Details)Jun 10 2018, 3:47 PM
zzag updated this revision to Diff 36085.EditedJun 13 2018, 9:20 AM

Don't animate Audio Volume Feedback and other windows that are not expected to be animated.

(yet, if the Sliding popups effect is disabled, Kickoff, KRunner, and other
slide windows will be animated)

zzag updated this revision to Diff 36147.Jun 14 2018, 11:58 AM

Gracefully release window if WindowClosedGrabRole role has been changed

zzag edited the summary of this revision. (Show Details)Jun 15 2018, 9:20 PM

This is really nice-looking!

zzag updated this revision to Diff 36605.Jun 24 2018, 3:06 PM

Drop the IsScaleWindowRole hack, don't need it anymore.

zzag planned changes to this revision.Jun 28 2018, 1:57 PM

Port to TimeLine.

zzag updated this revision to Diff 36927.Jun 30 2018, 7:18 AM

Port to TimeLine.

zzag updated this revision to Diff 36928.Jun 30 2018, 7:19 AM

Wrong commit.

zzag updated this revision to Diff 37066.Jul 2 2018, 1:47 PM
  • support information
  • contrain opacity values
zzag updated this revision to Diff 37146.Jul 4 2018, 2:06 PM

Delete redundant check

ngraham accepted this revision as: VDG.Jul 16 2018, 6:40 PM

+1 visually and conceptually (I like the idea of a single Scale effect that animates both opening and closing). KCM looks fine. No comment on the code since this isn't my area of expertise.

zzag added a comment.Jul 23 2018, 7:55 AM

Weekly ping.

Could we get Plasma or KWin review for this?

zzag updated this revision to Diff 38644.Jul 28 2018, 8:09 AM
  • Don't animate the Application Dashboard
  • Don't do full repaints
zzag updated this revision to Diff 38646.Jul 28 2018, 8:34 AM

Force background contrast and blur

zzag updated this revision to Diff 39117.EditedAug 5 2018, 10:01 AM
  • Comment on blacklisted windows
  • Edit description of the effect
davidedmundson added inline comments.Aug 7 2018, 11:49 AM
effects/scale/scale.cpp
175

If login/logout effects set the windowAdded grab role could we drop the blacklist?

261

Which plasmawindows do we want to avoid animating that don't have decorations and fall under:

w->isNormalWindow()

|| w->isDialog();

?

zzag added inline comments.Aug 7 2018, 12:19 PM
effects/scale/scale.cpp
175

No.

What if those effects are disabled?

261

Application Dashboard.

Also, if you disable the Sliding popups effect, the Application Launcher, calendar and so on.

davidedmundson accepted this revision.Aug 11 2018, 10:16 PM
davidedmundson added inline comments.
effects/scale/scale.cpp
261

Ok, I don't really like this given we control plasma, we shouldn't need be hardcoding blacklists and having heuristics.

But the login windows are already blacklisted in other effects. That can be a task for another day.

This revision is now accepted and ready to land.Aug 11 2018, 10:16 PM
zzag added inline comments.Aug 12 2018, 7:06 AM
effects/scale/scale.cpp
261

As I said in the glide effect diff, I also don't like it.

For now, that's somewhat ok solution, but I agree, we need to fix it.

This revision was automatically updated to reflect the committed changes.