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
