[effects/coverswitch] Port to TimeLine
ClosedPublic

Authored by zzag on Sep 5 2018, 7:30 PM.

Details

Test Plan

Compiles and still works.

Diff Detail

Repository
R108 KWin
Branch
effects-coverswitch-port-to-timeline
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2553
Build 2571: arc lint + arc unit
zzag created this revision.Sep 5 2018, 7:30 PM
Restricted Application added a project: KWin. · View Herald TranscriptSep 5 2018, 7:30 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Sep 5 2018, 7:30 PM
zzag added a comment.Sep 5 2018, 7:33 PM

That's 5.15+ stuff. 5.14 already includes pretty big amount of changes.

davidedmundson added inline comments.
effects/coverswitch/coverswitch.cpp
406

one set of parentheses seems redundant

545

it'll be a bigger refactor, but would it be more in line with Timeline to do .setDirection()

(maybe afterwards)

then we can kill all the

data.setOpacity(1.0 - timeLine.currentValue());
if (stop)
    data.setOpacity(timeLine.currentValue());

everywhere.

550–554

we should reset the timeline here

If animateStart is on and animateStop is off and a user cancels halfway through the load, we become inactive straight away and won't hit the main cleanup.

744

I know you've not touched this, but

should these two lines be wrapped in

&& timeLine.value < 0.5

922–923

should we not reset the timeline here?

zzag added inline comments.Sep 5 2018, 8:47 PM
effects/coverswitch/coverswitch.cpp
406

Well, I didn't add them. Yeah, probably, that's okay to delete them.

545

Yes, it would be nice to just toggle direction. That's not really cool that effects call setCurrentTime/setElapsed(in fact, it's discouraged to call setElapsed).

550–554

Probably, we also have to reset start.

744

Probably, I also was wondering why opacity is ranging from -1 to 1.

zzag updated this revision to Diff 41089.Sep 6 2018, 8:45 AM

Address some comments

zzag marked 7 inline comments as done.Sep 6 2018, 8:46 AM
zzag updated this revision to Diff 41094.Sep 6 2018, 9:29 AM

Address the remaining inline comment.

zzag marked 2 inline comments as done.Sep 6 2018, 9:30 AM
davidedmundson accepted this revision.Sep 6 2018, 9:36 AM
This revision is now accepted and ready to land.Sep 6 2018, 9:36 AM
This revision was automatically updated to reflect the committed changes.