Differential D26233

Clamp XdgShellClient::clientSize to surface size, not m_windowGeometry
ClosedPublic
Actions

Authored by davidedmundson on Dec 26 2019, 12:00 PM.

Details

Reviewers
zzag
Group Reviewers
KWin
Commits
R108:89024e2bcc1b: Clamp XdgShellClient::clientSize to surface size, not m_windowGeometry
Summary

It's perfectly legitimate to call setWindowSize before a buffer is
attached. This seems to have happen with plasma surfaces that commit
when attaching a shadow, but technically could happen anywhere.

By clamping to the applied surface here, we get the wrong window size
cached and not re-evaluated when a surface is eventually applied. This
leaves us thinking the windowsize is empty but with a massive margin
which actually holds the content.

We want all internal usages of xdgshellclient to use the window geometry
set. Only the wider kwin part needs to care about clamping it to the
surface.

This fixes popup placement in the plasma panel
BUG: 415317

As well as ghost notification popups with no background contrast that
you can't interact with.

Test Plan

Ran kwin

Diff Detail

Repository
R108 KWin
Branch
master
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 20282
Build 20300: arc lint + arc unit
Restricted Application added a project: KWin. · View Herald TranscriptDec 26 2019, 12:00 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
davidedmundson requested review of this revision.Dec 26 2019, 12:00 PM
romangg added a subscriber: romangg.Dec 26 2019, 12:14 PM

Good find. Can subSurfaceTreeRect be recomputed in a less common call, not in the clientSize getter? For example when attached buffer size changes.

zzag added a comment.Dec 30 2019, 11:45 AM

Urgh, the xdg-shell spec is ambiguous about the effective window geometry. Some compositors clamp the window geometry at commit time, some compositors clamp at query time (like this patch does), and some don't clamp at all. :/

For example when attached buffer size changes.

We can't connect to SurfaceInterface::sizeChanged since this signal is emitted only for the main surface if I recall correctly. I guess we could compute the bounding rect in the commit handler.

romangg added a comment.EditedDec 30 2019, 1:34 PM

Shall we go with this patch here for now as a quick fix and look into optimizing the number of calculations later?

@zzag: If you agree, accept the patch.

davidedmundson added a comment.Dec 30 2019, 1:59 PM

Good find. Can subSurfaceTreeRect be recomputed in a less common call, not in the clientSize getter? For example when attached buffer size changes.

It can, means storing yet another geometry as it it isn't confusing enough :)

Most clients only have one surface so I think it's a non-issue but I'm happy to change to whatever.

zzag accepted this revision.Jan 6 2020, 10:20 AM
This revision is now accepted and ready to land.Jan 6 2020, 10:20 AM
Closed by commit R108:89024e2bcc1b: Clamp XdgShellClient::clientSize to surface size, not m_windowGeometry (authored by davidedmundson). · Explain WhyJan 6 2020, 3:17 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mxdgshellclient.cpp (44 lines)

Diff 72201

View Options

xdgshellclient.cpp

Loading...