Close screen grabbing effect when screensaver starts
ClosedPublic

Authored by davidedmundson on Apr 29 2019, 8:44 AM.

Details

Summary

The screenlock fails on X11 if it can't grab the keyboard.

We can't nicely solve the generic case. We can solve the common case of
a kwin effect being active.

It's not critical, arguably not even desirable to have these effects
persist after the screen is locked through an external trigger. We can
just close the effect early.

Key grabs have to be relased early before the close animation completes
so that the locker doesn't have a race based on animation times.

It's not ideal, but no worse than the current state for not much work.

BUG: 234153

Test Plan

locked screen on a timer
opened various effects

Diff Detail

Repository
R108 KWin
Branch
master
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 12010
Build 12028: arc lint + arc unit
davidedmundson created this revision.Apr 29 2019, 8:44 AM
Restricted Application added a project: KWin. · View Herald TranscriptApr 29 2019, 8:44 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
davidedmundson requested review of this revision.Apr 29 2019, 8:44 AM
zzag added a subscriber: zzag.Apr 29 2019, 10:36 AM
zzag added inline comments.
effects/cube/cube.cpp
105

Shouldn't the cube effect stop mouse interception as well?

effects/presentwindows/presentwindows.cpp
43

Probably unrelated whitespace change.

effects/cube/cube.cpp
105

We're closing the effect which will have stopped everything by the time the user sees the lock screen in front of them.

Keyboard has to be released early so the screenlocker can grab it, that's not true for the mouse.

Clicking and holding the mouse anywhere whilst activating the lock screen fails currently. I don't think that's something that's worth fixing.

zzag added inline comments.Apr 29 2019, 1:18 PM
effects/cube/cube.cpp
105

Well, it's more about graceful "termination."

davidedmundson added inline comments.Apr 29 2019, 1:51 PM
effects/cube/cube.cpp
105

It would be problematic if we did release early. Kwin doesn't (and can't) redirect the mouse input so we don't want anyone to get input events whilst there's a global transformation in the render.

The alternative is we have some setActive(false) that skips the close animation as well as releasing everything immediately, which would solve having to call ungrab here. Arguably better, but it makes this patch a lot more invasive.

Do we have consensus on this?

zzag added inline comments.May 14 2019, 12:48 PM
effects/cube/cube.cpp
106

It would be very helpful to have some comment that mentions when stopMouseInterception will be called (when the stop animation has completed) and why we just ungrab keyboard here.

effects/flipswitch/flipswitch.cpp
81–84

Don't need that.

effects/presentwindows/presentwindows.cpp
116–119

Don't need that.

davidedmundson marked 3 inline comments as done.

update

zzag accepted this revision.May 21 2019, 9:59 AM
zzag added inline comments.
libkwineffects/kwineffects.h
1668

Should be terminated with **/

This revision is now accepted and ready to land.May 21 2019, 9:59 AM
This revision was automatically updated to reflect the committed changes.