romangg added inline comments.

INLINE COMMENTS

> main_wayland.cpp:780
>          pluginName = KWin::automaticBackendSelection();
> +        std::cerr << "Selected backend " << pluginName.toStdString() << 
> std::endl;
>      }

Put this after the if clause (such that it shows the selected backend also on 
manual setting). But it's unrelated to GBM remote accesss, so better remove it 
and commit it as separate patch.

> drm_backend.cpp:103
>  {
> +    qCInfo(KWIN_DRM) << "Initializing DRM backend";
>      LogindIntegration *logind = LogindIntegration::self();

Unrelated to GBM remote access. Remove.

> drm_output.h:139
>  private:
> +    friend class RemoteAccessManager;
>      friend class DrmBackend;

Is it only a friend class to access `m_waylandOutput.data()`? In this case 
better create a getter for it in DrmOutput.

Or better do the `passBuffer` call in `DrmBackend::present` and give instead of 
the DrmOutput the `KWayland::Server::OutputInterface` from there to 
`passBuffer`.

> egl_gbm_backend.cpp:160
> +{
> +    if (!qEnvironmentVariableIsSet("KWIN_REMOTE"))
> +        return;

Should be the default not directly activated remote funcitonality? And if one 
wants to deactivate remote set `KWIN_NO_REMOTE` or something.

> remoteaccess_manager.cpp:85
> +
> +    qCDebug(KWIN_DRM) << "Buffer passed: bo" << gbmbuf->getBo() << ", fd" << 
> buf->fd();
> +

This will spam the debug because it is called on every present.

REPOSITORY
  R108 KWin

BRANCH
  gbm-vnc

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

To: Kanedias, graesslin, davidedmundson, romangg, #kwin
Cc: jgrulich, romangg, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, 
plasma-devel, schernikov, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein

Reply via email to