On Sun, 14 Aug 2016 17:28:16 +0200
Armin Krezović <krezovic.ar...@gmail.com> wrote:

> This is a complete port of the RDP 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 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.
> 
> After everything has been set, output needs to be
> enabled manually using weston_output_enable().
> 
> v2:
> 
>  - Rename output_configure() to output_set_size()
>    in plugin API and describe it.
>  - Manually fetch parsed_options from wet_compositor.
>  - Call rdp_output_disable() explicitly from
>    rdp_output_destroy().
> 
> Signed-off-by: Armin Krezović <krezovic.ar...@gmail.com>
> ---
>  compositor/main.c          |  52 ++++++++++++++++--
>  libweston/compositor-rdp.c | 130 
> +++++++++++++++++++++++++++++++--------------
>  libweston/compositor-rdp.h |  24 ++++++++-
>  3 files changed, 160 insertions(+), 46 deletions(-)
> 
> diff --git a/compositor/main.c b/compositor/main.c
> index b26760c..a72c2f9 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -1284,13 +1284,46 @@ load_headless_backend(struct weston_compositor *c,
>  }
>  
>  static void
> +rdp_backend_output_configure(struct wl_listener *listener, void *data)
> +{
> +     struct weston_output *output = data;
> +     struct wet_compositor *compositor = 
> to_wet_compositor(output->compositor);
> +     struct wet_output_config *parsed_options = compositor->parsed_options;
> +     const struct weston_rdp_output_api *api = 
> weston_rdp_output_get_api(output->compositor);
> +     int width = 640;
> +     int height = 480;
> +
> +     assert(parsed_options);
> +
> +     if (!api) {
> +             weston_log("Cannot use weston_rdp_output_api.\n");
> +             return;
> +     }
> +
> +     if (parsed_options->width)
> +             width = parsed_options->width;
> +
> +     if (parsed_options->height)
> +             height = parsed_options->height;
> +
> +     weston_output_set_scale(output, 1);
> +     weston_output_set_transform(output, WL_OUTPUT_TRANSFORM_NORMAL);
> +
> +     if (api->output_set_size(output, width, height) < 0) {
> +             weston_log("Cannot configure output \"%s\" using 
> weston_rdp_output_api.\n",
> +                        output->name);
> +             return;
> +     }
> +
> +     weston_output_enable(output);
> +}
> +
> +static void
>  weston_rdp_backend_config_init(struct weston_rdp_backend_config *config)
>  {
>       config->base.struct_version = WESTON_RDP_BACKEND_CONFIG_VERSION;
>       config->base.struct_size = sizeof(struct weston_rdp_backend_config);
>  
> -     config->width = 640;
> -     config->height = 480;
>       config->bind_address = NULL;
>       config->port = 3389;
>       config->rdp_key = NULL;
> @@ -1307,12 +1340,16 @@ load_rdp_backend(struct weston_compositor *c,
>       struct weston_rdp_backend_config config  = {{ 0, }};
>       int ret = 0;
>  
> +     struct wet_output_config *parsed_options = wet_init_parsed_options(c);
> +     if (!parsed_options)
> +             return -1;
> +
>       weston_rdp_backend_config_init(&config);
>  
>       const struct weston_option rdp_options[] = {
>               { WESTON_OPTION_BOOLEAN, "env-socket", 0, &config.env_socket },
> -             { WESTON_OPTION_INTEGER, "width", 0, &config.width },
> -             { WESTON_OPTION_INTEGER, "height", 0, &config.height },
> +             { WESTON_OPTION_INTEGER, "width", 0, &parsed_options->width },
> +             { WESTON_OPTION_INTEGER, "height", 0, &parsed_options->height },
>               { WESTON_OPTION_STRING,  "address", 0, &config.bind_address },
>               { WESTON_OPTION_INTEGER, "port", 0, &config.port },
>               { WESTON_OPTION_BOOLEAN, "no-clients-resize", 0, 
> &config.no_clients_resize },
> @@ -1326,10 +1363,17 @@ load_rdp_backend(struct weston_compositor *c,
>       ret = weston_compositor_load_backend(c, WESTON_BACKEND_RDP,
>                                            &config.base);
>  
> +     if (ret < 0)
> +             goto out;
> +
> +     wet_set_pending_output_handler(c, rdp_backend_output_configure);
> +
> +out:
>       free(config.bind_address);
>       free(config.rdp_key);
>       free(config.server_cert);
>       free(config.server_key);
> +
>       return ret;
>  }
>  

Again, main.c stuff looks good.


> diff --git a/libweston/compositor-rdp.c b/libweston/compositor-rdp.c
> index 11f5f05..ffa2fdf 100644
> --- a/libweston/compositor-rdp.c
> +++ b/libweston/compositor-rdp.c
> @@ -371,15 +371,6 @@ rdp_output_repaint(struct weston_output *output_base, 
> pixman_region32_t *damage)
>       return 0;
>  }
>  
> -static void
> -rdp_output_destroy(struct weston_output *output_base)
> -{
> -     struct rdp_output *output = to_rdp_output(output_base);
> -
> -     wl_event_source_remove(output->finish_frame_timer);
> -     free(output);
> -}
> -
>  static int
>  finish_frame_handler(void *data)
>  {
> @@ -471,17 +462,13 @@ rdp_switch_mode(struct weston_output *output, struct 
> weston_mode *target_mode)
>  }
>  
>  static int
> -rdp_backend_create_output(struct rdp_backend *b, int width, int height)
> +rdp_output_configure(struct weston_output *base,
> +                  int width, int height)
>  {
> -     struct rdp_output *output;
> -     struct wl_event_loop *loop;
> +     struct rdp_output *output = to_rdp_output(base);
>       struct weston_mode *currentMode;
>       struct weston_mode initMode;
>  
> -     output = zalloc(sizeof *output);
> -     if (output == NULL)
> -             return -1;
> -
>       wl_list_init(&output->peers);
>       wl_list_init(&output->base.mode_list);
>  
> @@ -492,48 +479,100 @@ rdp_backend_create_output(struct rdp_backend *b, int 
> width, int height)
>  
>       currentMode = ensure_matching_mode(&output->base, &initMode);
>       if (!currentMode)
> -             goto out_free_output;
> +             return -1;
>  
>       output->base.current_mode = output->base.native_mode = currentMode;
> -     weston_output_init(&output->base, b->compositor, 0, 0, width, height,
> -                        WL_OUTPUT_TRANSFORM_NORMAL, 1);
>  
>       output->base.make = "weston";
>       output->base.model = "rdp";
> +     output->base.mm_width = width;
> +     output->base.mm_height = height;
> +
> +     output->base.start_repaint_loop = rdp_output_start_repaint_loop;
> +     output->base.repaint = rdp_output_repaint;
> +     output->base.assign_planes = NULL;
> +     output->base.set_backlight = NULL;
> +     output->base.set_dpms = NULL;
> +     output->base.switch_mode = rdp_switch_mode;
> +
> +     return 0;
> +}
> +
> +static int
> +rdp_output_enable(struct weston_output *base)
> +{
> +     struct rdp_output *output = to_rdp_output(base);
> +     struct rdp_backend *b = to_rdp_backend(base->compositor);
> +     struct wl_event_loop *loop;
> +
>       output->shadow_surface = pixman_image_create_bits(PIXMAN_x8r8g8b8,
> -                     width, height,
> +                     output->base.mm_width, output->base.mm_height,

Use base.current_mode.width and height.


>                   NULL,
> -                 width * 4);
> +                 output->base.mm_width * 4);
>       if (output->shadow_surface == NULL) {
>               weston_log("Failed to create surface for frame buffer.\n");
> -             goto out_output;
> +             return -1;
>       }
>  
> -     if (pixman_renderer_output_create(&output->base) < 0)
> -             goto out_shadow_surface;
> +     if (pixman_renderer_output_create(&output->base) < 0) {
> +             pixman_image_unref(output->shadow_surface);
> +             return -1;
> +     }
>  
>       loop = wl_display_get_event_loop(b->compositor->wl_display);
>       output->finish_frame_timer = wl_event_loop_add_timer(loop, 
> finish_frame_handler, output);
>  
> -     output->base.start_repaint_loop = rdp_output_start_repaint_loop;
> -     output->base.repaint = rdp_output_repaint;
> -     output->base.destroy = rdp_output_destroy;
> -     output->base.assign_planes = NULL;
> -     output->base.set_backlight = NULL;
> -     output->base.set_dpms = NULL;
> -     output->base.switch_mode = rdp_switch_mode;
>       b->output = output;
>  
> -     weston_compositor_add_output(b->compositor, &output->base);
>       return 0;
> +}
> +
> +static int
> +rdp_output_disable(struct weston_output *base)
> +{
> +     struct rdp_output *output = to_rdp_output(base);
> +     struct rdp_backend *b = to_rdp_backend(base->compositor);
> +
> +     if (!output->base.enabled)
> +             return 0;
>  
> -out_shadow_surface:
>       pixman_image_unref(output->shadow_surface);
> -out_output:
> +     pixman_renderer_output_destroy(&output->base);

I don't see the pixman_image_unref() or
pixman_renderer_output_destroy() in the original code for this path,
did you add them yourself?

Adding them is correct I believe, but it should be a separate patch.


> +
> +     wl_event_source_remove(output->finish_frame_timer);
> +     b->output = NULL;
> +
> +     return 0;
> +}
> +
> +static void
> +rdp_output_destroy(struct weston_output *base)
> +{
> +     struct rdp_output *output = to_rdp_output(base);
> +
> +     rdp_output_disable(&output->base);
>       weston_output_destroy(&output->base);
> -out_free_output:
> +
>       free(output);
> -     return -1;
> +}
> +
> +static int
> +rdp_backend_create_output(struct weston_compositor *compositor)
> +{
> +     struct rdp_output *output;
> +
> +     output = zalloc(sizeof *output);
> +     if (output == NULL)
> +             return -1;
> +
> +     output->base.name =  NULL;
> +     output->base.destroy = rdp_output_destroy;
> +     output->base.disable = rdp_output_disable;
> +     output->base.enable = rdp_output_enable;
> +
> +     weston_output_init_pending(&output->base, compositor);
> +
> +     return 0;
>  }
>  
>  static void
> @@ -1216,6 +1255,10 @@ rdp_incoming_peer(freerdp_listener *instance, 
> freerdp_peer *client)
>       FREERDP_CB_RETURN(TRUE);
>  }
>  
> +static const struct weston_rdp_output_api api = {
> +     rdp_output_configure,
> +};
> +
>  static struct rdp_backend *
>  rdp_backend_create(struct weston_compositor *compositor,
>                  struct weston_rdp_backend_config *config)
> @@ -1223,7 +1266,7 @@ rdp_backend_create(struct weston_compositor *compositor,
>       struct rdp_backend *b;
>       char *fd_str;
>       char *fd_tail;
> -     int fd;
> +     int fd, ret;
>  
>       b = zalloc(sizeof *b);
>       if (b == NULL)
> @@ -1251,7 +1294,7 @@ rdp_backend_create(struct weston_compositor *compositor,
>       if (pixman_renderer_init(compositor) < 0)
>               goto err_compositor;
>  
> -     if (rdp_backend_create_output(b, config->width, config->height) < 0)
> +     if (rdp_backend_create_output(compositor) < 0)
>               goto err_compositor;
>  
>       compositor->capabilities |= WESTON_CAP_ARBITRARY_MODES;
> @@ -1282,6 +1325,15 @@ rdp_backend_create(struct weston_compositor 
> *compositor,
>       }
>  
>       compositor->backend = &b->base;
> +
> +     ret = weston_plugin_api_register(compositor, WESTON_RDP_OUTPUT_API_NAME,
> +                                      &api, sizeof(api));
> +
> +     if (ret < 0) {
> +             weston_log("Failed to register output API.\n");
> +             goto err_output;
> +     }
> +
>       return b;
>  
>  err_listener:
> @@ -1301,8 +1353,6 @@ err_free_strings:
>  static void
>  config_init_to_defaults(struct weston_rdp_backend_config *config)
>  {
> -     config->width = 640;
> -     config->height = 480;
>       config->bind_address = NULL;
>       config->port = 3389;
>       config->rdp_key = NULL;
> diff --git a/libweston/compositor-rdp.h b/libweston/compositor-rdp.h
> index dfa1759..44bea3d 100644
> --- a/libweston/compositor-rdp.h
> +++ b/libweston/compositor-rdp.h
> @@ -31,13 +31,33 @@ extern "C" {
>  #endif
>  
>  #include "compositor.h"
> +#include "plugin-registry.h"
> +
> +#define WESTON_RDP_OUTPUT_API_NAME "weston_rdp_output_api_v1"
> +
> +struct weston_rdp_output_api {
> +     /** Initialize a RDP output with specified width and height.
> +      *
> +      * Returns 0 on success, -1 on failure.
> +      */
> +     int (*output_set_size)(struct weston_output *output,
> +                            int width, int height);
> +};
> +
> +static inline const struct weston_rdp_output_api *
> +weston_rdp_output_get_api(struct weston_compositor *compositor)
> +{
> +     const void *api;
> +     api = weston_plugin_api_get(compositor, WESTON_RDP_OUTPUT_API_NAME,
> +                                 sizeof(struct weston_rdp_output_api));
> +
> +     return (const struct weston_rdp_output_api *)api;
> +}
>  
>  #define WESTON_RDP_BACKEND_CONFIG_VERSION 1
>  
>  struct weston_rdp_backend_config {
>       struct weston_backend_config base;
> -     int width;
> -     int height;
>       char *bind_address;
>       int port;
>       char *rdp_key;

Whenever you remove or change existing fields from a struct
weston_*_backend_config, you need to bump the
WESTON_*_BACKEND_CONFIG_VERSION.

This applies to all the backends. Again I just didn't notice earlier.

Not much to fix in this patch, nice!


Thanks,
pq

Attachment: pgpBT_Nxrb46R.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to