Am 2017-07-12 18:35, schrieb David Edmundson:
On Wed, Jul 12, 2017 at 4:02 PM, Martin Flöser <mgraess...@kde.org>
wrote:

Hi all,

I had a look at the issues of KRunner on Wayland with Qt 5.9 and
want to share my findings.

What we see is KRunner working fine on first invokation, but
afterwards everything is messed up:
* maximized windows shortly change their size
* KRunner opens behind other windows

With WAYLAND_DEBUG I was able to understand the issue. On first
invokation everything is normal:
* Plasma shell surface gets created
* Plasma shell surface gets positioned
* Plasma shell surface gets marked as panel
* Panel is set to windows go below
* Panel is marked as accepts focus
* Surface gets mapped

Yay, a situation KWin can handle.

Now the window gets hidden, and shown again and Qt recreates the
surface. Due to queuing we have
1. Surface gets mapped
2. KWin shows the window at random position
3. Plasma shell surface gets created and marked as panel
4. KWin adjust it to be a panel and moves maximized windows around
5. Plasma shell surface gets marked as panel with windows go below
6. KWin adjust it to be a panel with windows go below and moves
maximized windows around
7. Panel gets marked as accepts focus (too late, too late)
8. KRunner gets positioned

So we are in a bad state. When looking at it our Plasma Shell
Surface protocol is not double buffered which explains why KWin
reacts on each and every of the actions. It was designed so that
double buffered is not needed as it was supposed to be used before
mapping the window.
 > Any ideas or advice?

I can do inane rambling instead.

----

With Xdgv6 (in theory!) this ends up being a non-issue.

A client is not allowed to attach a buffer until it gets a configure
event from the server.

This throws in an extra roundtrip and according to my logs on the
second showing that means we do get all the plasma shell stuff sent
before we send a configure which means we'll have processed them all
before we get the first buffer and map the window.

this is extremely good! That will solve many many issues we have with initial state handling.


In practice we get the role and location, but not the position (which
means I think I now get what Marco's wayland krunner was trying to
do.)

The position is not that much of an issue as long as KWin gets it before KWin renders.


---

We should be double buffering the proper shells (wl_shell, xdgV5/6)
and only emitting the surfaceCreated signals when the surface is
comitted.

IIRC wl_shell is not defined to be double buffered. Not sure how it's about xdg. There is always the chance of breaking things *cough*Qt*cough* when changing that - even if it is sane.

What could be an idea is to not do anything in KWin till we get the first map event. That is evaluate all the shell options only on map.


In practice that won't acheive anything as QWaylandWindow::initWindow
calls commit a few times randomly before the expose event that we're
currently hooking into is called. Also kwayland-integration calls
commit on the surface every time it attaches something, which I don't
think is right.

It is fine given the definition of the interfaces. We setup some state and commit it. If we don't commit we don't know when that will happen. We only adjusted our own relevant states and everything else is not affected.


--

Now with Qt's new behavior that bites our ass and I'm a little bit
lost what to do. I think we can improve the situation by first
setting the panel behavior, but the wrong position is something we
cannot prevent if the window is already mapped. Even changing the
protocol to be double buffered would not really help here as we are
too late. It's not every frame is perfect.

I do think we can change Qt's behaviour. With XdgV7 it's clarified
that there really is no need to delete the xdg_toplevel to unmap.

I hope they believe us now on this point...


That still leaves Qt having the "new behaviour" if one calls
QWindow::setParent or QWindow::setType, but from our POV that's fine.

well setType is a bit problematic for plasma-integration (windeco, etc.), but at least for Plasma surfaces it's fine.

Cheers
Martin

Reply via email to