D27861: WIP: Rework xdg-shell implementation

[David Edmundson]

davidedmundson added a comment.


  Generally very nice and tidy. Will be even more so once we remove PlasmaShellsurface.
  
  I have some  minor comments.
  
  I probably need to give it a second pass as it was quite overwhelming. Remember to drop the "WIP:" when ready

INLINE COMMENTS

> wayland_server.cpp:149
>  {
> -    if (!Workspace::self()) {
> -        // it's possible that a Surface gets created before Workspace is created
> -        return;
> +    if (client->readyForPainting()) {
> +        emit shellClientAdded(client);

This can never be true.
It's a protocol error on XDGShell to map before configure

> waylandxdgshellintegration.cpp:81
> +
> +void WaylandXdgShellIntegration::createClient(XdgSurfaceInterface *surface)
> +{

This method seems rather pointless. We are in specialisations for  registerXdgTopLevel / popup    then we call a generic method just to switch on whether it's a popup or a toplevel.

> xdgshellclient.cpp:116
>  {
> +    qDeleteAll(m_configureEvents);
> +}

We leak the m_lastAckedConfigure if someone calls ackConfigure and gets deleted before commit.

I'm guessing you didn't use unique_ptr because of the QQueue, but storing via a qsharedpointer would work nicely.

> xdgshellclient.cpp:1428
> +    connect(shellSurface, &PlasmaShellSurfaceInterface::panelBehaviorChanged, this, [this] {
> +        updateShowOnScreenEdge();
> +        workspace()->updateClientArea();

I can't see panelTakesFocusChanged.

I strongly suspect this will break Kai's reply notifications.

Arguably the event name doesn't match the usage, but this isn't the time and place for that.

REPOSITORY
  R108 KWin

REVISION DETAIL
  https://phabricator.kde.org/D27861

To: zzag, #kwin
Cc: anthonyfieroni, univerz, davidedmundson, kwin, Orage, cacarry, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, mkulinski, ragreen, jackyalcine, iodelay, crozbo, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, romangg, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwin/attachments/20200512/6c5001a6/attachment-0001.htm>