D15302: [effects/coverswitch] Port to TimeLine

zzag added inline comments.

INLINE COMMENTS

> davidedmundson wrote in coverswitch.cpp:406
> one set of parentheses seems redundant

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

> davidedmundson wrote in coverswitch.cpp: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.

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).

> davidedmundson wrote in coverswitch.cpp:550
> 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.

Probably, we also have to reset `start`.

> davidedmundson wrote in coverswitch.cpp:740
> I know you've not touched this, but
> 
> should these two lines be wrapped in
> 
> && timeLine.value < 0.5

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

REPOSITORY
  R108 KWin

REVISION DETAIL
  https://phabricator.kde.org/D15302

To: zzag, #kwin
Cc: davidedmundson, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart