| romangg |
| KWin |
This adds XDG WM Base (essentially XDG Shell v7/stable edition) into our
existing XDGShell classes which wrap v5, v6 and now this.
It's mostly copy and paste from V6 except for the enum types for gravity
and anchor edges on positioners.
There's been no attempt to share code with V6 as realistically that
won't get updates whereas XDGWMBase will; and at some point we will
want to drop V6 without things being too tangled.
Same test suite as V6 has
Compiled GTK master and ran against suitably modified kwin
running WAYLAND_DEBUG=1 gtk-demo showed we were using this interface
Everything worked as well as V6 does.
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
| autotests/client/test_xdg_shell.cpp | ||
|---|---|---|
| 120 | Missing default. Is it okay if version is unknown? | |
| src/client/xdgshell_stable.cpp | ||
| 29 | Seems like QDebug is not used here. | |
| 308 | Use static_cast. Same down below. | |
| 337 | Missing default. Is it okay if the state is unknown? | |
| 526 | Shouldn't it be also static? | |
| 537 | Nitpick: no semicolons after Q_UNUSED. | |
| src/server/xdgshell_stable_interface.cpp | ||
| 76 | I would also add override, e.g. ~Private() override; | |
| 243–244 | Which one should return? | |
| 489–494 | Can be simplified, e.g. if (!parentXdgSurface) { wl_resource_post_error(parentResource, XDG_WM_BASE_ERROR_INVALID_POPUP_PARENT, "Invalid popup parent"); return; } pd->parent = parentXdgSurface->surface(); pd->.... | |
| src/client/xdgshell_stable.cpp | ||
|---|---|---|
| 526 | Ignore it. | |
| src/client/registry.cpp | ||
|---|---|---|
| 89 | nitpick: put this include directly below the other xdg-shell ones above. | |
| src/client/xdgshell_stable.cpp | ||
| 196 | = XDG_POSITIONER_CONSTRAINT_ADJUSTMENT_NONE | |
| src/server/xdgshell_interface.h | ||
| 64 | Is this the natural evolution of all protocols? In the end they become you? | |
| src/server/xdgshell_stable_interface.cpp | ||
| 80 | not according to specs | |
| 161 | Could you pls name this better: for example wlStates | |
| 223 | Do it? | |
| 247 | Wrong id XDG_WM_BASE_ERROR_ROLE. | |
| src/server/xdgshell_stable_interface.cpp | ||
|---|---|---|
| 247 | What role do you think it should be? I'm using the error for when a wl_surface has another role in the case that you call get_xdg_surface twice on the same wl_surface, which seems close enough. | |
| src/server/xdgshell_stable_interface.cpp | ||
|---|---|---|
| 247 | XDG_SURFACE_ERROR_ALREADY_CONSTRUCTED | |
| src/client/xdgshell_stable.cpp | ||
|---|---|---|
| 306 | Did you forget to change it to static_cast? Or there is a reason why it's still reinterpret_cast? | |
XDG_SURFACE_ERROR_ALREADY_CONSTRUCTED
I'm not sure. There's an implication that XDG_SURFACE errors are meant for in response to requests on an xdg_surface interface, (i.e for use with xdg_surface.get_toplevel xdg_surface.get_popup) whereas this is a callback on the wm_base interface.
From my side I can't see anything wrong right now. Since we would merge after the next release anyways we have some time to correct any overlooked mistakes afterwards. Or would it make sense to wait on merging until Qt 5.12 with XDG WM Base support is available to have more test candidates?
| src/client/xdgshell_stable.cpp | ||
|---|---|---|
| 3 | 2018 - other files below as well. | |
Or would it make sense to wait on merging until Qt 5.12 with XDG WM Base support is available to have more test candidates?
I don't think it's needed. The protocol is 90% the same it's mostly just awkward name changes. Same on the Qt side.
We haven't changed public API here either, other than a new enum value, so absolutely worst case we can just revert the change in KWin in the unlikely event that we see problems.