[effects/slidingpopups] Tweak effect to make animation smoother and more consistent
ClosedPublic

Authored by ngraham on Jan 5 2019, 10:27 PM.

Details

Summary
  • Easing function for slide-in changed to InQuad
  • Easing function for slide-out changed to OutQuad
  • Opacity is now interpolated, too.

Diff Detail

Repository
R108 KWin
Branch
arcpatch-D18000
Lint
Lint SkippedExcuse: linting seems broken
Unit
No Unit Test Coverage
Build Status
Buildable 20296
Build 20314: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
davidedmundson added inline comments.
effects/slidingpopups/slidingpopups.cpp
140

Have you tested this with the autohide panel. I doubt it will look good.

We want that for newly appearing windows in the middle of the screen, but not for a window that clearly comes in from offscreen.
The current heuristic of basing that off slideLength seems to work.

sefaeyeoglu updated this revision to Diff 48803.Jan 6 2019, 3:45 PM

Change opacity for custom slideLengths only

sefaeyeoglu marked an inline comment as done.Jan 6 2019, 3:47 PM

I just added the if statement back (but keeping the interpolate). It actually looks better now!

sefaeyeoglu edited the summary of this revision. (Show Details)Jan 6 2019, 4:33 PM
abetts added a subscriber: abetts.Jan 6 2019, 4:39 PM

Do you have a before and after gif or video?

sefaeyeoglu added a comment.EditedJan 6 2019, 4:41 PM

Do you have a before and after gif or video?

Not right now, I am not at home at the moment. I can provide a before/after gif later.

Unrelated to your comment:

effects/slidingpopups/slidingpopups.cpp
512

The comment here needs to be changed. It says InQuart, as I was experimenting with different easing functions before settling on OutExpo.

zzag requested changes to this revision.EditedJan 6 2019, 5:03 PM

I tested this patch. In general, it doesn't feel consistent with other animations, imho. I suggest to open a new task regarding easing curves before actually changing them.

This revision now requires changes to proceed.Jan 6 2019, 5:03 PM
sefaeyeoglu added a comment.EditedJan 6 2019, 5:14 PM
In D18000#387618, @zzag wrote:

I tested this patch. In general, it doesn't feel consistent with other animations. I suggest to open a new task regarding easing curves before actually changing them.

It is true, that it may be inconsistent with other animations, as it a quicker (longer distance in the same time) animation in comparison.
I was looking around in the Human Interface Guidelines and didn't find a section about motion. We could create a meta task, which covers motion in Plasma in general. In my opinion most animations in Plasma feel a bit old (I am probably used to faster animations mainly from the Material Design Guidelines).

I looked into this here, as this was the first animation I wanted to change.

I really like most animations in Android (Pie) as they feel modern and polished. I may experiment with AOSP-like animations in the future.

Do you have a before and after gif or video?

Here you go. It is only 30fps and very small.

The first two are with the old animation. the last three are with the new animation. As I am running on the proprietary nvidia driver it may look choppy, but it runs smooth on my laptop with Intel graphics.

ngraham added a subscriber: ngraham.Jan 8 2019, 1:31 AM

Looks better or at least fine to me. No visual objection.

I don't really think that it looks that different than other animations. If one gets used to it it isn't that strange anymore.

zzag added a comment.EditedJan 9 2019, 11:55 AM

The main problem I see is that it's too rapid, imho. It's not very pleasant to watch how some entity moves too rapidly.

I'd like to see some analysis of what's wrong with the current choice of easing curves, how it can be fixed, and why we picked this easing curve instead of that, so we have a "bigger picture" of what should be done.

I can only speak from experience. I don't know if there are any scientific studies that look at user experience with a focus of animation.

So this is my own opinion:

Animations should be quick and clear. An object should animate in a clearly comprehensible way. That means that an object should not appear out of nowhere. It should rather come from a place the user can't see. As we have four screen borders we could just let the object translate from a screen edge, similar to how one would open a drawer in the real world. Of course this is similar to the Material Design Guidelines as this is about comparing design to the real world. Now to this example:
Let's say we have an application launcher menu in the bottom left of the screen. When I click it I expect it to come out of the lower screen edge on the left. Now thinking in the real world: When I open a drawer it does not fade in and only travel a specific short distance. I can see it open from 0% open to 100% open without any jump. Therefore the application launcher should act like a real drawer. Now let's talk about the timing of the animation. I don't open a drawer in just 150ms like in the slidingpopups effect. These 150ms may be tweaked to a bit longer, while staying snappy. There is a range of time where an animation has the perfect length. But that depends on the distance and easing and should be further investigated.

TL;DR

  • Animations should resemble their real world counterpaths (e.g. Sliding Popups -> Opening a drawer in the real world)
  • Timing should be investigated
  • Timing depends on distance and easing.

Compomise between old behaviour and my proposal. Now uses fixed slideLength again, but with different easing.

I recently set up a proper working environment again and started tinkering with different easings and slide-lengths. I decided to make a compromise between the old animation and the one I proposed before.

zzag added a comment.Sep 15 2019, 9:38 PM

So, I've been running your patch for a while and have to admit that disappearing animation looks better. However, I advise you and VDG to analyze (perhaps update hig as well?) our choice of easing curves in default effects, e.g. morphing popups, sliding popups, fade, etc, and based on that pick better curves.

effects/slidingpopups/slidingpopups.cpp
140–146

Unrelated change.

Tidy up code

Fix syntax error

sefaeyeoglu retitled this revision from Modernize easing function of slidingpopups effect. to Tweak slidingpopups effect to make animation smoother.Sep 15 2019, 9:57 PM
sefaeyeoglu edited the summary of this revision. (Show Details)

Fix wrong opacity condition for vertical popups

+1, should we land this while HIG animation page and easing curves are being discussed?

@sefaeyeoglu thanks for this patch. We have a set of emerging guidelines for animations in D25343. Can you address the inline comments so we can push this forward?

effects/slidingpopups/slidingpopups.cpp
473

Let's make this InQuad to be consistent with most other animations in Plasma, and conform to the emerging guidelines in D25343.

@sefaeyeoglu would yo like to continue working on this? Or should someone take it over?

Couldn't find time for this either. I will probably start working on this again, when my development environment is set up again.

ngraham added a comment.EditedDec 6 2019, 2:01 PM

Cool, let me know if you need a hand with anything, or would like like someone else to take over.

Would you like me to take over this if you're lacking the time?

Would you like me to take over this if you're lacking the time?

Whoops. Looks like I forgot about this. I guess there are people here with more time to work on this than me. Thanks for the reminder, though! I would be happy if you or someone else takes over this.

ngraham commandeered this revision.Dec 26 2019, 11:44 PM
ngraham added a reviewer: sefaeyeoglu.

Thanks!

ngraham updated this revision to Diff 72224.Dec 27 2019, 3:16 AM
  • Rebase
  • Use In/OutQuad
ngraham updated this revision to Diff 72225.Dec 27 2019, 3:18 AM
ngraham marked 2 inline comments as done.

Reduce diff

ngraham updated this revision to Diff 72226.Dec 27 2019, 3:19 AM
ngraham retitled this revision from Tweak slidingpopups effect to make animation smoother to Tweak slidingpopups effect to make animation smoother and more consistent.
ngraham edited the summary of this revision. (Show Details)

Update description

Ready for re-review.

zzag added inline comments.Dec 30 2019, 11:49 AM
effects/slidingpopups/slidingpopups.cpp
141

data.multiplyOpacity(t) and data.multiplyOpacity(interpolate(0.0, 1.0, t)); do the same thing. Could you please revert this change?

ngraham updated this revision to Diff 72406.Dec 30 2019, 5:57 PM

Remove unnecessary changes

ngraham marked an inline comment as done.Dec 30 2019, 5:57 PM
ndavis accepted this revision.Dec 30 2019, 10:12 PM

Visually, LGTM

sefaeyeoglu requested changes to this revision.Jan 3 2020, 3:48 PM

WAIT. The slide-in animation is choppy, as we are using InQuad. This means it starts smooth and ends linear. IMO it should be the other way around. Start linear, end smooth.

This revision now requires changes to proceed.Jan 3 2020, 3:48 PM

Video of what I mean

But InQuad for animating from invisible to visible is what we do in other places and it's what we're planning to formalize in D25343. Shouldn't we be consistent?

But InQuad for animating from invisible to visible is what we do in other places and it's what we're planning to formalize in D25343. Shouldn't we be consistent?

I guess we should discuss this there then. Most UI Kits I know of always decelerate a appear animation.

I commented on D25343 about this. Also note the following inline comment. If we decide on (really) "OutQuad" for slide-in and "InQuad" (or "OutQuad" in code), we just need to adjust L467. Maybe we should also add a comment at L506 that the easing curve will be reversed.

effects/slidingpopups/slidingpopups.cpp
512

Technically the "OutQuad" here turns into "InQuad". In L504 the animation is reversed, which also reverses the easing curve.

ngraham updated this revision to Diff 72910.Jan 6 2020, 6:04 PM

Use OutQuad for the appear animation

ngraham marked an inline comment as done.Jan 6 2020, 6:04 PM

The HIG patch landed: D25343. Folks who have requested changes, if you're good with this, can you change your status to Accepted now?

davidedmundson accepted this revision.Jan 7 2020, 4:08 PM
sefaeyeoglu accepted this revision.Jan 7 2020, 4:28 PM

Happy that this will land pretty soon!

zzag accepted this revision.Jan 7 2020, 4:37 PM
This revision is now accepted and ready to land.Jan 7 2020, 4:37 PM
zzag added a comment.Jan 7 2020, 4:38 PM

Oh, don't forget to add "[effects/slidingpopups]" to the subject line.

ngraham retitled this revision from Tweak slidingpopups effect to make animation smoother and more consistent to [effects/slidingpopups] Tweak effect to make animation smoother and more consistent.Jan 7 2020, 4:38 PM
This revision was automatically updated to reflect the committed changes.