D27861: WIP: Rework xdg-shell implementation

[David Edmundson]

davidedmundson added a comment.


  We should be discussing big changes before doing the full implementation. It's quite frustrating.
  
  RE: Kwayland
  Yes there are definitely some problems and it needs some directional planning and changes. That needs a broader mailing list discussion, it shouldn't be done within the context of any specific patch.
  You've been promising to send an email on that for weeks now :(
  
  -------
  
  > The new wrappers weren't implemented in
  > KWayland because I cannot guarantee that the future versions of the
  > wrappers will be API compatible.
  
  Whilst I do agree with the overall comments on kwayland, this specific case is a bad example.
  
  WMBase is finally stable API. Any new changes within WMBase versions have to be backwards compatible at a protocol level. Those have proved to be relatively easy to maintain compatibility at a library level.
  XdgShellV6 is obviously also stable at this point.
  
  We know that XdgShellV6(v1) will /always/ be basically identical to XdgWmBase(v1). As XdgWmBase goes through different versions, we will be keeping an abstraction layer inside XdgShellClient back to v1.
  It seems very backwards that we've successfully fought an abstraction layer over something unstable, then removed it the instant things are actually stable!
  
  ----
  
  Ending on some positive things:
  
  - I like the split of XdgTopLevel and XdgPopup that 100% makes sense. Also paves the way for future roles nicely.
  - We probably do want to redo our interface side to be based around WMBase rather than XdgShellV5. A rewrite probably was needed.
  - I like use of Qt's wayland scanner. It is a lot easier to read.

INLINE COMMENTS

> xdgshellclient_test.cpp:1417
>      QCOMPARE(client->frameGeometry().topLeft(), oldPosition);
> -    QCOMPARE(client->frameGeometry().size(), QSize(100, 50));
> +    QCOMPARE(client->frameGeometry().size(), QSize(90, 40));
>      QCOMPARE(client->bufferGeometry().topLeft(), oldPosition - QPoint(10, 10));

This needs explanation.

The one thing that makes such a big change somewhat safe is the extensive unit tests. 
If we change them at the same time, then we've lost all that.

REPOSITORY
  R108 KWin

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

To: zzag, #kwin
Cc: davidedmundson, kwin, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 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/20200305/3ec4c948/attachment-0001.html>