[PATCH weston] compositor: Separate hardcoded backend related code from compositor
Bryce Harrington
bryce at osg.samsung.com
Tue Jun 23 16:13:50 PDT 2015
On Tue, Jun 23, 2015 at 07:29:17PM +0900, JoonCheol Park wrote:
> Instead of adding available backends and usage outputs at build time,
> this patch finds all available backend plugins in MODULEDIR and prints
> them. It also prints usage output for selected backend by calling
> "backend_usage()" in the plugin.
>
> By this patch, we can remove all hardcode for backends from compositor.
> 3rd party backend plugin can be listed in help output. Backend developer
> can freely add additional description for backend.
>
> Signed-off-by: JoonCheol Park <jooncheol at gmail.com>
> ---
> src/compositor-drm.c | 11 ++++
> src/compositor-fbdev.c | 9 +++
> src/compositor-headless.c | 11 ++++
> src/compositor-rdp.c | 15 +++++
> src/compositor-rpi.c | 12 ++++
> src/compositor-wayland.c | 14 +++++
> src/compositor-x11.c | 13 +++++
> src/compositor.c | 141 ++++++++++++----------------------------------
> src/compositor.h | 3 +
> 9 files changed, 125 insertions(+), 104 deletions(-)
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 6d8684d..0714f34 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -3002,3 +3002,14 @@ backend_init(struct wl_display *display, int *argc, char *argv[],
>
> return drm_compositor_create(display, ¶m, argc, argv, config);
> }
> +
> +WL_EXPORT const char *
> +backend_usage(void)
> +{
> + return "Options for fbdev-backend.so:\n\n"
> + " --connector=ID\tBring up only this connector\n"
> + " --seat=SEAT\t\tThe seat that weston should run on\n"
> + " --tty=TTY\t\tThe tty to use\n"
> + " --use-pixman\t\tUse the pixman (CPU) renderer\n"
> + " --current-mode\tPrefer current KMS mode over EDID preferred mode";
> +}
I think that moving the help text into the relevant backend is a step in
the right direction. Thanks for posting this.
I'm not sure that this routine is the right abstraction though.
Notice that in the backend_init code for compositor-drm.c there already
exists a structure for the supported options:
const struct weston_option drm_options[] = {
{ WESTON_OPTION_INTEGER, "connector", 0, ¶m.connector },
{ WESTON_OPTION_STRING, "seat", 0, ¶m.seat_id },
{ WESTON_OPTION_INTEGER, "tty", 0, ¶m.tty },
{ WESTON_OPTION_BOOLEAN, "current-mode", 0, &option_current_mode },
{ WESTON_OPTION_BOOLEAN, "use-pixman", 0, ¶m.use_pixman },
};
One thought is to add a member to this structure for holding help text.
Then, move this structure from being internal to backend_init(), to
being an exported object, that can be used in main.c to generate the
usage text formatted as main() wishes.
This would decouple the presentation of the options from the definition
of them, as well as reduce the number of places one needs to tinker with
when adding a new option.
But I'll add this is just one possible approach. I chatted with Jon
Cruz about this very thing the other day and it sounded like he had some
further ideas.
Bryce
> diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> index a6fd823..c4d6166 100644
> --- a/src/compositor-fbdev.c
> +++ b/src/compositor-fbdev.c
> @@ -928,3 +928,12 @@ backend_init(struct wl_display *display, int *argc, char *argv[],
>
> return fbdev_compositor_create(display, argc, argv, config, ¶m);
> }
> +
> +WL_EXPORT const char *
> +backend_usage(void)
> +{
> + return "Options for fbdev-backend.so:\n\n"
> + " --tty=TTY\t\tThe tty to use\n"
> + " --device=DEVICE\tThe framebuffer device to use\n"
> + " --use-gl\t\tUse the GL renderer";
> +}
> diff --git a/src/compositor-headless.c b/src/compositor-headless.c
> index c0e35ce..d1d1b75 100644
> --- a/src/compositor-headless.c
> +++ b/src/compositor-headless.c
> @@ -287,3 +287,14 @@ backend_init(struct wl_display *display, int *argc, char *argv[],
> return headless_compositor_create(display, ¶m, display_name,
> argc, argv, config);
> }
> +
> +WL_EXPORT const char *
> +backend_usage(void)
> +{
> + return "Options for headless-backend.so:\n\n"
> + " --width=WIDTH\t\tWidth of memory surface\n"
> + " --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)";
> +}
> diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
> index 6d4d1e4..750aa67 100644
> --- a/src/compositor-rdp.c
> +++ b/src/compositor-rdp.c
> @@ -1275,3 +1275,18 @@ backend_init(struct wl_display *display, int *argc, char *argv[],
>
> return rdp_compositor_create(display, &config, argc, argv, wconfig);
> }
> +
> +WL_EXPORT const char *
> +backend_usage(void)
> +{
> + return "Options for rdp-backend.so:\n\n"
> + " --width=WIDTH\t\tWidth of desktop\n"
> + " --height=HEIGHT\tHeight of desktop\n"
> + " --env-socket=SOCKET\tUse that socket as peer connection\n"
> + " --address=ADDR\tThe address to bind\n"
> + " --port=PORT\t\tThe port to listen on\n"
> + " --no-clients-resize\tThe RDP peers will be forced to the size of the desktop\n"
> + " --rdp4-key=FILE\tThe file containing the key for RDP4 encryption\n"
> + " --rdp-tls-cert=FILE\tThe file containing the certificate for TLS encryption\n"
> + " --rdp-tls-key=FILE\tThe file containing the private key for TLS encryption";
> +}
> diff --git a/src/compositor-rpi.c b/src/compositor-rpi.c
> index 8012461..9bf39bc 100644
> --- a/src/compositor-rpi.c
> +++ b/src/compositor-rpi.c
> @@ -584,3 +584,15 @@ backend_init(struct wl_display *display, int *argc, char *argv[],
>
> return rpi_compositor_create(display, argc, argv, config, ¶m);
> }
> +
> +WL_EXPORT const char *
> +backend_usage(void)
> +{
> + return "Options for rpi-backend.so:\n\n"
> + " --tty=TTY\t\tThe tty to use\n"
> + " --single-buffer\tUse single-buffered Dispmanx elements.\n"
> + " --transform=TR\tThe output transformation, TR is one of:\n"
> + "\tnormal 90 180 270 flipped flipped-90 flipped-180 flipped-270\n"
> + " --opaque-regions\tEnable support for opaque regions, can be "
> + "very slow without support in the GPU firmware.";
> +}
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index ab73853..db86856 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -2170,3 +2170,17 @@ err_outputs:
> wayland_compositor_destroy(c);
> return NULL;
> }
> +
> +WL_EXPORT const char *
> +backend_usage(void)
> +{
> + return "Options for wayland-backend.so:\n\n"
> + " --width=WIDTH\t\tWidth of Wayland surface\n"
> + " --height=HEIGHT\tHeight of Wayland surface\n"
> + " --scale=SCALE\t\tScale factor of output\n"
> + " --fullscreen\t\tRun in fullscreen mode\n"
> + " --use-pixman\t\tUse the pixman (CPU) renderer\n"
> + " --output-count=COUNT\tCreate multiple outputs\n"
> + " --sprawl\t\tCreate one fullscreen output for every parent output\n"
> + " --display=DISPLAY\tWayland display to connect to";
> +}
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index 80ed8c0..bce6c5a 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -1698,3 +1698,16 @@ backend_init(struct wl_display *display, int *argc, char *argv[],
> use_pixman,
> argc, argv, config);
> }
> +
> +WL_EXPORT const char *
> +backend_usage(void)
> +{
> + return "Options for x11-backend.so:\n\n"
> + " --width=WIDTH\t\tWidth of X window\n"
> + " --height=HEIGHT\tHeight of X window\n"
> + " --scale=SCALE\t\tScale factor of output\n"
> + " --fullscreen\t\tRun in fullscreen mode\n"
> + " --use-pixman\t\tUse the pixman (CPU) renderer\n"
> + " --output-count=COUNT\tCreate multiple outputs\n"
> + " --no-input\t\tDont create input devices";
> +}
> diff --git a/src/compositor.c b/src/compositor.c
> index 1e09163..8f8ae59 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -50,6 +50,7 @@
> #include <sys/time.h>
> #include <time.h>
> #include <errno.h>
> +#include <dirent.h>
>
> #ifdef HAVE_LIBUNWIND
> #define UNW_LOCAL_ONLY
> @@ -5016,8 +5017,12 @@ verify_xdg_runtime_dir(void)
> }
>
> static int
> -usage(int error_code)
> +usage(const char *backend, int error_code)
> {
> + DIR *backends = NULL;
> + struct dirent *entry;
> + const char *(*backend_usage)(void) = NULL;
> +
> fprintf(stderr,
> "Usage: weston [OPTIONS]\n\n"
> "This is weston version " VERSION ", the Wayland reference compositor.\n"
> @@ -5027,28 +5032,29 @@ usage(int error_code)
>
> "Core options:\n\n"
> " --version\t\tPrint weston version\n"
> - " -B, --backend=MODULE\tBackend module, one of\n"
> -#if defined(BUILD_DRM_COMPOSITOR)
> - "\t\t\t\tdrm-backend.so\n"
> -#endif
> -#if defined(BUILD_FBDEV_COMPOSITOR)
> - "\t\t\t\tfbdev-backend.so\n"
> -#endif
> -#if defined(BUILD_X11_COMPOSITOR)
> - "\t\t\t\tx11-backend.so\n"
> -#endif
> -#if defined(BUILD_WAYLAND_COMPOSITOR)
> - "\t\t\t\twayland-backend.so\n"
> -#endif
> -#if defined(BUILD_RDP_COMPOSITOR)
> - "\t\t\t\trdp-backend.so\n"
> -#endif
> -#if defined(BUILD_HEADLESS_COMPOSITOR)
> - "\t\t\t\theadless-backend.so\n"
> -#endif
> -#if defined(BUILD_RPI_COMPOSITOR) && defined(HAVE_BCM_HOST)
> - "\t\t\t\trpi-backend.so\n"
> -#endif
> + " -B, --backend=MODULE\tBackend module, one of\n");
> +
> + backends = opendir(MODULEDIR);
> + if (backends) {
> + const char* backend_postfix = "-backend.so";
> +
> + while ((entry = readdir(backends))) {
> + char *name = entry->d_name;
> + int p = strlen(name) - strlen(backend_postfix);
> + if (p <= 0)
> + continue;
> + if (strcmp(name+p, backend_postfix) != 0)
> + continue;
> +
> + fprintf(stderr, "\t\t\t\t%s\n", name);
> + }
> +
> + closedir(backends);
> + }
> +
> +
> +
> + fprintf(stderr,
> " --shell=MODULE\tShell module, defaults to desktop-shell.so\n"
> " -S, --socket=NAME\tName of socket to listen on\n"
> " -i, --idle-time=SECS\tIdle time in seconds\n"
> @@ -5058,85 +5064,12 @@ usage(int error_code)
> " --no-config\t\tDo not read weston.ini\n"
> " -h, --help\t\tThis help message\n\n");
>
> -#if defined(BUILD_DRM_COMPOSITOR)
> - fprintf(stderr,
> - "Options for drm-backend.so:\n\n"
> - " --connector=ID\tBring up only this connector\n"
> - " --seat=SEAT\t\tThe seat that weston should run on\n"
> - " --tty=TTY\t\tThe tty to use\n"
> - " --use-pixman\t\tUse the pixman (CPU) renderer\n"
> - " --current-mode\tPrefer current KMS mode over EDID preferred mode\n\n");
> -#endif
> -
> -#if defined(BUILD_FBDEV_COMPOSITOR)
> - fprintf(stderr,
> - "Options for fbdev-backend.so:\n\n"
> - " --tty=TTY\t\tThe tty to use\n"
> - " --device=DEVICE\tThe framebuffer device to use\n"
> - " --use-gl\t\tUse the GL renderer\n\n");
> -#endif
> -
> -#if defined(BUILD_X11_COMPOSITOR)
> - fprintf(stderr,
> - "Options for x11-backend.so:\n\n"
> - " --width=WIDTH\t\tWidth of X window\n"
> - " --height=HEIGHT\tHeight of X window\n"
> - " --scale=SCALE\t\tScale factor of output\n"
> - " --fullscreen\t\tRun in fullscreen mode\n"
> - " --use-pixman\t\tUse the pixman (CPU) renderer\n"
> - " --output-count=COUNT\tCreate multiple outputs\n"
> - " --no-input\t\tDont create input devices\n\n");
> -#endif
> -
> -#if defined(BUILD_WAYLAND_COMPOSITOR)
> - fprintf(stderr,
> - "Options for wayland-backend.so:\n\n"
> - " --width=WIDTH\t\tWidth of Wayland surface\n"
> - " --height=HEIGHT\tHeight of Wayland surface\n"
> - " --scale=SCALE\t\tScale factor of output\n"
> - " --fullscreen\t\tRun in fullscreen mode\n"
> - " --use-pixman\t\tUse the pixman (CPU) renderer\n"
> - " --output-count=COUNT\tCreate multiple outputs\n"
> - " --sprawl\t\tCreate one fullscreen output for every parent output\n"
> - " --display=DISPLAY\tWayland display to connect to\n\n");
> -#endif
> -
> -#if defined(BUILD_RPI_COMPOSITOR) && defined(HAVE_BCM_HOST)
> - fprintf(stderr,
> - "Options for rpi-backend.so:\n\n"
> - " --tty=TTY\t\tThe tty to use\n"
> - " --single-buffer\tUse single-buffered Dispmanx elements.\n"
> - " --transform=TR\tThe output transformation, TR is one of:\n"
> - "\tnormal 90 180 270 flipped flipped-90 flipped-180 flipped-270\n"
> - " --opaque-regions\tEnable support for opaque regions, can be "
> - "very slow without support in the GPU firmware.\n"
> - "\n");
> -#endif
> -
> -#if defined(BUILD_RDP_COMPOSITOR)
> - fprintf(stderr,
> - "Options for rdp-backend.so:\n\n"
> - " --width=WIDTH\t\tWidth of desktop\n"
> - " --height=HEIGHT\tHeight of desktop\n"
> - " --env-socket=SOCKET\tUse that socket as peer connection\n"
> - " --address=ADDR\tThe address to bind\n"
> - " --port=PORT\t\tThe port to listen on\n"
> - " --no-clients-resize\tThe RDP peers will be forced to the size of the desktop\n"
> - " --rdp4-key=FILE\tThe file containing the key for RDP4 encryption\n"
> - " --rdp-tls-cert=FILE\tThe file containing the certificate for TLS encryption\n"
> - " --rdp-tls-key=FILE\tThe file containing the private key for TLS encryption\n"
> - "\n");
> -#endif
> + if (backend) {
> + backend_usage = weston_load_module(backend, "backend_usage");
>
> -#if defined(BUILD_HEADLESS_COMPOSITOR)
> - fprintf(stderr,
> - "Options for headless-backend.so:\n\n"
> - " --width=WIDTH\t\tWidth of memory surface\n"
> - " --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");
> -#endif
> + if (backend_usage)
> + fprintf(stderr, "\n%s\n", backend_usage());
> + }
>
> exit(error_code);
> }
> @@ -5320,16 +5253,16 @@ int main(int argc, char *argv[])
>
> parse_options(core_options, ARRAY_LENGTH(core_options), &argc, argv);
>
> + weston_log_file_open(log);
> +
> if (help)
> - usage(EXIT_SUCCESS);
> + usage(backend, EXIT_SUCCESS);
>
> if (version) {
> printf(PACKAGE_STRING "\n");
> return EXIT_SUCCESS;
> }
>
> - weston_log_file_open(log);
> -
> weston_log("%s\n"
> STAMP_SPACE "%s\n"
> STAMP_SPACE "Bug reports to: %s\n"
> diff --git a/src/compositor.h b/src/compositor.h
> index 3586f5b..bf47451 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -1538,6 +1538,9 @@ struct weston_compositor *
> backend_init(struct wl_display *display, int *argc, char *argv[],
> struct weston_config *config);
>
> +const char *
> +backend_usage(void);
> +
> int
> module_init(struct weston_compositor *compositor,
> int *argc, char *argv[]);
> --
> 1.9.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list