Re: [PATCH weston 02/11] compositor: add API to manage compositor instances
On Tue, 23 Jun 2015 18:19:38 -0700 Jon A. Cruz j...@osg.samsung.com wrote: 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 j...@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.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. Seriously? Personally I prefer documentation to reside with the implementation, so that it is less easy to forget to update the docs. The compiler will catch forgotten declaration, but not docs. Maybe I picked it up from the Linux coding style. Yes, I did merge your patch which does it differently than the whole rest of existing Weston and Wayland. That was an oversight. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 02/11] compositor: add API to manage compositor instances
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 j...@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, param.transform) 0) weston_log(Invalid transform \%s\\n, transform); -b = headless_backend_create(compositor, param, display_name, -argc, argv, config); +b = headless_backend_create(compositor, param, 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 =
Re: [PATCH weston 02/11] compositor: add API to manage compositor instances
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 j...@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, param.transform) 0) weston_log(Invalid transform \%s\\n, transform); - b = headless_backend_create(compositor, param, display_name, - argc, argv, config); + b = headless_backend_create(compositor, param, 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 =