D7369: [WIP] Wayland foreign protocol

David Edmundson noreply at phabricator.kde.org
Thu Aug 17 16:40:11 UTC 2017


davidedmundson requested changes to this revision.
davidedmundson added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> xdgforeign_v1.cpp:189
> +    return p;
> +    //Dave: there's no point creating a wrapper and then returning the low level struct.
> +    //IMHO we could return the surfaceId directly here, and not make wrappers for exported/imported.

Some idiot left a comment all over your code after fixing it.

We don't do the thing I was saying about anymore.

> xdgforeign_v1_interface.cpp:299
> +    //surface no longer exported
> +    connect(exp, &QObject::destroyed,
> +            s->q, [imp]() {

this should be connected to unbound.

> xdgforeign_v1_interface.cpp:307
> +            s->q, [s, imp, importedSI](SurfaceInterface *child) {
> +                //remove any previous association
> +                auto it = s->children.find(importedSI);

I don't think this is right.

Second half of this line:

A surface may be exported multiple times, and each exported handle may be used to create a xdg_imported multiple times.

> xdgforeign_v1_interface.cpp:497
> +
> +XdgImportedUnstableV1Interface::Private::~Private()
> +{

the super class destructor (Resource::Private::~Private) does this

> xdgforeign_v1_interface.h:51
> +Q_SIGNALS:
> +    void transientChanged(KWayland::Server::SurfaceInterface *child, KWayland::Server::SurfaceInterface *parent);
> +

docs, especially on what either argument being null means.

REPOSITORY
  R127 KWayland

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

To: mart, #plasma, #kwin, davidedmundson
Cc: davidedmundson, graesslin, plasma-devel, #frameworks, leezu, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein, lukas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170817/51ba2b38/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list