[wayland] XdgPopup Positioning
ClosedPublic

Authored by davidedmundson on Oct 19 2018, 5:34 PM.

Details

Summary

Support XDGShell Positioning. This gives a client a lot more control
over where the popup will be placed as well as control over how to
handle constraints. i.e what to do if the popup doesn't fit.

trasientOffset was replaced with a method on the client as semantically
it's the role of the client to handle constraints.

Both slide and flip constraint adjustments are implemented. Resize
constraint adjustment will be handled in a future patch.

WlShell is handled by treating it as 1x1 sized anchor with slide
constraint adjustment.

Test Plan

Manual test of a client implementing xdgpopup exists in kwayland
Extensive unit test here

Existing WlShell test passes (after D16314 which fixes the original)
XdgPopup has a new unit test suite against manually calculated values

Diff Detail

Repository
R108 KWin
Branch
xdg_popup
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4064
Build 4082: arc lint + arc unit
davidedmundson created this revision.Oct 19 2018, 5:34 PM
Restricted Application added a project: KWin. · View Herald TranscriptOct 19 2018, 5:34 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
davidedmundson requested review of this revision.Oct 19 2018, 5:34 PM

Remove unused include

zzag added a subscriber: zzag.EditedOct 19 2018, 7:25 PM

//if it still doesn't fit we should resize the unflipped version

"Resize" or "use"?

Coding style nitpick: please shift body(cases) of each switch statement 4 spaces to the left.

zzag added inline comments.Oct 19 2018, 7:41 PM
shell_client.cpp
1643

Aren't FlipX and FlipY having precedence over SlideX and SlideY respectively?

davidedmundson marked an inline comment as done.Oct 19 2018, 9:24 PM
davidedmundson added inline comments.
shell_client.cpp
1643

heh, the spec lists slide first in the enum

But yeah, we should.

graesslin accepted this revision.Oct 20 2018, 6:58 AM
This revision is now accepted and ready to land.Oct 20 2018, 6:58 AM
zzag added a comment.Oct 20 2018, 8:51 AM

Some minor nitpicks.

abstract_client.cpp
1175

Q_UNREACHABLE.

abstract_client.h
395–396

Typo: "only valid is"

autotests/integration/kwin_wayland_test.h
27

That's not Qt include. :-)

autotests/integration/transient_placement.cpp
310

Typo: contrain.

placement.cpp
505–509

Please reformat it.

shell_client.cpp
1620

Missing whitespace before "="

1623

Q_UNREACHABLE

1705

Missing whitespace between "switch" and "("

shell_client.h
213

We probably don't need const for anchor edges and gravity.

zzag added inline comments.Oct 20 2018, 8:54 AM
shell_client.cpp
1623

... or Q_ASSERT_X to print more "user"-friendly message.

davidedmundson marked 11 inline comments as done.Oct 20 2018, 4:17 PM
davidedmundson added inline comments.
shell_client.cpp
1623

This is the one of the few times where Q_UNREACHABLE actually saves a single jump instruction. May as well, given it documents the assert better.

shell_client.h
213

Sure I don't need them from the view of whoever is calling this method, I'm making a copy and functionally equivalent but I want to document them as being local within the body of the method.

This revision was automatically updated to reflect the committed changes.
davidedmundson marked an inline comment as done.