[scripting] Introduce redirect function
ClosedPublic

Authored by zzag on Oct 27 2018, 9:15 AM.

Details

Summary

Consider current implementation of the Squash effect: if a window was
minimized, an animation will be started; if the window is unminimized
and the animation is still active (that can happen when user clicks on
app's icon really fast), the animation will be stopped and a new one will
be created. Such behavior can lead to rapid jumps in the observed
"animation".

A better approach would be first try to reverse the already active
animation, and if that attempt wasn't successful, start a new animation.

This patch introduces a new function to the scripted effects API that
lets JavaScript effects to control direction of animations. The
prototype of the function looks as follows:

redirect(<animation id(s)>, <direction>, [<termination policy>])

the first argument is an animation id or a list of animation ids, the
second argument specifies the new direction of the animation or
animations if a list of ids was passed as the first argument. The
third argument specifies whether the animation(s) should be terminated
when it(they) reaches the source position, currently it's relevant only
for animations that are created with set() function. The termination
policy argument is optional, by default it's Effect.TerminateAtSource.

We can use this function to fix issues with rapid jumps in the Squash
effect. Also, redirect() lets us to write effects for simple animations
in slightly different style: first, we have to start the main animation
(e.g. for the Dialog Parent effect, it would be dimming of main windows)
and then change direction of the animation depending on external events,
e.g. when the Desktop Cube effect is activated.

Diff Detail

Repository
R108 KWin
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4316
Build 4334: arc lint + arc unit
zzag created this revision.Oct 27 2018, 9:15 AM
Restricted Application added a project: KWin. · View Herald TranscriptOct 27 2018, 9:15 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Oct 27 2018, 9:15 AM
zzag updated this revision to Diff 44428.Oct 29 2018, 11:08 AM

Fix merge conflicts.

abetts added a subscriber: abetts.Oct 29 2018, 2:12 PM

Would there be any visual change because of this new behavior?

zzag updated this revision to Diff 44842.Nov 4 2018, 1:27 PM

Rebase.

Generally, ship it!

libkwineffects/anidata_p.h
101

Would it be feasible to have matching names and types for keepAtTarget and keepAtSource. They're effectively doing the same thing

libkwineffects/kwinanimationeffect.cpp
346

It's better to not specify a default for a finite list.

This way you get a compile warning if someone added a 3rd direction.

zzag added inline comments.Nov 16 2018, 11:54 AM
libkwineffects/anidata_p.h
101

So, instead of having two booleans, we would have a single enum, right?
Yeah, it makes sense, but this would be unrelated to this change.

libkwineffects/kwinanimationeffect.cpp
346

I did this deliberately because one can go either forward or backward, but I don't insist on this one. Should I delete this part of code?

libkwineffects/anidata_p.h
101

Instead of

bool keepAtTarget;
AnimationEffect::TerminationPolicy terminationPolicy;

We have either:

AnimationEffect::TerminationPolicy targetTerminationPolicy;
AnimationEffect::TerminationPolicy sourceTerminationPolicy;

OR

AnimationEffect::TerminationPolicy terminationPolicy;

which has flags for {DontTerminate, TerminateAtSource, TerminateAtTarget}

That then becomes related to this patch as we'd have to name things appropriately

zzag added inline comments.Nov 16 2018, 12:37 PM
libkwineffects/anidata_p.h
101

which has flags for {DontTerminate, TerminateAtSource, TerminateAtTarget}

With TerminateAtTarget, effects can then change type of animation (animate vs set).

AnimationEffect::TerminationPolicy targetTerminationPolicy;
AnimationEffect::TerminationPolicy sourceTerminationPolicy;

If we have the following TerminationPolicy

enum TerminationPolicy {
    DontTerminate,
    Terminate
};

maybe it can work out, but I doubt whether it would be simpler than just using two booleans (bool terminateAtSource and bool terminateAtTarget).

zzag added a comment.Nov 16 2018, 12:43 PM

As an alternative, we could change the name of TerminationPolicy enum, so it's more concrete, e.g. RedirectTerminationPolicy.

zzag planned changes to this revision.Nov 16 2018, 2:33 PM
zzag updated this revision to Diff 45647.Nov 17 2018, 11:40 AM
  • Unify keepAtTarget and terminationPolicy
  • Delete the default case
davidedmundson accepted this revision.Nov 17 2018, 11:40 AM
This revision is now accepted and ready to land.Nov 17 2018, 11:40 AM
This revision was automatically updated to reflect the committed changes.