[PATCH weston 02/11] compositor: add API to manage compositor instances
Jon A. Cruz
jonc at osg.samsung.com
Tue Jun 23 18:19:38 PDT 2015
Oh I forgot an item on the placement of comments.
On 06/23/2015 04:31 PM, Jon A. Cruz wrote:
>
> Minor doxygen note: the explicit "\brief" is not needed, and the brief
> description should end with a '.' to allow the auto-brief to pick it up.
>
> Overall looks good, but missing a few extra management calls. However
> those seem applicable to patch 4/11.
>
>
> On 06/22/2015 01:02 PM, Giulio Camuffo wrote:
>> This commit adds three new exported functions:
>> - weston_compositor_create() returns a new weston_compositor instance,
>> initializing it as the now removed weston_compositor_init() did.
>> - weston_compositor_exit(compositor) asks the compositor to tear
>> down by calling the compositor's exit vfunc which is set by the
>> libweston application.
>> - weston_compositor_destroy(compositor) is called by the libweston
>> application when tearing down the compositor. The compositor is destroyed
>> and the memory freed.
>> ---
>
> Reviewed-by: Jon A. Cruz <jonc at osg.samsung.com>
>
>> src/compositor-drm.c | 8 +--
>> src/compositor-fbdev.c | 6 --
>> src/compositor-headless.c | 9 +--
>> src/compositor-rdp.c | 4 --
>> src/compositor-wayland.c | 9 +--
>> src/compositor-x11.c | 5 +-
>> src/compositor.c | 167 +++++++++++++++++++++++++++++++++-------------
>> src/compositor.h | 14 +++-
>> 8 files changed, 137 insertions(+), 85 deletions(-)
>>
>> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
>> index 6d24f05..4302f40 100644
>> --- a/src/compositor-drm.c
>> +++ b/src/compositor-drm.c
>> @@ -2424,7 +2424,7 @@ update_outputs(struct drm_backend *b, struct udev_device *drm_device)
>>
>> /* FIXME: handle zero outputs, without terminating */
>> if (b->connector_allocator == 0)
>> - wl_display_terminate(b->compositor->wl_display);
>> + weston_compositor_exit(b->compositor);
>> }
>>
>> static int
>> @@ -2845,12 +2845,6 @@ drm_backend_create(struct weston_compositor *compositor,
>>
>> b->use_pixman = param->use_pixman;
>>
>> - if (weston_compositor_init(compositor, argc, argv,
>> - config) < 0) {
>> - weston_log("%s failed\n", __func__);
>> - goto err_base;
>> - }
>> -
>> /* Check if we run drm-backend using weston-launch */
>> compositor->launcher = weston_launcher_connect(compositor, param->tty,
>> param->seat_id, true);
>> diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
>> index 6a640ee..7117bc5 100644
>> --- a/src/compositor-fbdev.c
>> +++ b/src/compositor-fbdev.c
>> @@ -826,10 +826,6 @@ fbdev_backend_create(struct weston_compositor *compositor, int *argc, char *argv
>> return NULL;
>>
>> backend->compositor = compositor;
>> - if (weston_compositor_init(compositor, argc, argv,
>> - config) < 0)
>> - goto out_free;
>> -
>> if (weston_compositor_set_presentation_clock_software(
>> compositor) < 0)
>> goto out_compositor;
>> @@ -902,8 +898,6 @@ out_udev:
>>
>> out_compositor:
>> weston_compositor_shutdown(compositor);
>> -
>> -out_free:
>> free(backend);
>>
>> return NULL;
>> diff --git a/src/compositor-headless.c b/src/compositor-headless.c
>> index 8e40152..b1be3a0 100644
>> --- a/src/compositor-headless.c
>> +++ b/src/compositor-headless.c
>> @@ -218,8 +218,7 @@ headless_destroy(struct weston_compositor *ec)
>> static struct headless_backend *
>> headless_backend_create(struct weston_compositor *compositor,
>> struct headless_parameters *param,
>> - const char *display_name, int *argc, char *argv[],
>> - struct weston_config *config)
>> + const char *display_name)
>> {
>> struct headless_backend *b;
>>
>> @@ -228,9 +227,6 @@ headless_backend_create(struct weston_compositor *compositor,
>> return NULL;
>>
>> b->compositor = compositor;
>> - if (weston_compositor_init(compositor, argc, argv, config) < 0)
>> - goto err_free;
>> -
>> if (weston_compositor_set_presentation_clock_software(compositor) < 0)
>> goto err_free;
>>
>> @@ -287,8 +283,7 @@ backend_init(struct weston_compositor *compositor,
>> if (weston_parse_transform(transform, ¶m.transform) < 0)
>> weston_log("Invalid transform \"%s\"\n", transform);
>>
>> - b = headless_backend_create(compositor, ¶m, display_name,
>> - argc, argv, config);
>> + b = headless_backend_create(compositor, ¶m, display_name);
>> if (b == NULL)
>> return -1;
>> return 0;
>> diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
>> index 96d4b5f..86c5b2a 100644
>> --- a/src/compositor-rdp.c
>> +++ b/src/compositor-rdp.c
>> @@ -1172,9 +1172,6 @@ rdp_backend_create(struct weston_compositor *compositor,
>> return NULL;
>>
>> b->compositor = compositor;
>> - if (weston_compositor_init(compositor, argc, argv, wconfig) < 0)
>> - goto err_free;
>> -
>> b->base.destroy = rdp_destroy;
>> b->base.restore = rdp_restore;
>> b->rdp_key = config->rdp_key ? strdup(config->rdp_key) : NULL;
>> @@ -1241,7 +1238,6 @@ err_free_strings:
>> free(b->server_cert);
>> if (b->server_key)
>> free(b->server_key);
>> -err_free:
>> free(b);
>> return NULL;
>> }
>> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
>> index 626d8de..35f1de0 100644
>> --- a/src/compositor-wayland.c
>> +++ b/src/compositor-wayland.c
>> @@ -1404,7 +1404,7 @@ input_handle_button(void *data, struct wl_pointer *pointer,
>> input->keyboard_focus = NULL;
>>
>> if (wl_list_empty(&input->backend->compositor->output_list))
>> - wl_display_terminate(input->backend->compositor->wl_display);
>> + weston_compositor_exit(input->backend->compositor);
>>
>> return;
>> }
>> @@ -1852,7 +1852,7 @@ wayland_backend_handle_event(int fd, uint32_t mask, void *data)
>> int count = 0;
>>
>> if ((mask & WL_EVENT_HANGUP) || (mask & WL_EVENT_ERROR)) {
>> - wl_display_terminate(b->compositor->wl_display);
>> + weston_compositor_exit(b->compositor);
>> return 0;
>> }
>>
>> @@ -1960,10 +1960,6 @@ wayland_backend_create(struct weston_compositor *compositor, int use_pixman,
>> return NULL;
>>
>> b->compositor = compositor;
>> - if (weston_compositor_init(compositor, argc, argv,
>> - config) < 0)
>> - goto err_free;
>> -
>> if (weston_compositor_set_presentation_clock_software(compositor) < 0)
>> goto err_compositor;
>>
>> @@ -2032,7 +2028,6 @@ err_display:
>> wl_display_disconnect(b->parent.wl_display);
>> err_compositor:
>> weston_compositor_shutdown(compositor);
>> -err_free:
>> free(b);
>> return NULL;
>> }
>> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
>> index 8ca809a..55c85ed 100644
>> --- a/src/compositor-x11.c
>> +++ b/src/compositor-x11.c
>> @@ -960,7 +960,7 @@ x11_backend_delete_window(struct x11_backend *b, xcb_window_t window)
>> xcb_flush(b->conn);
>>
>> if (wl_list_empty(&b->compositor->output_list))
>> - wl_display_terminate(b->compositor->wl_display);
>> + weston_compositor_exit(b->compositor);
>> }
>>
>> static void delete_cb(void *data)
>> @@ -1534,9 +1534,6 @@ x11_backend_create(struct weston_compositor *compositor,
>> return NULL;
>>
>> b->compositor = compositor;
>> - if (weston_compositor_init(compositor, argc, argv, config) < 0)
>> - goto err_free;
>> -
>> if (weston_compositor_set_presentation_clock_software(compositor) < 0)
>> goto err_free;
>>
>> diff --git a/src/compositor.c b/src/compositor.c
>> index 4e91329..1924e6a 100644
>> --- a/src/compositor.c
>> +++ b/src/compositor.c
>> @@ -4500,16 +4500,27 @@ timeline_key_binding_handler(struct weston_seat *seat, uint32_t time,
>> weston_timeline_open(compositor);
>> }
>>
>> -WL_EXPORT int
>> -weston_compositor_init(struct weston_compositor *ec,
>> - int *argc, char *argv[],
>> - struct weston_config *config)
Aside from some details on the comments themselves (\brief, \ref...) it
is more generally expected to see the doxygen comments in the .h file
where the function is declared, instead of the .c file where it is
implemented.
>> +/** \brief Create the compositor
>> + *
>> + * This functions creates and initializes a compositor instance.
>> + *
>> + * \param display The Wayland display to be used.
>> + * \param user_data A pointer to an object that can later be retrieved
>> + * using the \ref weston_compositor_get_user_data function.
>> + * \return The compositor instance on success or NULL on failure.
>> + */
>> +WL_EXPORT struct weston_compositor *
>> +weston_compositor_create(struct wl_display *display, void *user_data)
>> {
>> + struct weston_compositor *ec;
>> struct wl_event_loop *loop;
>> - struct xkb_rule_names xkb_names;
>> - struct weston_config_section *s;
>>
>> - ec->config = config;
>> + ec = zalloc(sizeof *ec);
>> + if (!ec)
>> + return NULL;
>> +
>> + ec->wl_display = display;
>> + ec->user_data = user_data;
>> wl_signal_init(&ec->destroy_signal);
>> wl_signal_init(&ec->create_surface_signal);
>> wl_signal_init(&ec->activate_signal);
>> @@ -4531,19 +4542,19 @@ weston_compositor_init(struct weston_compositor *ec,
>>
>> if (!wl_global_create(ec->wl_display, &wl_compositor_interface, 3,
>> ec, compositor_bind))
>> - return -1;
>> + goto fail;
>>
>> if (!wl_global_create(ec->wl_display, &wl_subcompositor_interface, 1,
>> ec, bind_subcompositor))
>> - return -1;
>> + goto fail;
>>
>> if (!wl_global_create(ec->wl_display, &wl_scaler_interface, 2,
>> ec, bind_scaler))
>> - return -1;
>> + goto fail;
>>
>> if (!wl_global_create(ec->wl_display, &presentation_interface, 1,
>> ec, bind_presentation))
>> - return -1;
>> + goto fail;
>>
>> wl_list_init(&ec->view_list);
>> wl_list_init(&ec->plane_list);
>> @@ -4560,7 +4571,39 @@ weston_compositor_init(struct weston_compositor *ec,
>> weston_plane_init(&ec->primary_plane, ec, 0, 0);
>> weston_compositor_stack_plane(ec, &ec->primary_plane, NULL);
>>
>> - s = weston_config_get_section(ec->config, "keyboard", NULL, NULL);
>> + wl_data_device_manager_init(ec->wl_display);
>> +
>> + wl_display_init_shm(ec->wl_display);
>> +
>> + loop = wl_display_get_event_loop(ec->wl_display);
>> + ec->idle_source = wl_event_loop_add_timer(loop, idle_handler, ec);
>> + wl_event_source_timer_update(ec->idle_source, ec->idle_time * 1000);
>> +
>> + ec->input_loop = wl_event_loop_create();
>> +
>> + weston_layer_init(&ec->fade_layer, &ec->layer_list);
>> + weston_layer_init(&ec->cursor_layer, &ec->fade_layer.link);
>> +
>> + weston_compositor_add_debug_binding(ec, KEY_T,
>> + timeline_key_binding_handler, ec);
>> +
>> + weston_compositor_schedule_repaint(ec);
>> +
>> + return ec;
>> +
>> +fail:
>> + free(ec);
>> + return NULL;
>> +}
>> +
>> +static int
>> +weston_compositor_init_config(struct weston_compositor *ec,
>> + struct weston_config *config)
>> +{
>> + struct xkb_rule_names xkb_names;
>> + struct weston_config_section *s;
>> +
>> + s = weston_config_get_section(config, "keyboard", NULL, NULL);
>> weston_config_section_get_string(s, "keymap_rules",
>> (char **) &xkb_names.rules, NULL);
>> weston_config_section_get_string(s, "keymap_model",
>> @@ -4580,23 +4623,7 @@ weston_compositor_init(struct weston_compositor *ec,
>> weston_config_section_get_int(s, "repeat-delay",
>> &ec->kb_repeat_delay, 400);
>>
>> - wl_data_device_manager_init(ec->wl_display);
>> -
>> - wl_display_init_shm(ec->wl_display);
>> -
>> - loop = wl_display_get_event_loop(ec->wl_display);
>> - ec->idle_source = wl_event_loop_add_timer(loop, idle_handler, ec);
>> - wl_event_source_timer_update(ec->idle_source, ec->idle_time * 1000);
>> -
>> - ec->input_loop = wl_event_loop_create();
>> -
>> - weston_layer_init(&ec->fade_layer, &ec->layer_list);
>> - weston_layer_init(&ec->cursor_layer, &ec->fade_layer.link);
>> -
>> - weston_compositor_add_debug_binding(ec, KEY_T,
>> - timeline_key_binding_handler, ec);
>> -
>> - s = weston_config_get_section(ec->config, "core", NULL, NULL);
>> + s = weston_config_get_section(config, "core", NULL, NULL);
>> weston_config_section_get_int(s, "repaint-window", &ec->repaint_msec,
>> DEFAULT_REPAINT_WINDOW);
>> if (ec->repaint_msec < -10 || ec->repaint_msec > 1000) {
>> @@ -4607,8 +4634,6 @@ weston_compositor_init(struct weston_compositor *ec,
>> weston_log("Output repaint window is %d ms maximum.\n",
>> ec->repaint_msec);
>>
>> - weston_compositor_schedule_repaint(ec);
>> -
>> return 0;
>> }
>>
>> @@ -4637,8 +4662,6 @@ weston_compositor_shutdown(struct weston_compositor *ec)
>> weston_plane_release(&ec->primary_plane);
>>
>> wl_event_loop_destroy(ec->input_loop);
>> -
>> - weston_config_destroy(ec->config);
>> }
>>
>> WL_EXPORT void
>> @@ -4648,7 +4671,7 @@ weston_compositor_exit_with_code(struct weston_compositor *compositor,
>> if (compositor->exit_code == EXIT_SUCCESS)
>> compositor->exit_code = exit_code;
>>
>> - wl_display_terminate(compositor->wl_display);
>> + weston_compositor_exit(compositor);
>> }
>>
>> WL_EXPORT void
>> @@ -5267,6 +5290,12 @@ load_configuration(struct weston_config **config, int32_t noconfig,
>> return 0;
>> }
>>
>> +static void
>> +handle_exit(struct weston_compositor *c)
>> +{
>> + wl_display_terminate(c->wl_display);
>> +}
>> +
>> int main(int argc, char *argv[])
>> {
>> int ret = EXIT_FAILURE;
>> @@ -5291,7 +5320,7 @@ int main(int argc, char *argv[])
>> int32_t noconfig = 0;
>> int32_t numlock_on;
>> char *config_file = NULL;
>> - struct weston_config *config;
>> + struct weston_config *config = NULL;
>> struct weston_config_section *section;
>> struct wl_client *primary_client;
>> struct wl_listener primary_client_destroyed;
>> @@ -5365,13 +5394,18 @@ int main(int argc, char *argv[])
>> if (!backend_init)
>> goto out_signals;
>>
>> - ec = zalloc(sizeof *ec);
>> + ec = weston_compositor_create(display, NULL);
>> if (ec == NULL) {
>> weston_log("fatal: failed to create compositor\n");
>> goto out_signals;
>> }
>>
>> - ec->wl_display = display;
>> + ec->config = config;
>> + if (weston_compositor_init_config(ec, config) < 0) {
>> + ret = EXIT_FAILURE;
>> + goto out_signals;
>> + }
>> +
>> if (backend_init(ec, &argc, argv, config) < 0) {
>> weston_log("fatal: failed to create compositor backend\n");
>> ret = EXIT_FAILURE;
>> @@ -5388,6 +5422,7 @@ int main(int argc, char *argv[])
>> ec->idle_time = idle_time;
>> ec->default_pointer_grab = NULL;
>> ec->exit_code = EXIT_SUCCESS;
>> + ec->exit = handle_exit;
>>
>> weston_compositor_log_capabilities(ec);
>>
>> @@ -5458,15 +5493,7 @@ int main(int argc, char *argv[])
>> ret = ec->exit_code;
>>
>> out:
>> - /* prevent further rendering while shutting down */
>> - ec->state = WESTON_COMPOSITOR_OFFSCREEN;
>> -
>> - wl_signal_emit(&ec->destroy_signal, ec);
>> -
>> - weston_compositor_xkb_destroy(ec);
>> -
>> - ec->backend->destroy(ec);
>> - free(ec);
>> + weston_compositor_destroy(ec);
>>
>> out_signals:
>> for (i = ARRAY_LENGTH(signals) - 1; i >= 0; i--)
>> @@ -5474,7 +5501,8 @@ out_signals:
>> wl_event_source_remove(signals[i]);
>>
>> wl_display_destroy(display);
>> -
>> + if (config)
>> + weston_config_destroy(config);
>> weston_log_file_close();
>>
>> free(config_file);
>> @@ -5487,3 +5515,48 @@ out_signals:
>>
>> return ret;
>> }
See earlier note on placement in .h instead of .c
>> +
>> +/** \brief Destroys the compositor.
>> + *
>> + * This function cleans up the compositor state and destroys it.
>> + *
>> + * \param compositor The compositor to be destroyed.
>> + */
>> +WL_EXPORT void
>> +weston_compositor_destroy(struct weston_compositor *compositor)
>> +{
>> + /* prevent further rendering while shutting down */
>> + compositor->state = WESTON_COMPOSITOR_OFFSCREEN;
>> +
>> + wl_signal_emit(&compositor->destroy_signal, compositor);
>> +
>> + weston_compositor_xkb_destroy(compositor);
>> +
>> + compositor->backend->destroy(compositor);
>> + free(compositor);
>> +}
>> +
See earlier note on placement in .h instead of .c
>> +/** \brief Instruct the compositor to exit
>> + *
>> + * This functions does not directly destroy the compositor object, it merely
>> + * command it to start the tear down process. It is not guaranteed that the
>> + * tear down will happen immediately.
>> + *
>> + * \param compositor The compositor to tear down.
>> + */
>> +WL_EXPORT void
>> +weston_compositor_exit(struct weston_compositor *compositor)
>> +{
>> + compositor->exit(compositor);
>> +}
>> +
See earlier note on placement in .h instead of .c
>> +/** \brief Return the user data stored in the compositor
>> + *
>> + * This function returns the user data pointer set with user_data parameter
>> + * to the \ref weston_compositor_create function.
>> + */
>> +WL_EXPORT void *
>> +weston_compositor_get_user_data(struct weston_compositor *compositor)
>> +{
>> + return compositor->user_data;
>> +}
>> diff --git a/src/compositor.h b/src/compositor.h
>> index 576bb9c..df2b563 100644
>> --- a/src/compositor.h
>> +++ b/src/compositor.h
>> @@ -689,6 +689,9 @@ struct weston_compositor {
>> int32_t repaint_msec;
>>
>> int exit_code;
>> +
>> + void *user_data;
>> + void (*exit)(struct weston_compositor *c);
>> };
>>
>> struct weston_buffer {
>> @@ -1337,9 +1340,14 @@ weston_buffer_reference(struct weston_buffer_reference *ref,
>> uint32_t
>> weston_compositor_get_time(void);
>>
>> -int
>> -weston_compositor_init(struct weston_compositor *ec,
>> - int *argc, char *argv[], struct weston_config *config);
>> +void
>> +weston_compositor_destroy(struct weston_compositor *ec);
>> +struct weston_compositor *
>> +weston_compositor_create(struct wl_display *display, void *user_data);
>> +void
>> +weston_compositor_exit(struct weston_compositor *ec);
>> +void *
>> +weston_compositor_get_user_data(struct weston_compositor *compositor);
>> int
>> weston_compositor_set_presentation_clock(struct weston_compositor *compositor,
>> clockid_t clk_id);
>>
>
--
Jon A. Cruz - Senior Open Source Developer
Samsung Open Source Group
jonc at osg.samsung.com
More information about the wayland-devel
mailing list