[PATCH weston 08/12 v2] weston: Port Wayland backend to new output handling API
Pekka Paalanen
ppaalanen at gmail.com
Wed Aug 17 15:07:55 UTC 2016
On Sun, 14 Aug 2016 17:28:17 +0200
Armin Krezović <krezovic.armin at gmail.com> wrote:
> This is a complete port of the Wayland backend that
> uses recently added output handling API for output
> configuration.
>
> - Output can be configured at runtime by passing the
> necessary configuration parameters, which can be
> filled in manually, obtained from the configuration
> file or obtained from the command line using
> previously added functionality. It is required that
> the scale and transform values are set using the
> previously added functionality.
>
> - Output can be created at runtime using the output
> API. The output creation only creates a pending
> output, which needs to be configured the same way as
> mentioned above.
>
> However, the backend can behave both as windowed backend
> and as a backend that issues "hotplug" events, when
> running under fullscreen shell or with --sprawl command
> line option. The first case was covered by reusing
> previously added functionality. The second case required
> another API to be introduced and implemented into both
> the backend and compositor for handling output setup.
>
> After everything has been set, output needs to be
> enabled manually using weston_output_enable().
>
> v2:
>
> - Fix wet_configure_windowed_output_from_config() usage.
> - Call wayland_output_disable() explicitly from
> wayland_output_destroy().
>
> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
> ---
> compositor/main.c | 275 +++++++++++++++-------------------
> libweston/compositor-wayland.c | 327 +++++++++++++++++++++++------------------
> libweston/compositor-wayland.h | 27 +++-
> 3 files changed, 325 insertions(+), 304 deletions(-)
>
> diff --git a/compositor/main.c b/compositor/main.c
> index a72c2f9..143ee21 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -1568,218 +1568,177 @@ out:
> }
>
> static void
> -weston_wayland_output_config_init(struct weston_wayland_backend_output_config *output_config,
> - struct weston_config_section *config_section,
> - int option_width, int option_height,
> - int option_scale)
> +wayland_backend_output_configure_hotplug(struct wl_listener *listener, void *data)
This new function looks like it matches the old behaviour, good.
> {
> - char *mode, *t, *str;
> - unsigned int slen;
> -
> - weston_config_section_get_string(config_section, "name", &output_config->name,
> - NULL);
> - if (output_config->name) {
> - slen = strlen(output_config->name);
> - slen += strlen(WINDOW_TITLE " - ");
> - str = malloc(slen + 1);
> - if (str)
> - snprintf(str, slen + 1, WINDOW_TITLE " - %s",
> - output_config->name);
> - free(output_config->name);
> - output_config->name = str;
> - }
> - if (!output_config->name)
> - output_config->name = strdup(WINDOW_TITLE);
> -
> - weston_config_section_get_string(config_section,
> - "mode", &mode, "1024x600");
> - if (sscanf(mode, "%dx%d", &output_config->width, &output_config->height) != 2) {
> - weston_log("Invalid mode \"%s\" for output %s\n",
> - mode, output_config->name);
> - output_config->width = 1024;
> - output_config->height = 640;
> - }
> - free(mode);
> + struct weston_output *output = data;
>
> - if (option_width)
> - output_config->width = option_width;
> - if (option_height)
> - output_config->height = option_height;
> + const struct weston_wayland_output_api *api =
> + weston_wayland_output_get_api(output->compositor);
>
> - weston_config_section_get_int(config_section, "scale", &output_config->scale, 1);
> + if (!api) {
> + weston_log("Cannot use weston_wayland_output_api.\n");
> + return;
> + }
>
> - if (option_scale)
> - output_config->scale = option_scale;
> + weston_output_set_scale(output, 1);
> + weston_output_set_transform(output, WL_OUTPUT_TRANSFORM_NORMAL);
>
> - weston_config_section_get_string(config_section,
> - "transform", &t, "normal");
> - if (weston_parse_transform(t, &output_config->transform) < 0)
> - weston_log("Invalid transform \"%s\" for output %s\n",
> - t, output_config->name);
> - free(t);
> + if (api->output_configure(output) < 0) {
> + weston_log("Cannot configure an output using weston_wayland_output_api.\n");
> + return;
> + }
>
> + weston_output_enable(output);
> }
>
> static void
> -weston_wayland_backend_config_release(struct weston_wayland_backend_config *config)
> -{
> - int i;
> -
> - for (i = 0; i < config->num_outputs; ++i) {
> - free(config->outputs[i].name);
> - }
> - free(config->cursor_theme);
> - free(config->display_name);
> - free(config->outputs);
> -}
> -
> -/*
> - * Append a new output struct at the end of new_config.outputs and return a
> - * pointer to the newly allocated structure or NULL if fail. The allocated
> - * structure is NOT cleared nor set to default values.
> - */
> -static struct weston_wayland_backend_output_config *
> -weston_wayland_backend_config_add_new_output(struct weston_wayland_backend_config *config)
> +wayland_backend_output_configure(struct wl_listener *listener, void *data)
> {
> - struct weston_wayland_backend_output_config *outputs;
> - const size_t element_size = sizeof(struct weston_wayland_backend_output_config);
> + struct weston_output *output = data;
> + struct wet_output_config defaults = {
> + .width = 1024,
> + .height = 640,
> + .scale = 1,
> + .transform = WL_OUTPUT_TRANSFORM_NORMAL
> + };
>
> - outputs = realloc(config->outputs,
> - (config->num_outputs + 1) * element_size);
> - if (!outputs)
> - return NULL;
> - config->num_outputs += 1;
> - config->outputs = outputs;
> - return &(config->outputs[config->num_outputs - 1]);
> + if (wet_configure_windowed_output_from_config(output, &defaults) < 0)
> + weston_log("Cannot configure output \"%s\".\n",
> + output->name ? output->name : "unnamed");
I think wayland_backend_output_configure() is missing a special case
for --fullscreen. That is, desktop shell (windowed api) but asked to be
fullscreen...
Actually no, this is fine. It would just need a comment explaining that
the backend will override the size if --fullscreen was given. I see
this would be inconvenient to realize otherwise.
> }
>
> static int
> -load_wayland_backend_config(struct weston_compositor *compositor, int *argc,
> - char *argv[], struct weston_config *wc,
> - struct weston_wayland_backend_config *config)
> +load_wayland_backend(struct weston_compositor *c,
> + int *argc, char **argv, struct weston_config *wc)
The new load_wayland_backend() looks quite clean, in spite of some
mixed code and declarations issue, compared to the old
load_wayland_backend_config(). That's nice.
> {
> + struct weston_wayland_backend_config config = {{ 0, }};
> struct weston_config_section *section;
> - struct weston_wayland_backend_output_config *oc;
> - int count, width, height, scale;
> + const struct weston_windowed_output_api *api;
> + const struct weston_wayland_output_api *wl_api;
> + int count = 1;
> + int ret = 0;
> const char *section_name;
> - char *name;
> +
> + struct wet_output_config *parsed_options = wet_init_parsed_options(c);
> + if (!parsed_options)
> + return -1;
> +
> + config.cursor_size = 32;
> + config.cursor_theme = NULL;
> + config.display_name = NULL;
> + config.fullscreen = 0;
> + config.sprawl = 0;
> + config.use_pixman = 0;
>
> const struct weston_option wayland_options[] = {
> - { WESTON_OPTION_INTEGER, "width", 0, &width },
> - { WESTON_OPTION_INTEGER, "height", 0, &height },
> - { WESTON_OPTION_INTEGER, "scale", 0, &scale },
> - { WESTON_OPTION_STRING, "display", 0, &config->display_name },
> - { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config->use_pixman },
> + { WESTON_OPTION_INTEGER, "width", 0, &parsed_options->width },
> + { WESTON_OPTION_INTEGER, "height", 0, &parsed_options->height },
> + { WESTON_OPTION_INTEGER, "scale", 0, &parsed_options->scale },
> + { WESTON_OPTION_STRING, "display", 0, &config.display_name },
> + { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config.use_pixman },
> { WESTON_OPTION_INTEGER, "output-count", 0, &count },
> - { WESTON_OPTION_BOOLEAN, "fullscreen", 0, &config->fullscreen },
> - { WESTON_OPTION_BOOLEAN, "sprawl", 0, &config->sprawl },
> + { WESTON_OPTION_BOOLEAN, "fullscreen", 0, &config.fullscreen },
> + { WESTON_OPTION_BOOLEAN, "sprawl", 0, &config.sprawl },
> };
>
> - width = 0;
> - height = 0;
> - scale = 0;
> - config->display_name = NULL;
> - config->use_pixman = 0;
> - count = 1;
> - config->fullscreen = 0;
> - config->sprawl = 0;
> - parse_options(wayland_options,
> - ARRAY_LENGTH(wayland_options), argc, argv);
> -
> - config->cursor_size = 32;
> - config->cursor_theme = NULL;
> - config->base.struct_size = sizeof(struct weston_wayland_backend_config);
> - config->base.struct_version = WESTON_WAYLAND_BACKEND_CONFIG_VERSION;
> + parse_options(wayland_options, ARRAY_LENGTH(wayland_options), argc, argv);
>
> section = weston_config_get_section(wc, "shell", NULL, NULL);
> weston_config_section_get_string(section, "cursor-theme",
> - &config->cursor_theme, NULL);
> + &config.cursor_theme, NULL);
> weston_config_section_get_int(section, "cursor-size",
> - &config->cursor_size, 32);
> + &config.cursor_size, 32);
> +
> + config.base.struct_size = sizeof(struct weston_wayland_backend_config);
> + config.base.struct_version = WESTON_WAYLAND_BACKEND_CONFIG_VERSION;
> +
> + /* load the actual wayland backend and configure it */
> + ret = weston_compositor_load_backend(c, WESTON_BACKEND_WAYLAND,
> + &config.base);
> +
> + free(config.cursor_theme);
> + free(config.display_name);
> +
> + if (ret < 0)
> + goto err;
> +
> + api = weston_windowed_output_get_api(c);
> +
> + if (api == NULL) {
> + /* Try wayland API */
> + wl_api = weston_wayland_output_get_api(c);
> +
> + if (wl_api == NULL) {
> + weston_log("Cannot bind to any output configuration API.\n");
> + ret = -1;
> + goto err;
> + }
> +
> + wet_set_pending_output_handler(c, wayland_backend_output_configure_hotplug);
>
> - if (config->sprawl) {
> - /* do nothing, everything is already set */
> return 0;
> }
>
> - if (config->fullscreen) {
> - oc = weston_wayland_backend_config_add_new_output(config);
> - if (!oc)
> - return -1;
> + wet_set_pending_output_handler(c, wayland_backend_output_configure);
>
> - oc->width = width;
> - oc->height = height;
> - oc->name = NULL;
> - oc->transform = WL_OUTPUT_TRANSFORM_NORMAL;
> - oc->scale = 1;
> + if (!api) {
> + weston_log("Cannot use weston_windowed_output_api.\n");
> + ret = -1;
> + goto err;
> + }
>
> + if (config.fullscreen) {
> + if (api->output_create(c, WINDOW_TITLE) < 0) {
> + ret = -1;
> + goto err;
> + }
> return 0;
> }
>
> section = NULL;
> while (weston_config_next_section(wc, §ion, §ion_name)) {
> - if (!section_name || strcmp(section_name, "output") != 0)
> + char *output_name;
> +
> + if (count == 0)
> + break;
> +
> + if (strcmp(section_name, "output") != 0) {
> continue;
> - weston_config_section_get_string(section, "name", &name, NULL);
> - if (name == NULL)
> + }
> +
> + weston_config_section_get_string(section, "name", &output_name, NULL);
> +
> + if (output_name == NULL)
> continue;
>
> - if (name[0] != 'W' || name[1] != 'L') {
> - free(name);
> + if (output_name[0] != 'W' || output_name[1] != 'L') {
> + free(output_name);
> continue;
> }
> - free(name);
>
> - oc = weston_wayland_backend_config_add_new_output(config);
> - if (!oc)
> - return -1;
> + if (api->output_create(c, output_name) < 0) {
> + ret = -1;
> + free(output_name);
> + goto err;
> + }
> + free(output_name);
>
> - weston_wayland_output_config_init(oc, section, width,
> - height, scale);
> --count;
> }
>
> - if (!width)
> - width = 1024;
> - if (!height)
> - height = 640;
> - if (!scale)
> - scale = 1;
> -
> while (count > 0) {
> - oc = weston_wayland_backend_config_add_new_output(config);
> - if (!oc)
> - return -1;
> -
> - oc->width = width;
> - oc->height = height;
> - oc->name = NULL;
> - oc->transform = WL_OUTPUT_TRANSFORM_NORMAL;
> - oc->scale = scale;
> + if (api->output_create(c, WINDOW_TITLE) < 0) {
I don't like too much about creating several outputs with the same
name, but considering they previously had no name at all, it's ok.
Ideally all outputs would be uniquely named "WL#" with a number.
Looks like there is some confusion in output names vs. window title
left over from the previous restructuring, but that's an issue for
another time. Output name should not include WINDOW_TITLE. The Wayland
backend will compose the window title from WINDOW_TITLE and output name.
> + ret = -1;
> + goto err;
> + }
>
> --count;
> }
>
> return 0;
> -}
> +err:
> + free(parsed_options);
wet_init_parsed_options() put the pointer also into
wet_compositor::parsed_options. After this free, that pointer will be
stale.
>
> -static int
> -load_wayland_backend(struct weston_compositor *c,
> - int *argc, char **argv, struct weston_config *wc)
> -{
> - struct weston_wayland_backend_config config = {{ 0, }};
> - int ret = 0;
> -
> - ret = load_wayland_backend_config(c, argc, argv, wc, &config);
> - if (ret < 0) {
> - weston_wayland_backend_config_release(&config);
> - return ret;
> - }
> -
> - /* load the actual wayland backend and configure it */
> - ret = weston_compositor_load_backend(c, WESTON_BACKEND_WAYLAND,
> - &config.base);
> - weston_wayland_backend_config_release(&config);
> return ret;
> }
>
> diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
> index 7c12b4c..4b0113f 100644
> --- a/libweston/compositor-wayland.c
> +++ b/libweston/compositor-wayland.c
> @@ -51,6 +51,7 @@
> #include "fullscreen-shell-unstable-v1-client-protocol.h"
> #include "presentation-time-server-protocol.h"
> #include "linux-dmabuf.h"
> +#include "windowed-output-api.h"
>
> #define WINDOW_TITLE "Weston Compositor"
>
> @@ -74,6 +75,7 @@ struct wayland_backend {
>
> int use_pixman;
> int sprawl_across_outputs;
> + int fullscreen;
>
> struct theme *theme;
> cairo_device_t *frame_device;
> @@ -119,6 +121,9 @@ struct wayland_output {
>
> struct weston_mode mode;
> uint32_t scale;
> +
> + struct weston_mode *poutput_mode;
Hmm, can't you use base.current_mode instead of adding this?
> + void *user_data;
This is an internal struct not exposed to any other component, so
seeing "user_data" is strange. Can't you use the proper pointer type
with a descriptive name?
Seems like it is only ever used to hold a pointer to a parent output,
so it should be:
struct wayland_parent_output *parent_output;
> };
>
> struct wayland_parent_output {
> @@ -616,21 +621,24 @@ wayland_output_repaint_pixman(struct weston_output *output_base,
> return 0;
> }
>
> -static void
> -wayland_output_destroy(struct weston_output *output_base)
> +static int
> +wayland_output_disable(struct weston_output *base)
> {
> - struct wayland_output *output = to_wayland_output(output_base);
> - struct wayland_backend *b =
> - to_wayland_backend(output->base.compositor);
> + struct wayland_output *output = to_wayland_output(base);
> + struct wayland_backend *b = to_wayland_backend(base->compositor);
> +
> + if (!output->base.enabled)
> + return 0;
>
> if (b->use_pixman) {
> - pixman_renderer_output_destroy(output_base);
> + pixman_renderer_output_destroy(&output->base);
> } else {
> - gl_renderer->output_destroy(output_base);
> + gl_renderer->output_destroy(&output->base);
> }
>
> wl_egl_window_destroy(output->gl.egl_window);
> wl_surface_destroy(output->parent.surface);
> +
> if (output->parent.shell_surface)
> wl_shell_surface_destroy(output->parent.shell_surface);
>
> @@ -642,10 +650,18 @@ wayland_output_destroy(struct weston_output *output_base)
> cairo_surface_destroy(output->gl.border.right);
> cairo_surface_destroy(output->gl.border.bottom);
Ehheh, looks like someone may have forgot to destroy shm.buffers and
shm.free_buffers on shutdown. Well, not your problem.
>
> + return 0;
> +}
> +
> +static void
> +wayland_output_destroy(struct weston_output *base)
> +{
> + struct wayland_output *output = to_wayland_output(base);
> +
> + wayland_output_disable(&output->base);
> weston_output_destroy(&output->base);
> - free(output);
>
> - return;
> + free(output);
> }
>
> static const struct wl_shell_surface_listener shell_surface_listener;
> @@ -1002,32 +1018,21 @@ err_output:
> return -1;
> }
>
> -static struct wayland_output *
> -wayland_output_create(struct wayland_backend *b, int x, int y,
> - int width, int height, const char *name, int fullscreen,
> - uint32_t transform, int32_t scale)
> +static int
> +wayland_output_enable(struct weston_output *base)
> {
> - struct wayland_output *output;
> - int output_width, output_height;
> + struct wayland_output *output = to_wayland_output(base);
> + struct wayland_backend *b = to_wayland_backend(base->compositor);
>
> weston_log("Creating %dx%d wayland output at (%d, %d)\n",
> - width, height, x, y);
> -
> - output = zalloc(sizeof *output);
> - if (output == NULL)
> - return NULL;
> -
> - output->name = name ? strdup(name) : NULL;
> - output->base.make = "wayland";
> - output->base.model = "none";
> -
> - output_width = width * scale;
> - output_height = height * scale;
> + output->base.mm_width, output->base.mm_height,
This used to be the logical size...
The old weston_output_init() has been called before we get here, so you
can dig up the logical size from weston_output::width and height IIRC.
> + output->base.x, output->base.y);
>
> output->parent.surface =
> wl_compositor_create_surface(b->parent.compositor);
> if (!output->parent.surface)
> - goto err_name;
> + return -1;
> +
> wl_surface_set_user_data(output->parent.surface, output);
>
> output->parent.draw_initial_frame = 1;
> @@ -1038,118 +1043,169 @@ wayland_output_create(struct wayland_backend *b, int x, int y,
> output->parent.surface);
> if (!output->parent.shell_surface)
> goto err_surface;
> +
> wl_shell_surface_add_listener(output->parent.shell_surface,
> &shell_surface_listener, output);
> }
>
> - if (fullscreen && b->parent.shell) {
> + if (!b->sprawl_across_outputs && b->fullscreen && b->parent.shell) {
Hmm, fullscreen was limited to exactly one output, so sprawl with it
does not make sense... I suppose this hunk is ok, but still looks a
little out of place in this patch. What prompted
adding !b->sprawl_across_outputs && ?
> wl_shell_surface_set_fullscreen(output->parent.shell_surface,
> 0, 0, NULL);
> wl_display_roundtrip(b->parent.wl_display);
> - if (!width)
> - output_width = output->parent.configure_width;
> - if (!height)
> - output_height = output->parent.configure_height;
> }
>
> - output->mode.flags =
> - WL_OUTPUT_MODE_CURRENT | WL_OUTPUT_MODE_PREFERRED;
> - output->mode.width = output_width;
> - output->mode.height = output_height;
> - output->mode.refresh = 60000;
> - output->scale = scale;
> - wl_list_init(&output->base.mode_list);
> - wl_list_insert(&output->base.mode_list, &output->mode.link);
> - output->base.current_mode = &output->mode;
> -
> wl_list_init(&output->shm.buffers);
> wl_list_init(&output->shm.free_buffers);
>
> - weston_output_init(&output->base, b->compositor, x, y, width, height,
> - transform, scale);
> -
> if (b->use_pixman) {
> if (wayland_output_init_pixman_renderer(output) < 0)
> goto err_output;
> - output->base.repaint = wayland_output_repaint_pixman;
> } else {
> if (wayland_output_init_gl_renderer(output) < 0)
> goto err_output;
> - output->base.repaint = wayland_output_repaint_gl;
> }
>
> - output->base.start_repaint_loop = wayland_output_start_repaint_loop;
> - output->base.destroy = wayland_output_destroy;
> - output->base.assign_planes = NULL;
> - output->base.set_backlight = NULL;
> - output->base.set_dpms = NULL;
> - output->base.switch_mode = wayland_output_switch_mode;
> -
> - weston_compositor_add_output(b->compositor, &output->base);
> + if (b->sprawl_across_outputs) {
Aha, sprawl_across_outputs is used to indicate the
weston_output_create_for_parent() case. It doesn't strike obvious to
me, though it does seem correct.
> + wayland_output_set_fullscreen(output,
> + WL_SHELL_SURFACE_FULLSCREEN_METHOD_DRIVER,
> + output->poutput_mode->refresh, output->parent.output);
> +
> + if (output->parent.shell_surface) {
> + wl_shell_surface_set_fullscreen(output->parent.shell_surface,
> + WL_SHELL_SURFACE_FULLSCREEN_METHOD_DRIVER,
> + output->poutput_mode->refresh, output->parent.output);
> + } else if (b->parent.fshell) {
> + zwp_fullscreen_shell_v1_present_surface(b->parent.fshell,
> + output->parent.surface,
> + ZWP_FULLSCREEN_SHELL_V1_PRESENT_METHOD_CENTER,
> + output->parent.output);
> + zwp_fullscreen_shell_mode_feedback_v1_destroy(
> + zwp_fullscreen_shell_v1_present_surface_for_mode(b->parent.fshell,
> + output->parent.surface,
> + output->parent.output,
> + output->poutput_mode->refresh));
> + }
All the above looks somehow very fishy, but I see that's how it already
was, so... not a problem for this time.
> + } else if (b->fullscreen) {
> + wayland_output_set_fullscreen(output, 0, 0, NULL);
> + } else {
> + wayland_output_set_windowed(output);
> + }
These look ok.
>
> - return output;
> + return 0;
>
> err_output:
> - weston_output_destroy(&output->base);
> if (output->parent.shell_surface)
> wl_shell_surface_destroy(output->parent.shell_surface);
> err_surface:
> wl_surface_destroy(output->parent.surface);
> -err_name:
> - free(output->name);
> -
> - /* FIXME: cleanup weston_output */
> - free(output);
>
> - return NULL;
> + return -1;
> }
>
> static struct wayland_output *
> -wayland_output_create_for_config(struct wayland_backend *b,
> - struct weston_wayland_backend_output_config *oc,
> - int fullscreen, int32_t x, int32_t y)
> +wayland_output_create_common(void)
> {
> struct wayland_output *output;
>
> - output = wayland_output_create(b, x, y, oc->width, oc->height, oc->name,
> - fullscreen, oc->transform, oc->scale);
> + output = zalloc(sizeof *output);
> + if (output == NULL) {
> + perror("zalloc");
> + return NULL;
> + }
> +
> + output->base.destroy = wayland_output_destroy;
> + output->base.disable = wayland_output_disable;
> + output->base.enable = wayland_output_enable;
>
> return output;
> }
>
> -static struct wayland_output *
> -wayland_output_create_for_parent_output(struct wayland_backend *b,
> - struct wayland_parent_output *poutput)
> +static int
> +wayland_output_create(struct weston_compositor *compositor, const char *name)
> {
> - struct wayland_output *output;
> - struct weston_mode *mode;
> - int32_t x;
> + struct wayland_output *output = wayland_output_create_common();
> +
> + output->base.name = name ? strdup(name) : NULL;
> +
> + weston_output_init_pending(&output->base, compositor);
> +
> + return 0;
> +}
> +
> +static int
> +wayland_output_configure(struct weston_output *base, int width, int height)
> +{
> + struct wayland_output *output = to_wayland_output(base);
> + struct wayland_backend *b = to_wayland_backend(base->compositor);
> +
> + int output_width, output_height;
> +
> + if (width < 1) {
> + weston_log("Invalid width \"%d\" for output %s\n",
> + width, output->base.name);
> + return -1;
> + }
> +
> + if (height < 1) {
> + weston_log("Invalid height \"%d\" for output %s\n",
> + height, output->base.name);
> + return -1;
> + }
> +
> + output_width = width * output->base.scale;
> + output_height = height * output->base.scale;
> +
> + output->mode.flags =
> + WL_OUTPUT_MODE_CURRENT | WL_OUTPUT_MODE_PREFERRED;
> +
> + output->mode.width = output_width;
> + output->mode.height = output_height;
> + output->mode.refresh = 60000;
> + output->scale = output->base.scale;
> + wl_list_init(&output->base.mode_list);
> + wl_list_insert(&output->base.mode_list, &output->mode.link);
> +
> + output->base.mm_width = width;
> + output->base.mm_height = height;
> +
> + if (b->use_pixman)
> + output->base.repaint = wayland_output_repaint_pixman;
> + else
> + output->base.repaint = wayland_output_repaint_gl;
> +
> + output->base.start_repaint_loop = wayland_output_start_repaint_loop;
> + output->base.assign_planes = NULL;
> + output->base.set_backlight = NULL;
> + output->base.set_dpms = NULL;
> + output->base.switch_mode = wayland_output_switch_mode;
> + output->base.current_mode = &output->mode;
> + output->base.make = "wayland";
> + output->base.model = "none";
> +
> + return 0;
> +}
> +
> +static int
> +wayland_output_configure_hotplug(struct weston_output *base)
> +{
> + struct wayland_output *output = to_wayland_output(base);
> + struct wayland_parent_output *poutput = output->user_data;
>
> if (poutput->current_mode) {
> - mode = poutput->current_mode;
> + output->poutput_mode = poutput->current_mode;
> } else if (poutput->preferred_mode) {
> - mode = poutput->preferred_mode;
> + output->poutput_mode = poutput->preferred_mode;
> } else if (!wl_list_empty(&poutput->mode_list)) {
> - mode = container_of(poutput->mode_list.next,
> - struct weston_mode, link);
> - } else {
> - weston_log("No valid modes found. Skipping output\n");
> - return NULL;
> - }
> -
> - if (!wl_list_empty(&b->compositor->output_list)) {
> - output = container_of(b->compositor->output_list.prev,
> - struct wayland_output, base.link);
> - x = output->base.x + output->base.current_mode->width;
> + output->poutput_mode = container_of(poutput->mode_list.next,
> + struct weston_mode, link);
> } else {
> - x = 0;
> + weston_log("No valid modes found. Cannot configure an output.\n");
> + return -1;
> }
>
> - output = wayland_output_create(b, x, 0, mode->width, mode->height,
> - NULL, 0,
> - WL_OUTPUT_TRANSFORM_NORMAL, 1);
> - if (!output)
> - return NULL;
> + if (wayland_output_configure(&output->base, output->poutput_mode->width,
> + output->poutput_mode->height) < 0)
If wayland_output_configure() multiplies by scale and applies
transformation, then passing mode->width and mode->height directly here
is wrong.
The "video mode" is given in output pixels, so it is already multiplied
by scale and rotated by transform.
I see the old code was equally wrong, particularly if the output was
rotated 90 degrees it would create a window with inverted aspect ratio.
Ok, so if this keeps the old broken behaviour, it's fine.
Testing the upstream x11-backend, it looks like we expect to put the
logical output size (in global coordinates) in weston.ini, and then
expect the scale and transform to be applied on that to get the output
resolution in output pixels.
> + return -1;
>
> output->parent.output = poutput->global;
>
> @@ -1159,27 +1215,20 @@ wayland_output_create_for_parent_output(struct wayland_backend *b,
> wl_list_insert_list(&output->base.mode_list, &poutput->mode_list);
> wl_list_init(&poutput->mode_list);
>
> - wayland_output_set_fullscreen(output,
> - WL_SHELL_SURFACE_FULLSCREEN_METHOD_DRIVER,
> - mode->refresh, poutput->global);
> + return 0;
> +}
>
> - if (output->parent.shell_surface) {
> - wl_shell_surface_set_fullscreen(output->parent.shell_surface,
> - WL_SHELL_SURFACE_FULLSCREEN_METHOD_DRIVER,
> - mode->refresh, poutput->global);
> - } else if (b->parent.fshell) {
> - zwp_fullscreen_shell_v1_present_surface(b->parent.fshell,
> - output->parent.surface,
> - ZWP_FULLSCREEN_SHELL_V1_PRESENT_METHOD_CENTER,
> - poutput->global);
> - zwp_fullscreen_shell_mode_feedback_v1_destroy(
> - zwp_fullscreen_shell_v1_present_surface_for_mode(b->parent.fshell,
> - output->parent.surface,
> - poutput->global,
> - mode->refresh));
> - }
> +static int
> +wayland_output_create_for_parent_output(struct wayland_backend *b,
> + struct wayland_parent_output *poutput)
> +{
> + struct wayland_output *output = wayland_output_create_common();
> + output->user_data = poutput;
> + output->base.name = NULL;
>
> - return output;
> + weston_output_init_pending(&output->base, b->compositor);
> +
> + return 0;
> }
>
> static void
> @@ -2211,6 +2260,7 @@ wayland_backend_create(struct weston_compositor *compositor,
> create_cursor(b, new_config);
>
> b->use_pixman = new_config->use_pixman;
> + b->fullscreen = new_config->fullscreen;
>
> if (!b->use_pixman) {
> gl_renderer = weston_load_module("gl-renderer.so",
> @@ -2284,6 +2334,15 @@ wayland_backend_destroy(struct wayland_backend *b)
> free(b);
> }
>
> +static const struct weston_windowed_output_api windowed_api = {
> + wayland_output_configure,
Rename to *_set_size.
> + wayland_output_create,
> +};
> +
> +static const struct weston_wayland_output_api wayland_api = {
> + wayland_output_configure_hotplug,
> +};
> +
> static void
> config_init_to_defaults(struct weston_wayland_backend_config *config)
> {
> @@ -2294,10 +2353,9 @@ backend_init(struct weston_compositor *compositor,
> struct weston_backend_config *config_base)
> {
> struct wayland_backend *b;
> - struct wayland_output *output;
> struct wayland_parent_output *poutput;
> struct weston_wayland_backend_config new_config;
> - int x, count;
> + int ret;
>
> if (config_base == NULL ||
> config_base->struct_version != WESTON_WAYLAND_BACKEND_CONFIG_VERSION ||
> @@ -2321,41 +2379,30 @@ backend_init(struct weston_compositor *compositor,
> wl_list_for_each(poutput, &b->parent.output_list, link)
> wayland_output_create_for_parent_output(b, poutput);
>
> - return 0;
> - }
> -
> - if (new_config.fullscreen) {
> - if (new_config.num_outputs != 1 || !new_config.outputs)
> - goto err_outputs;
> + ret = weston_plugin_api_register(compositor, WESTON_WAYLAND_OUTPUT_API_NAME,
> + &wayland_api, sizeof(wayland_api));
>
> - output = wayland_output_create_for_config(b,
> - &new_config.outputs[0],
> - 1, 0, 0);
> - if (!output)
> - goto err_outputs;
> + if (ret < 0) {
> + weston_log("Failed to register output API.\n");
> + wayland_backend_destroy(b);
> + return -1;
> + }
>
> - wayland_output_set_fullscreen(output, 0, 0, NULL);
> return 0;
> }
>
> - x = 0;
> - for (count = 0; count < new_config.num_outputs; ++count) {
> - output = wayland_output_create_for_config(b, &new_config.outputs[count],
> - 0, x, 0);
> - if (!output)
> - goto err_outputs;
> - if (wayland_output_set_windowed(output))
> - goto err_outputs;
> + ret = weston_plugin_api_register(compositor, WESTON_WINDOWED_OUTPUT_API_NAME,
> + &windowed_api, sizeof(windowed_api));
>
> - x += output->base.width;
> + if (ret < 0) {
> + weston_log("Failed to register output API.\n");
> + wayland_backend_destroy(b);
> + return -1;
> }
>
> +
> weston_compositor_add_key_binding(compositor, KEY_F,
> MODIFIER_CTRL | MODIFIER_ALT,
> fullscreen_binding, b);
> return 0;
> -
> -err_outputs:
> - wayland_backend_destroy(b);
> - return -1;
> }
> diff --git a/libweston/compositor-wayland.h b/libweston/compositor-wayland.h
> index b705dee..3164db6 100644
> --- a/libweston/compositor-wayland.h
> +++ b/libweston/compositor-wayland.h
> @@ -27,6 +27,7 @@
> #define WESTON_COMPOSITOR_WAYLAND_H
>
> #include "compositor.h"
> +#include "plugin-registry.h"
>
> #ifdef __cplusplus
> extern "C" {
> @@ -36,14 +37,28 @@ extern "C" {
>
> #define WESTON_WAYLAND_BACKEND_CONFIG_VERSION 1
>
> -struct weston_wayland_backend_output_config {
> - int width;
> - int height;
> - char *name;
> - uint32_t transform;
> - int32_t scale;
> +
> +#define WESTON_WAYLAND_OUTPUT_API_NAME "weston_wayland_output_api_v1"
> +
> +struct weston_wayland_output_api {
> + /** Configure a fullscreen/sprawl-ed output so it can
> + * be enabled.
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> + int (*output_configure)(struct weston_output *output);
> };
I think we discussed in IRC that this API won't be necessary after all.
Splitting weston_output_init_pending() to the separate signal emission
step should fix it.
>
> +static inline const struct weston_wayland_output_api *
> +weston_wayland_output_get_api(struct weston_compositor *compositor)
> +{
> + const void *api;
> + api = weston_plugin_api_get(compositor, WESTON_WAYLAND_OUTPUT_API_NAME,
> + sizeof(struct weston_wayland_output_api));
> +
> + return (const struct weston_wayland_output_api *)api;
> +}
> +
> struct weston_wayland_backend_config {
> struct weston_backend_config base;
> int use_pixman;
I think the overall structure is fairly good already, but reading
through this I did discover a bunch of existing issues. The Wayland
backend would need more love than just keeping up with API changes...
Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160817/eb44c1d2/attachment-0001.sig>
More information about the wayland-devel
mailing list