D24455: [wayland] Implement window geometry more properly

davidedmundson accepted this revision.
davidedmundson added a comment.
This revision is now accepted and ready to land.


  Technically there's one potentially interesting behavioural change. 
  If a client is server side decorated and then draws a subsurface outside the main surface, previously I believe we would have drawn our frame over the main surface, whereas now I think we'll draw our frame over the union. Probably not a real world issue though.

INLINE COMMENTS

> xdgshellclient_test.cpp:1311
> +    QCOMPARE(client->frameGeometry().topLeft(), oldPosition);
> +    QCOMPARE(client->frameGeometry().size(), QSize(120, 60));
> +    QCOMPARE(client->bufferGeometry().topLeft(), oldPosition + QPoint(20, 10));

Generally it's a lot easier to read something like

QCOMPARE(...,  QSize(100 + 20, 50 + 10));

and then it's apparent where the new numbers come from

(don't bother changing it though)

> xdgshellclient.cpp:549
>      const auto old = geometryBeforeUpdateBlocking();
> +    addRepaintDuringGeometryUpdates();
>      updateGeometryBeforeUpdateBlocking();

the

if (!m_unampped) has gone

Arguably it's a non issue as we'll just trigger a repaint that will in turn not do anything, but I'm wondering if it was there deliberately.

> zzag wrote in xdgshellclient.h:197
> Will change. I added this bool to avoid a few unnecessary comparisons in QRect::isValid. However, I guess it's a premature optimization.

It makes sense to keep a boolean to see if it's externally set or implicitly set. I think with a rename to
m_windowGeometryExplicitlySet  or something it's fine.

REPOSITORY
  R108 KWin

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

To: zzag, #kwin, davidedmundson
Cc: davidedmundson, romangg, kwin, LeGast00n, The-Feren-OS-Dev, sbergeron, jraleigh, fbampaloukas, GB_2, mkulinski, ragreen, jackyalcine, iodelay, crozbo, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, ahiemstra, mart