On Thu, 11 Aug 2016 17:33:58 +0200 Armin Krezović <krezovic.ar...@gmail.com> wrote:
> This implements output configuration for outputs which use
> previously added weston_windowed_output_api. The function
> takes an output that's to be configured, default configuration
> that's to be set in case no configuration is specified in
> the config file or on command line and optional third argument,
> parsed_options, which will override defaults and options for
> configuration if they are present.
>
> This also introduces new compositor specific functions for
> setting output's scale and transform from either hardcoded
> default, config file option or command line option.
>
> Pending output handling is also wired up in the compositor.
Hi,
I wouldn't say "wired up" because wet_set_pending_output_handler() is
not called yet in this patch. This patch is more like adding helpers
for the windowed backend-cases.
There are a couple details pointed out below, but all in all this patch
looks good to me. With the potential double-free fixed, you can slap a
Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk>
on this patch.
> Signed-off-by: Armin Krezović <krezovic.ar...@gmail.com>
> ---
> compositor/main.c | 171
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 171 insertions(+)
>
> diff --git a/compositor/main.c b/compositor/main.c
> index 0e5af5b..c5d8e81 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -63,11 +63,21 @@
> #include "compositor-fbdev.h"
> #include "compositor-x11.h"
> #include "compositor-wayland.h"
> +#include "output-api.h"
>
> #define WINDOW_TITLE "Weston Compositor"
>
> +struct wet_output_config {
> + int width;
> + int height;
> + int32_t scale;
> + uint32_t transform;
> +};
> +
> struct wet_compositor {
> struct weston_config *config;
> + struct wet_output_config *parsed_options;
> + struct wl_listener pending_output_listener;
> };
>
> static FILE *weston_logfile = NULL;
> @@ -425,6 +435,47 @@ to_wet_compositor(struct weston_compositor *compositor)
> return weston_compositor_get_user_data(compositor);
> }
>
> +static void
> +wet_set_pending_output_handler(struct weston_compositor *ec,
> + wl_notify_func_t handler)
> +{
> + struct wet_compositor *compositor = to_wet_compositor(ec);
> +
> + compositor->pending_output_listener.notify = handler;
> + wl_signal_add(&ec->output_pending_signal,
> &compositor->pending_output_listener);
> +}
> +
> +static struct wet_output_config *
> +wet_get_parsed_options(struct weston_compositor *ec)
> +{
> + struct wet_compositor *compositor = to_wet_compositor(ec);
> +
> + return compositor->parsed_options;
> +}
> +
> +static struct wet_output_config *
> +wet_init_parsed_options(struct weston_compositor *ec)
> +{
> + struct wet_compositor *compositor = to_wet_compositor(ec);
> + struct wet_output_config *config = NULL;
> +
> + config = zalloc(sizeof *config);
> +
> + if (!config) {
> + perror("out of memory");
> + return NULL;
> + }
> +
> + config->width = 0;
> + config->height = 0;
> + config->scale = 0;
> + config->transform = UINT32_MAX;
> +
> + compositor->parsed_options = config;
> +
> + return config;
> +}
> +
> WL_EXPORT struct weston_config *
> wet_get_config(struct weston_compositor *ec)
> {
> @@ -940,6 +991,120 @@ handle_exit(struct weston_compositor *c)
> wl_display_terminate(c->wl_display);
> }
>
> +static void
> +wet_output_set_scale(struct weston_output *output,
> + struct weston_config_section *section,
> + int32_t default_scale,
> + int32_t parsed_scale)
> +{
> + int32_t scale = default_scale;
> +
> + if (section)
> + weston_config_section_get_int(section, "scale", &scale,
> default_scale);
> +
> + if (parsed_scale)
> + scale = parsed_scale;
> +
> + weston_output_set_scale(output, scale);
> +}
> +
> +/* UINT32_MAX is treated as invalid because 0 is a valid
> + * enumeration value and the parameter is unsigned
> + */
> +static void
> +wet_output_set_transform(struct weston_output *output,
> + struct weston_config_section *section,
> + uint32_t default_transform,
> + uint32_t parsed_transform)
> +{
> + char *t;
> + uint32_t transform = default_transform;
> +
> + if (section) {
> + weston_config_section_get_string(section,
> + "transform", &t, "normal");
> +
> + if (weston_parse_transform(t, &transform) < 0) {
> + weston_log("Invalid transform \"%s\" for output %s\n",
> + t, output->name);
> + transform = default_transform;
> + }
> + free(t);
> + }
> +
> + if (parsed_transform != UINT32_MAX)
> + transform = parsed_transform;
> +
> + weston_output_set_transform(output, transform);
> +}
> +
> +static int
> +wet_configure_windowed_output_from_config(struct weston_output *output,
> + struct wet_output_config *defaults,
> + struct wet_output_config
> *parsed_options)
> +{
> + const struct weston_windowed_output_api *api =
> + weston_windowed_output_get_api(output->compositor);
> +
> + struct weston_config *wc = wet_get_config(output->compositor);
> + struct weston_config_section *section = NULL;
> +
> + int width = defaults->width;
> + int height = defaults->height;
> + int32_t scale = defaults->scale;
> + int32_t parsed_scale = 0;
> + uint32_t transform = defaults->transform;
> + uint32_t parsed_transform = UINT32_MAX;
> +
> + if (!api) {
> + weston_log("Cannot use weston_windowed_output_api.\n");
> + return -1;
> + }
> +
> + if (output->name)
> + section = weston_config_get_section(wc, "output", "name",
> output->name);
> +
> + if (section) {
> + char *mode;
> +
> + weston_config_section_get_string(section, "mode", &mode, NULL);
> + if (!mode || sscanf(mode, "%dx%d", &width,
> + &height) != 2) {
> + weston_log("Invalid mode \"%s\" for output %s. Using
> defaults.\n",
> + mode, output->name);
> + free(mode);
Double-free.
'mode' can be NULL.
> + width = defaults->width;
> + height = defaults->height;
> + }
> + free(mode);
> + }
> +
> + if (parsed_options && parsed_options->width)
> + width = parsed_options->width;
> +
> + if (parsed_options && parsed_options->height)
> + height = parsed_options->height;
> +
> + if (parsed_options && parsed_options->scale)
> + parsed_scale = parsed_options->scale;
> +
> + if (parsed_options && parsed_options->transform != UINT32_MAX)
> + parsed_transform = parsed_options->transform;
Isn't the above redundant for transform and scale, don't the
wet_output_set_*() do that already?
Can 'parsed_options' even be NULL? I mean, I don't see the point of
allowing it. Maybe just assert() that it's set, since all backend-cases
using this function have all the same command line options, right?
> +
> + wet_output_set_scale(output, section, scale, parsed_scale);
> + wet_output_set_transform(output, section, transform, parsed_transform);
> +
> + if (api->output_configure(output, width, height) < 0) {
> + weston_log("Cannot configure output \"%s\" using
> weston_windowed_output_api.\n",
> + output->name ? output->name : "unnamed");
> + return -1;
> + }
> +
> + weston_output_enable(output);
> +
> + return 0;
> +}
> +
> static enum weston_drm_backend_output_mode
> drm_configure_output(struct weston_compositor *c,
> bool use_current_mode,
> @@ -1659,6 +1824,7 @@ int main(int argc, char *argv[])
> if (load_configuration(&config, noconfig, config_file) < 0)
> goto out_signals;
> user_data.config = config;
> + user_data.parsed_options = NULL;
>
> section = weston_config_get_section(config, "core", NULL, NULL);
>
> @@ -1683,6 +1849,8 @@ int main(int argc, char *argv[])
> goto out;
> }
>
> + weston_pending_output_coldplug(ec);
> +
> catch_signals();
> segv_compositor = ec;
>
> @@ -1766,6 +1934,9 @@ int main(int argc, char *argv[])
> ret = ec->exit_code;
>
> out:
> + if (user_data.parsed_options)
> + free(user_data.parsed_options);
> +
> weston_compositor_destroy(ec);
>
> out_signals:
Thanks,
pq
pgp6OnRGb75Eu.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
