D7369: Wayland foreign protocol
This revision was automatically updated to reflect the committed changes. Closed by commit R127:ce2ff96c5756: Wayland foreign protocol (authored by mart). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D7369?vs=20639=20667#toc REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7369?vs=20639=20667 REVISION DETAIL https://phabricator.kde.org/D7369 AFFECTED FILES autotests/client/CMakeLists.txt autotests/client/test_xdg_foreign.cpp src/client/CMakeLists.txt src/client/protocols/xdg-foreign-unstable-v2.xml src/client/registry.cpp src/client/registry.h src/client/xdgforeign.cpp src/client/xdgforeign.h src/client/xdgforeign_p.h src/client/xdgforeign_v2.cpp src/client/xdgforeign_v2.h src/server/CMakeLists.txt src/server/display.cpp src/server/display.h src/server/xdgforeign_interface.cpp src/server/xdgforeign_interface.h src/server/xdgforeign_v2_interface.cpp src/server/xdgforeign_v2_interface_p.h tests/CMakeLists.txt tests/xdgforeigntest.cpp To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein
D7369: Wayland foreign protocol
graesslin accepted this revision. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7369 To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein
D7369: Wayland foreign protocol
mart updated this revision to Diff 20639. mart marked 7 inline comments as done. mart added a comment. Restricted Application edited projects, added Plasma on Wayland; removed Plasma. - more documentation REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7369?vs=20638=20639 BRANCH mart/xdgforeign REVISION DETAIL https://phabricator.kde.org/D7369 AFFECTED FILES autotests/client/CMakeLists.txt autotests/client/test_xdg_foreign.cpp src/client/CMakeLists.txt src/client/protocols/xdg-foreign-unstable-v2.xml src/client/registry.cpp src/client/registry.h src/client/xdgforeign.cpp src/client/xdgforeign.h src/client/xdgforeign_p.h src/client/xdgforeign_v2.cpp src/client/xdgforeign_v2.h src/server/CMakeLists.txt src/server/display.cpp src/server/display.h src/server/xdgforeign_interface.cpp src/server/xdgforeign_interface.h src/server/xdgforeign_v2_interface.cpp src/server/xdgforeign_v2_interface_p.h tests/CMakeLists.txt tests/xdgforeigntest.cpp To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein
D7369: Wayland foreign protocol
mart updated this revision to Diff 20638. mart added a comment. Restricted Application edited projects, added Plasma; removed Plasma on Wayland. - documentation++ - name the methods exportTopLevel/importTopLevel REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7369?vs=20584=20638 BRANCH mart/xdgforeign REVISION DETAIL https://phabricator.kde.org/D7369 AFFECTED FILES autotests/client/CMakeLists.txt autotests/client/test_xdg_foreign.cpp src/client/CMakeLists.txt src/client/protocols/xdg-foreign-unstable-v2.xml src/client/registry.cpp src/client/registry.h src/client/xdgforeign.cpp src/client/xdgforeign.h src/client/xdgforeign_p.h src/client/xdgforeign_v2.cpp src/client/xdgforeign_v2.h src/server/CMakeLists.txt src/server/display.cpp src/server/display.h src/server/xdgforeign_interface.cpp src/server/xdgforeign_interface.h src/server/xdgforeign_v2_interface.cpp src/server/xdgforeign_v2_interface_p.h tests/CMakeLists.txt tests/xdgforeigntest.cpp To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D7369: Wayland foreign protocol
graesslin added a comment.
There is still quite some documentation missing. Especially the server side
is not yet documented enough that I would know how to use the new API and what
it does. It's totally fine to copy and adapt the documentation from the Wayland
xml protocol.
INLINE COMMENTS
> xdgforeign.h:116
> +
> +XdgExported *exportSurface(Surface *surface, QObject *parent = nullptr);
> +
Please add documentation
> xdgforeign.h:208
> +
> +XdgImported *import(const QString & handle, QObject *parent = nullptr);
> +
Please add documentation
> xdgforeign.h:268
> +
> +QString handle() const;
> +
what's the handle?
> xdgforeign.h:327
> +
> +void setParentOf(Surface *surface);
> +
please add documentation
> xdgforeign_interface.h:38
> +
> +class KWAYLANDSERVER_EXPORT XdgForeignInterface : public QObject
> +{
This needs documentation
> xdgforeign_interface.h:45-46
> +
> +void create();
> +bool isValid();
> +
Please add documentation? I wouldn't know what create is needed for.
> xdgforeign_interface.h:49
> +/**
> + * if a client did set
> + */
did set "what"?
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D7369
To: mart, #plasma, #kwin, davidedmundson, graesslin
Cc: davidedmundson, graesslin, plasma-devel, #frameworks, leezu, ZrenBot,
alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp,
sebas, apol, mart, hein
D7369: Wayland foreign protocol
mart added a comment. In https://phabricator.kde.org/D7369#154153, @graesslin wrote: > In https://phabricator.kde.org/D7369#153978, @mart wrote: > > > In https://phabricator.kde.org/D7369#152358, @graesslin wrote: > > > > > Documentation is still missing, Unstable suffix is still in two many API calls. > > > > > > i think the importerUnstableV2Announced exporterUnstableV2Announced should remain with unstable and versioned? > > > Yes, those can stay. Other signals are similar. so how is the current state, can go in? (btw, the unstable-v2 protocol was just pushed to master, so another roadblock done) REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7369 To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein
D7369: Wayland foreign protocol
mart updated this revision to Diff 20584. mart added a comment. Restricted Application edited projects, added Plasma on Wayland; removed Plasma. - update to the upstream xml protocol REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7369?vs=20553=20584 BRANCH mart/xdgforeign REVISION DETAIL https://phabricator.kde.org/D7369 AFFECTED FILES autotests/client/CMakeLists.txt autotests/client/test_xdg_foreign.cpp src/client/CMakeLists.txt src/client/protocols/xdg-foreign-unstable-v2.xml src/client/registry.cpp src/client/registry.h src/client/xdgforeign.cpp src/client/xdgforeign.h src/client/xdgforeign_p.h src/client/xdgforeign_v2.cpp src/client/xdgforeign_v2.h src/server/CMakeLists.txt src/server/display.cpp src/server/display.h src/server/xdgforeign_interface.cpp src/server/xdgforeign_interface.h src/server/xdgforeign_v2_interface.cpp src/server/xdgforeign_v2_interface_p.h tests/CMakeLists.txt tests/xdgforeigntest.cpp To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein
D7369: Wayland foreign protocol
graesslin added a comment. In https://phabricator.kde.org/D7369#153978, @mart wrote: > In https://phabricator.kde.org/D7369#152358, @graesslin wrote: > > > Documentation is still missing, Unstable suffix is still in two many API calls. > > > i think the importerUnstableV2Announced exporterUnstableV2Announced should remain with unstable and versioned? Yes, those can stay. Other signals are similar. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7369 To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D7369: Wayland foreign protocol
mart updated this revision to Diff 20553. mart added a comment. Restricted Application edited projects, added Plasma; removed Plasma on Wayland. - add api docs, getridof some unstable, calls with unstable follow the other interfaces REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7369?vs=20257=20553 BRANCH mart/xdgforeign REVISION DETAIL https://phabricator.kde.org/D7369 AFFECTED FILES autotests/client/CMakeLists.txt autotests/client/test_xdg_foreign.cpp src/client/CMakeLists.txt src/client/protocols/xdg-foreign-unstable-v2.xml src/client/registry.cpp src/client/registry.h src/client/xdgforeign.cpp src/client/xdgforeign.h src/client/xdgforeign_p.h src/client/xdgforeign_v2.cpp src/client/xdgforeign_v2.h src/server/CMakeLists.txt src/server/display.cpp src/server/display.h src/server/xdgforeign_interface.cpp src/server/xdgforeign_interface.h src/server/xdgforeign_v2_interface.cpp src/server/xdgforeign_v2_interface_p.h tests/CMakeLists.txt tests/xdgforeigntest.cpp To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D7369: Wayland foreign protocol
mart added a comment. In https://phabricator.kde.org/D7369#152358, @graesslin wrote: > Documentation is still missing, Unstable suffix is still in two many API calls. i think the importerUnstableV2Announced exporterUnstableV2Announced should remain with unstable and versioned? REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7369 To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein
D7369: Wayland foreign protocol
graesslin requested changes to this revision. graesslin added a comment. This revision now requires changes to proceed. Documentation is still missing, Unstable suffix is still in two many API calls. INLINE COMMENTS > registry.h:527-528 > + > +zxdg_exporter_v2 *bindXdgExporterUnstableV2(uint32_t name, uint32_t > version) const; > +zxdg_importer_v2 *bindXdgImporterUnstableV2(uint32_t name, uint32_t > version) const; > ///@} documentation missing REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7369 To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein
D7369: Wayland foreign protocol
mart updated this revision to Diff 20257. mart added a comment. Restricted Application edited projects, added Plasma on Wayland; removed Plasma. - move to xdg-foreign-unstable-v2 - get rid of setTransientFor REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7369?vs=18767=20257 BRANCH mart/xdgforeign REVISION DETAIL https://phabricator.kde.org/D7369 AFFECTED FILES autotests/client/CMakeLists.txt autotests/client/test_xdg_foreign.cpp src/client/CMakeLists.txt src/client/protocols/xdg-foreign-unstable-v2.xml src/client/registry.cpp src/client/registry.h src/client/xdgforeign.cpp src/client/xdgforeign.h src/client/xdgforeign_p.h src/client/xdgforeign_v2.cpp src/client/xdgforeign_v2.h src/server/CMakeLists.txt src/server/display.cpp src/server/display.h src/server/xdgforeign_interface.cpp src/server/xdgforeign_interface.h src/server/xdgforeign_v2_interface.cpp src/server/xdgforeign_v2_interface_p.h tests/CMakeLists.txt tests/xdgforeigntest.cpp To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein
D7369: Wayland foreign protocol
graesslin requested changes to this revision. graesslin added a comment. This revision now requires changes to proceed. Overall lots of documentation is missing. From API layout that looks much better now, but we still have a few places where "Unstable" is needlessly in the API. Most important in Display the create method is not forward compatible by not having an enum to describe which interface version should be created. INLINE COMMENTS > registry.h:527-528 > + > +zxdg_exporter_v1 *bindXdgExporterUnstableV1(uint32_t name, uint32_t > version) const; > +zxdg_importer_v1 *bindXdgImporterUnstableV1(uint32_t name, uint32_t > version) const; > ///@} please add documentation > registry.h:940-941 > + > +XdgExporter *createXdgExporterUnstable(quint32 name, quint32 version, > QObject *parent = nullptr); > +XdgImporter *createXdgImporterUnstable(quint32 name, quint32 version, > QObject *parent = nullptr); > ///@} please add documentation > registry.h:940-941 > + > +XdgExporter *createXdgExporterUnstable(quint32 name, quint32 version, > QObject *parent = nullptr); > +XdgImporter *createXdgImporterUnstable(quint32 name, quint32 version, > QObject *parent = nullptr); > ///@} Why did you keep the suffix "Unstable"? > registry.h:1132-1133 > + > +void exporterUnstableV1Announced(quint32 name, quint32 version); > +void importerUnstableV1Announced(quint32 name, quint32 version); > ///@} please add documentation > registry.h:1288-1290 > +void exporterUnstableV1Removed(quint32 name); > +void importerUnstableV1Removed(quint32 name); > ///@} please add documentation > display.h:223 > > +XdgForeignInterface *createXdgForeignUnstableInterface(QObject *parent = > nullptr); > /** please add documentation. Also the common way would be to have an enum to describe which interface should be created, but therefore drop the Unstable from the API name. > xdgforeign_interface.h:37 > +class XdgImporterUnstableV1Interface; > + > +class KWAYLANDSERVER_EXPORT XdgForeignInterface : public QObject Something like an enum ForeignInterfaceVersion is missing, please compare TextInputInterface > xdgforeign_v1_interface.cpp:260 > + > +QPointer imp = new > XdgImportedUnstableV1Interface(s->q, surface); > +imp->create(s->display->getConnection(client), > wl_resource_get_version(resource), id); what's an imp? Could you please write full variable names? REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7369 To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D7369: Wayland foreign protocol
mart retitled this revision from "[WIP] Wayland foreign protocol" to "Wayland foreign protocol". REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7369 To: mart, #plasma, #kwin, davidedmundson Cc: davidedmundson, graesslin, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
