D18570: EGLStream DRM Backend Initial Implementation

cfeck added inline comments.

INLINE COMMENTS

> abstract_egl_backend.cpp:130
>  
> +GLboolean AbstractEglBackend::queryWaylandBufferWL(wl_resource *buffer,
> +                                                   EGLint attribute, EGLint *value)

Does it really need to be declared `GLboolean`? You are using the logical `&&` operator anyway.

> drm_backend.cpp:642
> +    if (m_useEglDevice) {
> +        setSoftWareCursor(true);
> +    }

A was searching for the definition of this function, because it is named wrongly (should be `setSoftwareCursor`), but it seems to be a mistake in the base class. If it's private API, it might be worth to rename it.

> drm_backend.cpp:701
>      }
>      const QImage &cursorImage = softwareCursor();
>      if (cursorImage.isNull()) {

For implicitely shared Qt classes, it might be dangerous to use a reference, because that won't increase the shared-ref counter. Please just omit the `&`.

> drm_backend.cpp:706
>      }
>      for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) {
>          (*it)->updateCursor();

If `for (const &output : m_outputs)` also works here (and at other places), you could improve readability.

> egl_stream_backend.cpp:188
> +        if (!hasExtension(ext)) {
> +            setFailed(QStringLiteral("Missing required EGL extension: ") + ext);
> +            return false;

`ext` is only a byte array. To add it to a string, Qt needs to know in which encoding the byte array is. Please use `... + QLatin1String(ext)`.

> egl_stream_backend.cpp:324
> +    const auto outputs = m_backend->drmOutputs();
> +    for (DrmOutput *drmOutput: outputs) {
> +        createOutput(drmOutput);

Space before `:`

> egl_stream_backend.cpp:438
> +    
> +    const QSize &overall = screens()->size();
> +    const QRect &v = output.output->geometry();

Not sure here, but I think these should also not be reference variables.

> egl_stream_backend.cpp:475
> +{
> +    for (auto &o: m_outputs) {
> +        makeContextCurrent(o);

Space

REPOSITORY
  R108 KWin

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

To: ekurzinger, davidedmundson, graesslin, #kwin, romangg
Cc: cfeck, rapiteanu, romangg, hein, nicolasfella, zzag, kwin, jraleigh, GB_2, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, ngraham, alexeymin, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart