[wayland] Asyncronously update maximise flags
ClosedPublic

Authored by davidedmundson on Aug 29 2018, 6:56 PM.

Details

Summary

A window maximising is an async operation. We work out what size we want
the client to be, then request the client to update. The window isn't
really maximised until we get that new buffer with the new size.

This patch splits the requested, pending and current state, updating as
appropriate.

Things are a bit complex with things like borders. Technically we
shouldn't update them till we get a response, but we also need to have
the correct geometry of the full size window in our request. For now
they behave as before, updating when we request the change.

X code is untouched.

This hopefully fixes maximise animations on wayland as now we update the
geometry before emitting maximisedChanged.

Test Plan

Maximised a window with the button and double clicking title bar.

I get only the following events on maximise/restore:
19:51:39.156 KWin::EffectsHandlerImpl::slotGeometryShapeChanged geometry shape changed QRect(47,24 640x509) QRect(0,0 716x573)
19:51:39.157 KWin::EffectsHandlerImpl::slotClientMaximized slot client maximised true true

19:51:40.522 KWin::EffectsHandlerImpl::slotGeometryShapeChanged geometry shape changed QRect(0,0 716x573) QRect(47,24 640x509)
19:51:40.522 KWin::EffectsHandlerImpl::slotClientMaximized slot client maximised false false

Though to me it looks to flicker slightly on the first frame?
@vzzag can you take a look

Diff Detail

Repository
R108 KWin
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2344
Build 2362: arc lint + arc unit
davidedmundson created this revision.Aug 29 2018, 6:56 PM
Restricted Application added a project: KWin. · View Herald TranscriptAug 29 2018, 6:56 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
davidedmundson requested review of this revision.Aug 29 2018, 6:56 PM
zzag added a subscriber: zzag.
This comment was removed by zzag.
zzag added a comment.EditedAug 29 2018, 10:46 PM

The maximize effect receives 2 windowMaximizedStateChanged.

If I comment https://phabricator.kde.org/source/kwin/browse/master/geometry.cpp$2194-2197, the effect works.

Edit: I think that's because of updateMaximizeMode(m_requestedMaximizeMode) at shell_client.cpp:594. Not sure, it takes a while to compile KWin. :(

Don't update maximise state if geometry is blocked
When it becomes unblocked it will call setGeometry and we will update then

That code /shouldn't/ emit anything as changeMaximise doesn't change change m_maximize so old == new.

but that was happening when geometry updates were blocked, that's now fixed.

abetts added a subscriber: abetts.Aug 30 2018, 4:28 AM
abetts removed a subscriber: abetts.

clientMaximizedStateChanged is now called on ShellClients. Before it was only called on Clients in geometry.cpp. It's not mentioned in the Summary. Is this change intended?

clientMaximizedStateChanged is now called on ShellClients. Before it was only called on Clients in geometry.cpp. It's not mentioned in the Summary. Is this change intended?

It was called in AbstractClient::setMaximize which affects both ShellClients and Clients.

With this patch on wayland that does nothing because the state hasn't changed before and after the virtual changeMaximize doesn't change. So we still get the same amount when we have an async update.

I would move those signals into Client:changeMaximize for consistency, but that's called directly in a few places and it would introduce an X behavioural change (though probably a safe one)

Looking at the code this patch is based on the changes in D15135. Please add it as parent revision.

romangg accepted this revision.Sep 17 2018, 5:15 PM
romangg added inline comments.
shell_client.cpp
836

Improve coding style when you are at it.

shell_client.h
226

Why did you move these lines from below up here? I would leave them where m_mximizeMode was before. But it's not important. You can push as is as well.

This revision is now accepted and ready to land.Sep 17 2018, 5:15 PM
This revision was automatically updated to reflect the committed changes.
zzag added a comment.Oct 6 2018, 10:36 AM

Have border issues been fixed? After a window is restored, it doesn't have borders anymore.

Have border issues been fixed? After a window is restored, it doesn't have borders anymore.

I was unaware of border issues, but you are right. I shall fix ASAP.

This change broke TestShellClient::testMaximizedToFullscreen - at least git bisect pointed out this commit.

git bisect start
# good: [65b8ba1770c21ed96e5faa4615623a4cddd79928] [wayland] Syncronise pending geometry with acked configure requests
git bisect good 65b8ba1770c21ed96e5faa4615623a4cddd79928
# bad: [e637d432111f14639ed3388e11f5dc2188e9e676] Remove potential endless loop from XClipboardSyncTest
git bisect bad e637d432111f14639ed3388e11f5dc2188e9e676
# bad: [7b19fa87d0f73a6f168d2ffefc32730ce8044424] [libkwineffects] Delete unused field in AniData
git bisect bad 7b19fa87d0f73a6f168d2ffefc32730ce8044424
# bad: [17d4cbe12510ffd7d82bb7bb70b10759a63b1f47] [wayland] Correctly initialise m_requestedClientSize
git bisect bad 17d4cbe12510ffd7d82bb7bb70b10759a63b1f47
# bad: [5d5816be2bac29a7196a05d981d979bab6072ce6] [libkwineffects] Don't expose the fullscreen effect lock to the public API
git bisect bad 5d5816be2bac29a7196a05d981d979bab6072ce6
# bad: [e60c878647a71fa7fecbbf7feae258e8e6ba25c8] SVN_SILENT made messages (.desktop file) - always resolve ours
git bisect bad e60c878647a71fa7fecbbf7feae258e8e6ba25c8
# bad: [c244ee5e3e00e6023cae32aa06ed56d8cb437030] SVN_SILENT made messages (.desktop file) - always resolve ours
git bisect bad c244ee5e3e00e6023cae32aa06ed56d8cb437030
# bad: [e16334b3f2a54a8b1252f4334aeca67a995b78c5] SVN_SILENT made messages (.desktop file) - always resolve ours
git bisect bad e16334b3f2a54a8b1252f4334aeca67a995b78c5