On 20.06.2016 15:42, Pekka Paalanen wrote: > 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 >
Thank you both, I'll implement your suggestions in the next patchset version. Armin.
signature.asc
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
