romangg added a comment.

  Do typedefs for `KWayland::Server::LinuxDmabuf` in files where you use it 
more than once.

INLINE COMMENTS

> abstract_egl_backend.cpp:346
>  
> +void AbstractEglBackend::aboutToDestroy(EglDmabufBuffer *buffer)
> +{

Name should be more descriptive in relation to functionality, also it's not a 
signal, so "aboutTo" imo not recommended.

Suggestion: `removeDmabufBuffer`

> abstract_egl_backend.h:100
>      QList<QByteArray> m_clientExtensions;
> +    QLinkedList<EglDmabufBuffer *> m_dmabufBuffers;
> +    bool m_haveDmabufImport = false;

Why QLinkedList? It should be no better than QList for the removeOne call. For 
this better use QSet.

> abstract_egl_backend.h:135
>  
> +class KWIN_EXPORT EglDmabufBuffer : public 
> KWayland::Server::LinuxDmabuf::Buffer
> +{

Why is it necessary to export?

REPOSITORY
  R108 KWin

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

To: fredrik, #kwin, #plasma, davidedmundson, mart, graesslin
Cc: romangg, anthonyfieroni, plasma-devel, kwin, iodelay, bwowk, ZrenBot, 
lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

Reply via email to