fredrik added inline comments.
Restricted Application edited projects, added Plasma; removed Plasma on Wayland.

INLINE COMMENTS

> linuxdmabuf_v1_interface.h:39
> +
> +namespace LinuxDmabuf
> +{

Do we want these nested namespaces? We could have LinuxDmabufFlags, 
LinuxDmabufBuffer etc. instead.

> linuxdmabuf_v1_interface.h:65
> +     */
> +    class Buffer {
> +    public:

Should the Buffer class use a d-pointer?

> linuxdmabuf_v1_interface.h:107
> +     */
> +    class Bridge {
> +    public:

Is this the solution we want for interfacing with the compositor?

My preference would be to use std::function callbacks, with setters in 
LinuxDmabufUnstableV1Interface. Setting up the interface could then look like 
this:

  m_linuxDmabuf = m_display->createLinuxDmabufInterface(m_display);
  m_linuxDmabuf->setQuerySupportedFormats([]{ return 
Compositor::self()->scene()->supportedDrmFormats(); });
  ...
  m_linuxDmabuf->create();

This can also be extended without breaking binary compatibility. But I don't 
think we can use std::function in frameworks. There are also BIC issues when 
mixing different STL implementations, which we may or may not care about.

REPOSITORY
  R127 KWayland

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

To: fredrik, #kwin, #plasma, graesslin, davidedmundson, mart
Cc: plasma-devel, #frameworks, michaelh, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart, schernikov, alexeymin, eliasp, hein

Reply via email to