D13510: Add XDG WM Base support to our XDGShell API

Roman Gilg noreply at phabricator.kde.org
Thu Jun 14 23:39:33 UTC 2018


romangg added inline comments.

INLINE COMMENTS

> registry.cpp:88
>  #include <wayland-xdg-output-unstable-v1-client-protocol.h>
> +#include <wayland-xdg-shell-client-protocol.h>
>  

nitpick: put this include directly below the other xdg-shell ones above.

> xdgshell_stable.cpp:195
> +
> +    uint32_t constraint = 0;
> +    if (positioner.constraints().testFlag(XdgPositioner::Constraint::SlideX)) {

`= XDG_POSITIONER_CONSTRAINT_ADJUSTMENT_NONE`

> xdgshell_interface.h:64
> +    Stable,
> +    Dave
>  };

Is this the natural evolution of all protocols? In the end they become you?

> xdgshell_stable_interface.cpp:79
> +        if (!configureSerials.contains(serial)) {
> +            // TODO: send error?
> +            return;

not according to specs

> xdgshell_stable_interface.cpp:160
> +        const quint32 serial = global->display()->nextSerial();
> +        wl_array state;
> +        wl_array_init(&state);

Could you pls name this better: for example wlStates

> xdgshell_stable_interface.cpp:222
> +    Q_UNUSED(client)
> +    // TODO: send protocol error if there are still surfaces mapped
> +    wl_resource_destroy(resource);

Do it?

> xdgshell_stable_interface.cpp:246
> +    if (it != surfaces.constEnd()) {
> +        wl_resource_post_error(surface->resource(), XDG_WM_BASE_ERROR_ROLE, "ShellSurface already created");
> +        return;

Wrong id `XDG_WM_BASE_ERROR_ROLE`.

REPOSITORY
  R127 KWayland

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

To: davidedmundson, #kwin
Cc: romangg, zzag, kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180614/a6da1704/attachment.html>


More information about the Kde-frameworks-devel mailing list