[PATCH weston 3/6] clients/simple-shm: Port to xdg_shell unstable v6

[Quentin Glidic]

On 12/08/2016 05:40, Jonas Ådahl wrote:
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>

One concern (in two places) below.


> ---
>  Makefile.am          |  4 +--
>  clients/simple-shm.c | 93 ++++++++++++++++++++++++++++++++--------------------
>  2 files changed, 60 insertions(+), 37 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index dcd0a25..eb093b8 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -519,8 +519,8 @@ demo_clients +=					\
>
>  weston_simple_shm_SOURCES = clients/simple-shm.c
>  nodist_weston_simple_shm_SOURCES =		\
> -	protocol/xdg-shell-unstable-v5-protocol.c		\
> -	protocol/xdg-shell-unstable-v5-client-protocol.h	\
> +	protocol/xdg-shell-unstable-v6-protocol.c		\
> +	protocol/xdg-shell-unstable-v6-client-protocol.h	\
>  	protocol/fullscreen-shell-unstable-v1-protocol.c	\
>  	protocol/fullscreen-shell-unstable-v1-client-protocol.h	\
>  	protocol/ivi-application-protocol.c		\
> diff --git a/clients/simple-shm.c b/clients/simple-shm.c
> index b1fc5f1..9fa2e21 100644
> --- a/clients/simple-shm.c
> +++ b/clients/simple-shm.c
> @@ -37,7 +37,7 @@
>  #include <wayland-client.h>
>  #include "shared/os-compatibility.h"
>  #include "shared/zalloc.h"
> -#include "xdg-shell-unstable-v5-client-protocol.h"
> +#include "xdg-shell-unstable-v6-client-protocol.h"
>  #include "fullscreen-shell-unstable-v1-client-protocol.h"
>
>  #include <sys/types.h>
> @@ -48,7 +48,7 @@ struct display {
>  	struct wl_display *display;
>  	struct wl_registry *registry;
>  	struct wl_compositor *compositor;
> -	struct xdg_shell *shell;
> +	struct zxdg_shell_v6 *shell;
>  	struct zwp_fullscreen_shell_v1 *fshell;
>  	struct wl_shm *shm;
>  	bool has_xrgb;
> @@ -65,16 +65,21 @@ struct window {
>  	struct display *display;
>  	int width, height;
>  	struct wl_surface *surface;
> -	struct xdg_surface *xdg_surface;
> +	struct zxdg_surface_v6 *xdg_surface;
> +	struct zxdg_toplevel_v6 *xdg_toplevel;
>  	struct ivi_surface *ivi_surface;
>  	struct buffer buffers[2];
>  	struct buffer *prev_buffer;
>  	struct wl_callback *callback;
> +	bool wait_for_configure;
>  };
>
>  static int running = 1;
>
>  static void
> +redraw(void *data, struct wl_callback *callback, uint32_t time);
> +
> +static void
>  buffer_release(void *data, struct wl_buffer *buffer)
>  {
>  	struct buffer *mybuf = data;
> @@ -125,22 +130,39 @@ create_shm_buffer(struct display *display, struct buffer *buffer,
>  }
>
>  static void
> -handle_configure(void *data, struct xdg_surface *surface,
> -		 int32_t width, int32_t height,
> -		 struct wl_array *states, uint32_t serial)
> +handle_xdg_surface_configure(void *data, struct zxdg_surface_v6 *surface,
> +			     uint32_t serial)
>  {
> -	xdg_surface_ack_configure(surface, serial);
> +	struct window *window = data;
> +
> +	zxdg_surface_v6_ack_configure(surface, serial);
> +
> +	if (window->wait_for_configure) {
> +		redraw(window, NULL, 0);
> +		window->wait_for_configure = false;
> +	}

So you trigger a redraw (and a frame callback, which means a full series 
of redraws) on configure. Ok. Now below…


>  }
>
> +static const struct zxdg_surface_v6_listener xdg_surface_listener = {
> +	handle_xdg_surface_configure,
> +};
> +
>  static void
> -handle_delete(void *data, struct xdg_surface *xdg_surface)
> +handle_xdg_toplevel_configure(void *data, struct zxdg_toplevel_v6 *xdg_toplevel,
> +			      int32_t width, int32_t height,
> +			      struct wl_array *state)
> +{
> +}
> +
> +static void
> +handle_xdg_toplevel_close(void *data, struct zxdg_toplevel_v6 *xdg_toplevel)
>  {
>  	running = 0;
>  }
>
> -static const struct xdg_surface_listener xdg_surface_listener = {
> -	handle_configure,
> -	handle_delete,
> +static const struct zxdg_toplevel_v6_listener xdg_toplevel_listener = {
> +	handle_xdg_toplevel_configure,
> +	handle_xdg_toplevel_close,
>  };
>
>  static void
> @@ -171,16 +193,21 @@ create_window(struct display *display, int width, int height)
>
>  	if (display->shell) {
>  		window->xdg_surface =
> -			xdg_shell_get_xdg_surface(display->shell,
> +			zxdg_shell_v6_get_xdg_surface(display->shell,
>  						  window->surface);
> -
>  		assert(window->xdg_surface);
> -
> -		xdg_surface_add_listener(window->xdg_surface,
> -					 &xdg_surface_listener, window);
> -
> -		xdg_surface_set_title(window->xdg_surface, "simple-shm");
> -
> +		zxdg_surface_v6_add_listener(window->xdg_surface,
> +					     &xdg_surface_listener, window);
> +
> +		window->xdg_toplevel =
> +			zxdg_surface_v6_get_toplevel(window->xdg_surface);
> +		assert(window->xdg_toplevel);
> +		zxdg_toplevel_v6_add_listener(window->xdg_toplevel,
> +					      &xdg_toplevel_listener, window);
> +
> +		zxdg_toplevel_v6_set_title(window->xdg_toplevel, "simple-shm");
> +		wl_surface_commit(window->surface);
> +		window->wait_for_configure = true;
>  	} else if (display->fshell) {
>  		zwp_fullscreen_shell_v1_present_surface(display->fshell,
>  							window->surface,
> @@ -217,8 +244,10 @@ destroy_window(struct window *window)
>  	if (window->buffers[1].buffer)
>  		wl_buffer_destroy(window->buffers[1].buffer);
>
> +	if (window->xdg_toplevel)
> +		zxdg_toplevel_v6_destroy(window->xdg_toplevel);
>  	if (window->xdg_surface)
> -		xdg_surface_destroy(window->xdg_surface);
> +		zxdg_surface_v6_destroy(window->xdg_surface);
>  	wl_surface_destroy(window->surface);
>  	free(window);
>  }
> @@ -347,21 +376,15 @@ struct wl_shm_listener shm_listener = {
>  };
>
>  static void
> -xdg_shell_ping(void *data, struct xdg_shell *shell, uint32_t serial)
> +xdg_shell_ping(void *data, struct zxdg_shell_v6 *shell, uint32_t serial)
>  {
> -	xdg_shell_pong(shell, serial);
> +	zxdg_shell_v6_pong(shell, serial);
>  }
>
> -static const struct xdg_shell_listener xdg_shell_listener = {
> +static const struct zxdg_shell_v6_listener xdg_shell_listener = {
>  	xdg_shell_ping,
>  };
>
> -#define XDG_VERSION 5 /* The version of xdg-shell that we implement */
> -#ifdef static_assert
> -static_assert(XDG_VERSION == XDG_SHELL_VERSION_CURRENT,
> -	      "Interface version doesn't match implementation version");
> -#endif
> -
>  static void
>  registry_handle_global(void *data, struct wl_registry *registry,
>  		       uint32_t id, const char *interface, uint32_t version)
> @@ -372,11 +395,10 @@ registry_handle_global(void *data, struct wl_registry *registry,
>  		d->compositor =
>  			wl_registry_bind(registry,
>  					 id, &wl_compositor_interface, 1);
> -	} else if (strcmp(interface, "xdg_shell") == 0) {
> +	} else if (strcmp(interface, "zxdg_shell_v6") == 0) {
>  		d->shell = wl_registry_bind(registry,
> -					    id, &xdg_shell_interface, 1);
> -		xdg_shell_use_unstable_version(d->shell, XDG_VERSION);
> -		xdg_shell_add_listener(d->shell, &xdg_shell_listener, d);
> +					    id, &zxdg_shell_v6_interface, 1);
> +		zxdg_shell_v6_add_listener(d->shell, &xdg_shell_listener, d);
>  	} else if (strcmp(interface, "zwp_fullscreen_shell_v1") == 0) {
>  		d->fshell = wl_registry_bind(registry,
>  					     id, &zwp_fullscreen_shell_v1_interface, 1);
> @@ -483,7 +505,7 @@ destroy_display(struct display *display)
>  		wl_shm_destroy(display->shm);
>
>  	if (display->shell)
> -		xdg_shell_destroy(display->shell);
> +		zxdg_shell_v6_destroy(display->shell);
>
>  	if (display->fshell)
>  		zwp_fullscreen_shell_v1_release(display->fshell);
> @@ -525,7 +547,8 @@ main(int argc, char **argv)
>  	wl_surface_damage(window->surface, 0, 0,
>  			  window->width, window->height);
>
> -	redraw(window, NULL, 0);
> +	if (!window->wait_for_configure)
> +		redraw(window, NULL, 0);

… here. You trigger a redraw (chain) if configure was received, but in 
this case, the redraw is already triggered.
I guess you used the same code for all simple-*, except this one lacks 
(or don’t need) the "initialized" check.


>  	while (running && ret != -1)
>  		ret = wl_display_dispatch(display->display);
>


-- 

Quentin “Sardem FF7” Glidic