[Differential] [Updated] D1231: Add Remote Access interface to KWayland

[Kanedias (Oleg Chernovskiy)]

Kanedias marked 12 inline comments as done.
Kanedias added inline comments.

INLINE COMMENTS

> graesslin wrote in test_remote_access.cpp:161
> use QCOMPARE to test two values. If it fails with QCOMPARE you get the values it actually had. With QVERIFY you only see that it failed

Got it

> graesslin wrote in registry.h:144
> just a note: it's 5.24 since today...

Understood, changed

> graesslin wrote in remote_access.h:193
> hmm maybe parametersObtained?

Sounds reasonable, changed

> graesslin wrote in remote_access_interface.cpp:28
> you can just include "logging_p.h"

Noted, re-using categories from there.

> graesslin wrote in remote_access_interface.cpp:240
> qCDebug

Done

> graesslin wrote in remote_access_interface.cpp:263
> qCDebug

Done

> graesslin wrote in remote_access_interface.cpp:336
> you can use the new resourceDestroyedCallback in resource_p.h. It handles the destroy correctly and that will trigger the unbind and deleteLater automatically.

Reused, thanks

> graesslin wrote in remote_access_interface.cpp:346
> that would trigger a double delete as I had to learn very painfully lately.

Removed that code completely, thanks to `resourceDestroyedCallback`

> graesslin wrote in remote_access_interface.cpp:356-359
> you don't need that, it's already in Resource

Removed

> graesslin wrote in remote_access_interface.h:45
> Thinking out loud:
> 
> What if in future we pass non GbmBuffers? Should we then still call it GbmBuffer or do we need a new class? Maybe we should make it a nested class to RemoteAccessManagerInterface and then just call it Buffer? Or keep it outside and call it more generic RemoteBuffer?

Yes, it will no longer be GBM. Seeing so much EGLStreams and Vulkan struggles around it would be good to rename it into something more generic... noted!

> graesslin wrote in remote_access_interface.h:77
> who's the owner of the GbmBuffer? Who will delete it?

The owner is the process that originally passed it. So KWin here. In my non-submitted PoC from KWin side it closes its fd and deletes it after `bufferReleased` call

> graesslin wrote in remote_access_interface_p.h:33
> could you please add a note about that it's an internal class

Done, improved Doxygen comment also

> graesslin wrote in remote_access_interface_p.h:58
> nitpick

Sure, fixed

REPOSITORY
  rKWAYLAND KWayland

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: Kanedias, graesslin
Cc: plasma-devel, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160613/b4afa03e/attachment-0001.html>