D27788: Implement EGL_KHR_partial_update and EGL_EXT_swap_buffers_with_damage

[Vlad Zahorodnii]

zzag added a comment.


  Overall, looks pretty good to me, but I still have some minor nitpicks.

INLINE COMMENTS

> backend.cpp:119
>  
> +void OpenGLBackend::aboutToStartPainting(const QRegion& damage)
> +{

Style: add whitespace before `&`

> backend.h:82
> +     */
> +    virtual void aboutToStartPainting(const QRegion & damage);
> +

Style: remove whitespace after `&`

> egl_gbm_backend.cpp:447
> +        rects << rect.left();
> +        rects << height - rect.bottom() - 1;
> +        rects << rect.width();

I suggest to use (rect.y() + rect.height()) instead of rect.bottom() + 1 because QRect::bottom() deviates from the true bottom border. QRect::right() and QRect::bottom() are good when you need to snap the right or the bottom border of one rectangle to another, e.g.

  rect.moveRight(anotherRect.right());

otherwise, it is better to stick with x() + width() and y() + height() to avoid off by one errors.

> egl_gbm_backend.cpp:468
> +                                                output.output->geometry().height());
> +        const bool correct = rects.isEmpty()
> +                        || eglSetDamageRegionKHR(eglDisplay(), output.eglSurface,

According to the spec, eglSetDamageRegionKHR() handles the case where n_rects is 0, so we can drop `rects.isEmpty()` check and thus simplify code a bit more.

REPOSITORY
  R108 KWin

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

To: apol, #kwin, #plasma:_mobile
Cc: mwolff, zzag, davidedmundson, kwin, Orage, cacarry, 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/20200313/181fbec0/attachment-0001.html>