[PATCH weston 09/12 v2] weston: Port X11 backend to new output handling API
Pekka Paalanen
ppaalanen at gmail.com
Thu Aug 18 11:35:23 UTC 2016
On Sun, 14 Aug 2016 17:28:18 +0200
Armin Krezović <krezovic.armin at gmail.com> wrote:
> This is a complete port of the X11 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.
>
> Same as before, a single output is created at runtime
> using the default configuration or a configuration
> parsed from the command line. The output-count
> functionality is also preserved, which means more than
> one output can be created initially, and more outputs can
> be added at runtime using the output API.
>
> v2:
>
> - Fix wet_configure_windowed_output_from_config() usage.
> - Call x11_output_disable() explicitly from
> x11_output_destroy().
>
> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
> ---
> compositor/main.c | 151 ++++++++++-------------
> libweston/compositor-x11.c | 295 ++++++++++++++++++++++++---------------------
> libweston/compositor-x11.h | 11 --
> 3 files changed, 223 insertions(+), 234 deletions(-)
>
> diff --git a/compositor/main.c b/compositor/main.c
> index 143ee21..a39a954 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
>
> - config.base.struct_version = WESTON_X11_BACKEND_CONFIG_VERSION;
> - config.base.struct_size = sizeof(struct weston_x11_backend_config);
> -
> - /* load the actual backend and configure it */
> - ret = weston_compositor_load_backend(c, WESTON_BACKEND_X11,
> - &config.base);
> -
> -out:
> - for (j = 0; j < config.num_outputs; ++j)
> - free(config.outputs[j].name);
> - free(config.outputs);
> + return 0;
>
> +err:
> + free(parsed_options);
wet_compositor::parsed_options gets stale again.
All the other main.c bits look ok.
> return ret;
> }
>
> diff --git a/libweston/compositor-x11.c b/libweston/compositor-x11.c
> index b900184..5720ddf 100644
> --- a/libweston/compositor-x11.c
> +++ b/libweston/compositor-x11.c
> @@ -59,6 +59,7 @@
> #include "pixman-renderer.h"
> #include "presentation-time-server-protocol.h"
> #include "linux-dmabuf.h"
> +#include "windowed-output-api.h"
>
> #define DEFAULT_AXIS_STEP_DISTANCE 10
>
> @@ -75,6 +76,8 @@ struct x11_backend {
> struct xkb_keymap *xkb_keymap;
> unsigned int has_xkb;
> uint8_t xkb_event_base;
> + int fullscreen;
> + int no_input;
> int use_pixman;
>
> int has_net_wm_state_fullscreen;
> @@ -516,30 +519,6 @@ x11_output_deinit_shm(struct x11_backend *b, struct x11_output *output)
> }
>
> static void
> -x11_output_destroy(struct weston_output *output_base)
> -{
> - struct x11_output *output = to_x11_output(output_base);
> - struct x11_backend *backend =
> - to_x11_backend(output->base.compositor);
> -
> - wl_event_source_remove(output->finish_frame_timer);
> -
> - if (backend->use_pixman) {
> - pixman_renderer_output_destroy(output_base);
> - x11_output_deinit_shm(backend, output);
> - } else
> - gl_renderer->output_destroy(output_base);
> -
> - xcb_destroy_window(backend->conn, output->window);
> -
> - xcb_flush(backend->conn);
> -
> - weston_output_destroy(&output->base);
> -
> - free(output);
> -}
> -
> -static void
> x11_output_set_wm_protocols(struct x11_backend *b,
> struct x11_output *output)
> {
> @@ -789,20 +768,54 @@ x11_output_init_shm(struct x11_backend *b, struct x11_output *output,
> return 0;
> }
>
> -static struct x11_output *
> -x11_backend_create_output(struct x11_backend *b, int x, int y,
> - int width, int height, int fullscreen,
> - int no_input, char *configured_name,
> - uint32_t transform, int32_t scale)
> +static int
> +x11_output_disable(struct weston_output *base)
> +{
> + struct x11_output *output = to_x11_output(base);
> + struct x11_backend *backend = to_x11_backend(base->compositor);
> +
> + if (!output->base.enabled)
> + return 0;
> +
> + wl_event_source_remove(output->finish_frame_timer);
> +
> + if (backend->use_pixman) {
> + pixman_renderer_output_destroy(&output->base);
> + x11_output_deinit_shm(backend, output);
> + } else {
> + gl_renderer->output_destroy(&output->base);
> + }
> +
> + xcb_destroy_window(backend->conn, output->window);
> + xcb_flush(backend->conn);
> +
> + return 0;
> +}
> +
> +static void
> +x11_output_destroy(struct weston_output *base)
> +{
> + struct x11_output *output = to_x11_output(base);
> +
> + x11_output_disable(&output->base);
> + weston_output_destroy(&output->base);
> +
> + free(output);
> +}
The disable/destroy stuff looks good.
> +
> +static int
> +x11_output_enable(struct weston_output *base)
> {
> + struct x11_output *output = to_x11_output(base);
> + struct x11_backend *b = to_x11_backend(base->compositor);
> +
> static const char name[] = "Weston Compositor";
> static const char class[] = "weston-1\0Weston Compositor";
> char *title = NULL;
> - struct x11_output *output;
> xcb_screen_t *screen;
> struct wm_normal_hints normal_hints;
> struct wl_event_loop *loop;
> - int output_width, output_height, width_mm, height_mm;
> +
> int ret;
> uint32_t mask = XCB_CW_EVENT_MASK | XCB_CW_CURSOR;
> xcb_atom_t atom_list[1];
> @@ -812,10 +825,7 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
> 0
> };
>
> - output_width = width * scale;
> - output_height = height * scale;
> -
> - if (!no_input)
> + if (!b->no_input)
> values[0] |=
> XCB_EVENT_MASK_KEY_PRESS |
> XCB_EVENT_MASK_KEY_RELEASE |
> @@ -827,22 +837,6 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
> XCB_EVENT_MASK_KEYMAP_STATE |
> XCB_EVENT_MASK_FOCUS_CHANGE;
>
> - output = zalloc(sizeof *output);
> - if (output == NULL) {
> - perror("zalloc");
> - return NULL;
> - }
> -
> - 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);
> -
> values[1] = b->null_cursor;
> output->window = xcb_generate_id(b->conn);
> screen = x11_compositor_get_default_screen(b);
> @@ -851,13 +845,13 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
> output->window,
> screen->root,
> 0, 0,
> - output_width, output_height,
> + output->mode.width, output->mode.height,
> 0,
> XCB_WINDOW_CLASS_INPUT_OUTPUT,
> screen->root_visual,
> mask, values);
>
> - if (fullscreen) {
> + if (b->fullscreen) {
> atom_list[0] = b->atom.net_wm_state_fullscreen;
> xcb_change_property(b->conn, XCB_PROP_MODE_REPLACE,
> output->window,
> @@ -869,10 +863,10 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
> memset(&normal_hints, 0, sizeof normal_hints);
> normal_hints.flags =
> WM_NORMAL_HINTS_MAX_SIZE | WM_NORMAL_HINTS_MIN_SIZE;
> - normal_hints.min_width = output_width;
> - normal_hints.min_height = output_height;
> - normal_hints.max_width = output_width;
> - normal_hints.max_height = output_height;
> + normal_hints.min_width = output->mode.width;
> + normal_hints.min_height = output->mode.height;
> + normal_hints.max_width = output->mode.width;
> + normal_hints.max_height = output->mode.height;
> xcb_change_property(b->conn, XCB_PROP_MODE_REPLACE, output->window,
> b->atom.wm_normal_hints,
> b->atom.wm_size_hints, 32,
> @@ -881,8 +875,8 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
> }
>
> /* Set window name. Don't bother with non-EWMH WMs. */
> - if (configured_name) {
> - if (asprintf(&title, "%s - %s", name, configured_name) < 0)
> + if (output->base.name) {
> + if (asprintf(&title, "%s - %s", name, output->base.name) < 0)
> title = NULL;
> } else {
> title = strdup(name);
> @@ -894,9 +888,7 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
> strlen(title), title);
> free(title);
> } else {
> - xcb_destroy_window(b->conn, output->window);
> - free(output);
> - return NULL;
> + goto err;
> }
>
> xcb_change_property(b->conn, XCB_PROP_MODE_REPLACE, output->window,
> @@ -909,44 +901,20 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
>
> xcb_map_window(b->conn, output->window);
>
> - if (fullscreen)
> + if (b->fullscreen)
> x11_output_wait_for_map(b, output);
>
> - output->base.start_repaint_loop = x11_output_start_repaint_loop;
> - if (b->use_pixman)
> - output->base.repaint = x11_output_repaint_shm;
> - else
> - output->base.repaint = x11_output_repaint_gl;
> - output->base.destroy = x11_output_destroy;
> - output->base.assign_planes = NULL;
> - output->base.set_backlight = NULL;
> - output->base.set_dpms = NULL;
> - output->base.switch_mode = NULL;
> - output->base.current_mode = &output->mode;
> - output->base.make = "weston-X11";
> - output->base.model = "none";
> -
> - if (configured_name)
> - output->base.name = strdup(configured_name);
> -
> - width_mm = width * b->screen->width_in_millimeters /
> - b->screen->width_in_pixels;
> - height_mm = height * b->screen->height_in_millimeters /
> - b->screen->height_in_pixels;
> - weston_output_init(&output->base, b->compositor,
> - x, y, width_mm, height_mm, transform, scale);
> -
> if (b->use_pixman) {
> if (x11_output_init_shm(b, output,
> output->mode.width,
> output->mode.height) < 0) {
> weston_log("Failed to initialize SHM for the X11 output\n");
> - return NULL;
> + goto err;
> }
> if (pixman_renderer_output_create(&output->base) < 0) {
> weston_log("Failed to create pixman renderer for output\n");
> x11_output_deinit_shm(b, output);
> - return NULL;
> + goto err;
> }
> } else {
> /* eglCreatePlatformWindowSurfaceEXT takes a Window*
> @@ -960,19 +928,100 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
> NULL,
> 0);
> if (ret < 0)
> - return NULL;
> + goto err;
> }
>
> loop = wl_display_get_event_loop(b->compositor->wl_display);
> output->finish_frame_timer =
> wl_event_loop_add_timer(loop, finish_frame_handler, output);
>
> - weston_compositor_add_output(b->compositor, &output->base);
> -
> weston_log("x11 output %dx%d, window id %d\n",
> - width, height, output->window);
> + output->mode.width, output->mode.height, output->window);
> +
> + return 0;
> +
> +err:
> + xcb_destroy_window(b->conn, output->window);
> + xcb_flush(b->conn);
> +
> + return -1;
> +}
> +
> +static int
> +x11_output_configure(struct weston_output *base, int width, int height)
> +{
> + struct x11_output *output = to_x11_output(base);
> + struct x11_backend *b = to_x11_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;
Oh right, we really do depend on weston_output_set_scale() being called
first. I didn't notice until now. There are two options:
- assert that it has been called
- have this function just store the given width,height, and move all
this code into enable()
The latter would seem a more proper solution, except we cannot do that
because weston_output_enable() already calls the old
weston_output_init() which needs these values.
So, assert it is, even though I don't really like requiring a
particular sequence from the user like that. Loosening the ordering
requirement would need weston_output_enable() to be reduced to just
calling the backend's ->enable() which would then do all the rest of
the stuff. I believe this has to be done eventually if not now, but I
won't push for it. (Hello de-mid-layering!)
The same applies to weston_output_set_transform().
Hmm, I did think the x11-backend behaved a little odd when defining
both scale and transform. With a 90-degree rotation, the aspect ratio
indeed becomes inverted.
Btw. also all the asserts we are adding here would eventually need to
be converted into proper checks and error messages if they are to stay.
These asserts are essentially just "/* TODO: check this properly */".
> +
> + 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 * b->screen->width_in_millimeters /
> + b->screen->width_in_pixels;
> + output->base.mm_height = height * b->screen->height_in_millimeters /
> + b->screen->height_in_pixels;
> +
> + if (b->use_pixman)
> + output->base.repaint = x11_output_repaint_shm;
> + else
> + output->base.repaint = x11_output_repaint_gl;
> +
> + output->base.start_repaint_loop = x11_output_start_repaint_loop;
> + output->base.assign_planes = NULL;
> + output->base.set_backlight = NULL;
> + output->base.set_dpms = NULL;
> + output->base.switch_mode = NULL;
> + output->base.current_mode = &output->mode;
> + output->base.make = "weston-X11";
> + output->base.model = "none";
>
> - return output;
> + return 0;
> +}
> +
> +static int
> +x11_output_create(struct weston_compositor *compositor,
> + const char *name)
> +{
> + struct x11_output *output;
> +
> + output = zalloc(sizeof *output);
> + if (output == NULL) {
> + perror("zalloc");
> + return -1;
> + }
> +
> + output->base.name = name ? strdup(name) : NULL;
> + output->base.destroy = x11_output_destroy;
> + output->base.disable = x11_output_disable;
> + output->base.enable = x11_output_enable;
> +
> + weston_output_init_pending(&output->base, compositor);
> +
> + return 0;
> }
>
> static struct x11_output *
> @@ -1586,21 +1635,27 @@ init_gl_renderer(struct x11_backend *b)
> return ret;
> }
>
> +static const struct weston_windowed_output_api api = {
> + x11_output_configure,
> + x11_output_create,
> +};
> +
> static struct x11_backend *
> x11_backend_create(struct weston_compositor *compositor,
> struct weston_x11_backend_config *config)
> {
> struct x11_backend *b;
> - struct x11_output *output;
> struct wl_event_loop *loop;
> - int x = 0;
> - unsigned i;
> + int ret;
>
> b = zalloc(sizeof *b);
> if (b == NULL)
> return NULL;
>
> b->compositor = compositor;
> + b->fullscreen = config->fullscreen;
> + b->no_input = config->no_input;
> +
> if (weston_compositor_set_presentation_clock_software(compositor) < 0)
> goto err_free;
>
> @@ -1646,44 +1701,6 @@ x11_backend_create(struct weston_compositor *compositor,
> goto err_renderer;
> }
>
> - for (i = 0; i < config->num_outputs; ++i) {
> - struct weston_x11_backend_output_config *output_iterator =
> - &config->outputs[i];
> -
> - if (output_iterator->name == NULL) {
> - continue;
> - }
> -
> - if (output_iterator->width < 1) {
> - weston_log("Invalid width \"%d\" for output %s\n",
> - output_iterator->width, output_iterator->name);
> - goto err_x11_input;
> - }
> -
> - if (output_iterator->height < 1) {
> - weston_log("Invalid height \"%d\" for output %s\n",
> - output_iterator->height, output_iterator->name);
> - goto err_x11_input;
> - }
> -
> - output = x11_backend_create_output(b,
> - x,
> - 0,
> - output_iterator->width,
> - output_iterator->height,
> - config->fullscreen,
> - config->no_input,
> - output_iterator->name,
> - output_iterator->transform,
> - output_iterator->scale);
> - if (output == NULL) {
> - weston_log("Failed to create configured x11 output\n");
> - goto err_x11_input;
> - }
> -
> - x = pixman_region32_extents(&output->base.region)->x2;
> - }
> -
> loop = wl_display_get_event_loop(compositor->wl_display);
> b->xcb_source =
> wl_event_loop_add_fd(loop,
> @@ -1700,6 +1717,14 @@ x11_backend_create(struct weston_compositor *compositor,
>
> compositor->backend = &b->base;
>
> + ret = weston_plugin_api_register(compositor, WESTON_WINDOWED_OUTPUT_API_NAME,
> + &api, sizeof(api));
> +
> + if (ret < 0) {
> + weston_log("Failed to register output API.\n");
> + goto err_x11_input;
> + }
> +
> return b;
>
> err_x11_input:
> diff --git a/libweston/compositor-x11.h b/libweston/compositor-x11.h
> index 6a17f96..06e586a 100644
> --- a/libweston/compositor-x11.h
> +++ b/libweston/compositor-x11.h
> @@ -36,14 +36,6 @@ extern "C" {
>
> #define WESTON_X11_BACKEND_CONFIG_VERSION 1
>
> -struct weston_x11_backend_output_config {
> - int width;
> - int height;
> - char *name;
> - uint32_t transform;
> - int32_t scale;
> -};
> -
> struct weston_x11_backend_config {
> struct weston_backend_config base;
>
> @@ -52,9 +44,6 @@ struct weston_x11_backend_config {
>
> /** Whether to use the pixman renderer instead of the OpenGL ES renderer. */
> bool use_pixman;
> -
> - uint32_t num_outputs;
> - struct weston_x11_backend_output_config *outputs;
> };
>
> #ifdef __cplusplus
Yeah, not much to complain about this patch, just the usual
s/configure/set_size/ and I forget if something else.
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/20160818/c0d234d2/attachment-0001.sig>
More information about the wayland-devel
mailing list