subdiff added a comment.

  Hi Oleg,
  
  sorry that I haven't given you feedback to your patch series until now. First 
some smaller points:
  
  - Please merge current master.
  - What about the software cursor dynamically switching? I don't see where it 
does that.
  - The RemoteAccessManager is active all the time. It would be nice if it 
would be only active if there are clients who ask for buffers.
  
  Then in general did you look at GNOME's way forward of screen 
recording/sharing on Wayland? They use PipeWire (http://pipewire.org/) for 
that. I'm not very knowledgeable on the whole desktop sharing / video stream 
side, so just a small info on what you think about it and how this relates to 
your code would be nice. I'm asking that, because I find joint work on single 
solutions very important.

INLINE COMMENTS

> remoteaccess_manager.cpp:77
> +    auto buf = new BufferHandle;
> +    buf->setFd(gbm_bo_get_fd(gbmbuf->getBo()));
> +    buf->setSize(gbm_bo_get_width(gbmbuf->getBo()), 
> gbm_bo_get_height(gbmbuf->getBo()));

Local var for `gbmbuf->getBo()`

> remoteaccess_manager.h:24
> +#include "drm_backend.h"
> +#include "../../../wayland_server.h"
> +// KWayland

Do you need all these includes in the header file, or could you forward declare 
for example the KWayland::Server::RemoteAccessManagerInterface and then include 
in the cpp file?

REPOSITORY
  R108 KWin

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

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

Reply via email to