On Sun, 19 Jun 2016 11:06:05 +0200 Quentin Glidic <[email protected]> wrote:
> On 18/06/2016 19:15, Armin Krezović wrote: > > This patch adds a new command line option which can be > > used to tell headless backend not to create any > > virtual outputs. > > > > This will be used for output hotplug emulation, where > > weston will start with no outputs available, and the > > virtual output will be created at runtime. > > > > Signed-off-by: Armin Krezović <[email protected]> > > Good, just one nitpick below to make it perfect. > > Reviewed-by: Quentin Glidic <[email protected]> > > > > --- > > src/compositor-headless.c | 7 +++++-- > > src/compositor-headless.h | 1 + > > src/main.c | 4 +++- > > 3 files changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/src/compositor-headless.c b/src/compositor-headless.c > > index 45e47ca..05dd8af 100644 > > --- a/src/compositor-headless.c > > +++ b/src/compositor-headless.c > > @@ -235,8 +235,11 @@ headless_backend_create(struct weston_compositor > > *compositor, > > if (b->use_pixman) { > > pixman_renderer_init(compositor); > > } > > - if (headless_backend_create_output(b, config) < 0) > > - goto err_input; > > + > > + if (!config->no_outputs) { > > + if (headless_backend_create_output(b, config) < 0) > > + goto err_input; > > + } > > > > if (!b->use_pixman && noop_renderer_init(compositor) < 0) > > goto err_input; > > diff --git a/src/compositor-headless.h b/src/compositor-headless.h > > index 79f39c8..b128ab0 100644 > > --- a/src/compositor-headless.h > > +++ b/src/compositor-headless.h > > @@ -44,6 +44,7 @@ struct weston_headless_backend_config { > > int use_pixman; > > > > uint32_t transform; > > + int no_outputs; Hi, the patch is good, just one more minor comment. This is a boolean flag, so the type should be 'bool' instead of 'int'. As people often struggle reading double-negatives, the flag would be more readable as 'bool create_output'. It does mean main.c needs to invert the flag because it uses '--no-output' which I think is fine. > > }; > > > > #ifdef __cplusplus > > diff --git a/src/main.c b/src/main.c > > index ec3d799..dc6529d 100644 > > --- a/src/main.c > > +++ b/src/main.c > > @@ -526,7 +526,8 @@ usage(int error_code) > > " --height=HEIGHT\tHeight of memory surface\n" > > " --transform=TR\tThe output transformation, TR is one of:\n" > > "\tnormal 90 180 270 flipped flipped-90 flipped-180 > > flipped-270\n" > > - " --use-pixman\t\tUse the pixman (CPU) renderer (default: no > > rendering)\n\n"); > > + " --use-pixman\t\tUse the pixman (CPU) renderer (default: no > > rendering)\n" > > + " --no-outputs\t\tDo not create any virtual outputs\n\n"); > > A tiny improvement to make here: put the last \n and the closing on its > own line, so we future additions/removal stop moving it around. See > commit e77f8ad79bdf3613dc7e587ea0cf5b9d39e4f8e0 for a similar change > related to the fbdev backend. Agreed. > > > #endif > > > > #if defined(BUILD_RDP_COMPOSITOR) > > @@ -1037,6 +1038,7 @@ load_headless_backend(struct weston_compositor *c, > > { WESTON_OPTION_INTEGER, "height", 0, &config.height }, > > { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config.use_pixman }, > > { WESTON_OPTION_STRING, "transform", 0, &transform }, > > + { WESTON_OPTION_BOOLEAN, "no-outputs", 0, &config.no_outputs }, > > }; > > > > parse_options(options, ARRAY_LENGTH(options), argc, argv); > > All these are really minor comments, so in any case: Reviewed-by: Pekka Paalanen <[email protected]> If you do end up re-sending the series, then it would be good to fix them. Thanks, pq
pgpz5UjbEzFY4.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
