Kanedias marked 8 inline comments as done. Kanedias added inline comments. INLINE COMMENTS
> graesslin wrote in remote-access.xml:19 > I noticed that standard Wayland protocols also have a destructor request for > the manager interface. I'd suggest to also add it (it makes sense as then the > server can destroy the resource). Added destructor and release callback. > graesslin wrote in remote-access.xml:33 > so the idea here would be that if in future we want to support other buffers > (e.g. shared mem) we just add a new event? Yes, with appropriate "since" clause > graesslin wrote in remote-access.xml:41 > Just for thought: in other interfaces the destructor is mostly called > "release" Renamed > graesslin wrote in registry.h:128 > please add as last interface otherwise it breaks API Moved to last in enum. Or did you mean in forward references declaration too?.. > graesslin wrote in registry.h:379 > we moved to frameworks which means we are now at 5.23 - sorry about that. I > also had to rename a bunch of @since 5.7 ;-) Corrected > graesslin wrote in remote_access_interface.cpp:146-149 > if really only one client should be allowed (why?) it would be better to send > a dedicated error state to inform it instead of "abusing" no memory. Added ability to have multiple clients in the same time > graesslin wrote in remote_access_interface.cpp:158 > this allows to have only one client bind it. As soon as a second client binds > the protocol it will get overwritten and breaks the existing one. > > I think you need a QVector<wl_resource*> here. Reimplemented > graesslin wrote in remote_access_interface.h:46 > For ABI compatibility it's better to only have d-ptr-ized classes/structs in > the public header. Implemented GbmBuffer with d-pointer > graesslin wrote in remote_access_interface.h:49-50 > why do we need gbm_surface and gbm_bo? This looks like adding not needed > details to the interface Removed, reimplemented > graesslin wrote in remote_access_interface.h:43 > for ABI compatibility we cannot have structs defined in the header. See > https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B#The_Do.27s_and_Don.27ts > for explanation. Especially the last point of the Donts is relevant as it > means we are never able to change this again. > > Thus I think the proper way has to be: > > class Buffer > { > public: > void setFd(int qint32); > void setSize(const QSize &size); > void setStride(qint32 stride); > enum class Format { > Format1, > Format2 > }; > void setFormat(Format format); > > private: > class Private; > QScopedPointer<Private> d; > }; > > and then in the Implementation have the Private class which holds the data > members. Thanks for clarification. Reimplemented. 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
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
