D18824: Implement wl_eglstream_controller Server Interface

This revision was automatically updated to reflect the committed changes. Closed by commit R127:e208389cc68c: Implement wl_eglstream_controller Server Interface (authored by ekurzinger). REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18824?vs=56078=56299

D18824: Implement wl_eglstream_controller Server Interface

romangg accepted this revision. romangg added a comment. This revision is now accepted and ready to land. Thanks. Don't forget to add the isValid check to the KWin patches now. Maybe also add a comment to the KWayland code, what's the reason for the separate create() call and that we want to

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger marked 2 inline comments as done. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D18824 To: ekurzinger, romangg, davidedmundson, #kwin Cc: zzag, ngraham, mati865, schernikov, guilhermesi, kde-frameworks-devel, michaelh, bruns

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger updated this revision to Diff 56078. ekurzinger added a comment. Changed the dynamic_cast in create() to static_cast. Since the Private is created in the constructor of EglStreamControllerInterface, it should be safe to assume it is of the correct type. Also marked Private as

D18824: Implement wl_eglstream_controller Server Interface

zzag added inline comments. INLINE COMMENTS > eglstream_controller_interface.cpp:103 > +{ > +dynamic_cast(*d).create(); > +} Can we avoid dynamic casting a reference? Is it possible to use static_cast instead? > eglstream_controller_interface_p.h:33 > + > +class

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger marked an inline comment as done. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D18824 To: ekurzinger, romangg, davidedmundson, #kwin Cc: ngraham, mati865, schernikov, guilhermesi, kde-frameworks-devel, michaelh, bruns

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger updated this revision to Diff 56075. ekurzinger added a comment. Ok, now we will bail out early during create() if we fail to dynamically load the interface (before calling wl_global_create). The compositor can then use isValid() to check if whether creating the interface

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger marked an inline comment as done. ekurzinger added a comment. In D18824#448597 , @romangg wrote: > In the first revision the interface failed to create when the dynamically loaded library was not present (we said the class should

D18824: Implement wl_eglstream_controller Server Interface

davidedmundson added a comment. > Now it just fails silently on bind. Why is it ok to fail silently on bind? We wouldn't get that far. If the library didn't load then we wouldn't have sent an interface to wl_global_create I don't know how that behaves, but we would have either

D18824: Implement wl_eglstream_controller Server Interface

davidedmundson accepted this revision. davidedmundson added inline comments. INLINE COMMENTS > eglstream_controller_interface.h:49 > +public: > +virtual ~EglStreamControllerInterface(); > + pedantically that should be: ~EglStreamControllerInterface() override; REPOSITORY R127 KWayland

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger updated this revision to Diff 56067. ekurzinger added a comment. Re-added protocol XML file to diff and corrected version in display.h REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18824?vs=56009=56067 REVISION DETAIL

D18824: Implement wl_eglstream_controller Server Interface

romangg added a comment. The xml file is missing form the diff now. Don't forget to add it when pushing. In the first revision the interface failed to create when the dynamically loaded library was not present. Now it just fails silently on bind. Why is this ok? INLINE COMMENTS >

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger updated this revision to Diff 56009. ekurzinger added a comment. removed C-style casts REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18824?vs=51709=56009 REVISION DETAIL https://phabricator.kde.org/D18824 AFFECTED FILES

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger marked an inline comment as done. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D18824 To: ekurzinger, romangg, davidedmundson, #kwin Cc: ngraham, mati865, schernikov, guilhermesi, kde-frameworks-devel, michaelh, bruns

D18824: Implement wl_eglstream_controller Server Interface

zzag resigned from this revision. zzag added a comment. I don't have any NVIDIA hardware to test these patches so I'm kinda useless. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D18824 To: ekurzinger, romangg, davidedmundson, #kwin Cc: ngraham, mati865,

D18824: Implement wl_eglstream_controller Server Interface

zzag added inline comments. INLINE COMMENTS > eglstream_controller_interface.cpp:58 > +Q_UNUSED(client); > +Private *p = (Private *)wl_resource_get_user_data(resource); > +emit p->q->streamConsumerAttached(SurfaceInterface::get(surface), (void > *)eglStream, attribs); Please don't

D18824: Implement wl_eglstream_controller Server Interface

ngraham added a comment. @romangg, are you satisfied with this now? REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D18824 To: ekurzinger, romangg, davidedmundson, zzag, #kwin Cc: ngraham, mati865, schernikov, guilhermesi, kde-frameworks-devel, michaelh, bruns

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger marked 7 inline comments as done. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D18824 To: ekurzinger, romangg, davidedmundson, zzag, #kwin Cc: schernikov, guilhermesi, kde-frameworks-devel, michaelh, ngraham, bruns

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger updated this revision to Diff 51709. ekurzinger added a comment. Thanks for taking a look, Roman. I've addressed your style suggestions and added support for passing stream attributes to the compositor. I had ignored these initially since the driver doesn't actually use this

D18824: Implement wl_eglstream_controller Server Interface

romangg added inline comments. INLINE COMMENTS > CMakeLists.txt:349 >xdgoutput_interface.h > + eglstream_controller_interface.h > ) alphabetical sorted > display.cpp:529 > +} else { > +qCWarning(KWAYLAND_SERVER) << "Unable to load > libnvidia-egl-wayland.so.1"; > +}

D18824: Implement wl_eglstream_controller Server Interface

romangg requested changes to this revision. romangg added a comment. This revision now requires changes to proceed. Impressive. Some small issues to resolve. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D18824 To: ekurzinger, romangg, davidedmundson, zzag, #kwin

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger added a dependent revision: D18570: EGLStream DRM Backend Initial Implementation. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D18824 To: ekurzinger, romangg, davidedmundson, zzag, #kwin Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger removed a dependency: D18570: EGLStream DRM Backend Initial Implementation. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D18824 To: ekurzinger, romangg, davidedmundson, zzag, #kwin Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger added a dependency: D18570: EGLStream DRM Backend Initial Implementation. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D18824 To: ekurzinger, romangg, davidedmundson, zzag, #kwin Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18824: Implement wl_eglstream_controller Server Interface

ekurzinger created this revision. ekurzinger added reviewers: romangg, davidedmundson, zzag, KWin. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ekurzinger requested review of this revision. REVISION SUMMARY This implements a wrapper class for the