[effects/dimscreen] Port to JavaScript
ClosedPublic

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

Details

Summary

The ported effect looks quite similar to the C++ version except one
thing: it works correctly when user activates/deactivates a full
screen effect, for example the Desktop Cube effect.

Other than that, there are no behavioral or visual differences.

Diff Detail

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

Use complete() instead of rewind().

zzag added inline comments.Nov 4 2018, 1:03 PM
effects/dimscreen/package/contents/code/main.js
61

That's a typo, it should be window.popupWindow.

zzag updated this revision to Diff 44838.Nov 4 2018, 1:04 PM

Fix typo.

zzag updated this revision to Diff 44846.Nov 4 2018, 1:29 PM

Move use strict to the beginning of the script

davidedmundson added inline comments.
effects/dimscreen/package/contents/code/main.js
50

This used to be:

data.multiplyBrightness((1.0 - 0.33 * timeline.currentValue()));

so this should be ~0.67?

68

I'd rather we cleaned the animation ID when it ends rather than leaving dangling IDs everywhere and having to work round it - but I won't insist on it if you disagree.

zzag added inline comments.Nov 9 2018, 5:53 PM
effects/dimscreen/package/contents/code/main.js
50

Oh my God, this is so embarrassing. Yes, it should be 0.67.

68

... cleaned the animation ID when it ends rather than leaving dangling IDs

Makes sense.

zzag added inline comments.Nov 10 2018, 11:33 AM
effects/dimscreen/package/contents/code/main.js
68

Well, currently, we need to do more work to implement it: set() doesn't return a JavaScript Array object(so, we can't use indexOf, for example), and animationEnded doesn't pass actual animation id (it's always 0).

So, I'd like to postpone the cleanup of dangling animation ids.


Personally, I'd prefer to pass a closure to animate() and set() to get notified when the animation is completed, so the cleanup is more simpler, but I don't think we'll be able to implement something like this any time soon.

zzag updated this revision to Diff 45226.Nov 10 2018, 11:33 AM

Fix bad math.

davidedmundson accepted this revision.Nov 10 2018, 11:57 AM

Personally, I'd prefer to pass a closure to animate() and set()

Fully agree.

There's a few options for that, some can be backwards compatible and simple.

This revision is now accepted and ready to land.Nov 10 2018, 11:57 AM
This revision was automatically updated to reflect the committed changes.