Delayed focus updates for Toplevels without surface
AcceptedPublic

Authored by romangg on Feb 23 2019, 6:36 PM.

Details

Reviewers
zzag
Group Reviewers
KWin
Summary

XWayland clients have a surface associated with them asynchronously. In this
case we don't directly want to set focus on this Toplevel, but wait until the
surface is set.

This patch aims in conjunction with an unrelated fix to SDL at improving Steam
Big Picture Mode in our Wayland session.

Test Plan

Steam BPM regains focus on game close.

Diff Detail

Repository
R108 KWin
Branch
bpmFocusFix
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 11761
Build 11779: arc lint + arc unit
romangg created this revision.Feb 23 2019, 6:36 PM
Restricted Application added a project: KWin. · View Herald TranscriptFeb 23 2019, 6:36 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
romangg requested review of this revision.Feb 23 2019, 6:36 PM
zzag added a subscriber: zzag.Apr 9 2019, 5:57 AM
zzag added inline comments.
input.h
456

Hmm, it seems like it isn't set to an appropriate value anywhere.

In this case we don't directly want to set focus on this Toplevel, but wait until the surface is set.

Can you expand on why?

anthonyfieroni added inline comments.
input.cpp
2270

It should be m_at.surfaceCreatedConnection = ...

In this case we don't directly want to set focus on this Toplevel, but wait until the surface is set.

Can you expand on why?

XWayland surfaces are created asynchronously to the respective Toplevels. And if the Toplevel exists, but no surface yet the focus update will fail.

input.cpp
2270

yes. thanks.

romangg updated this revision to Diff 57928.Sun, May 12, 1:00 PM
romangg marked 2 inline comments as done.
  • Fix surfaceCreatedConnection being set
zzag accepted this revision.Mon, May 13, 9:20 AM

Naming nitpick: I suggest to rename m_at.at to m_at.toplevel in order to avoid repetition.

input.cpp
2268

Can you leave a comment that explains why we do this?

input.h
456

Given that you connect to surfaceChanged, perhaps it should be surfaceChangedConnection.

This revision is now accepted and ready to land.Mon, May 13, 9:20 AM