[PATCH 2/3] renderer: introduce destroy callback
John Kåre Alsaker
john.kare.alsaker at gmail.com
Tue Jan 8 11:50:28 PST 2013
I'd like for output_create, output_destroy, renderer_destroy,
set_border and repaint_output to be virtual functions as part of a new
abstraction which is responsible for handling buffers and renderers
for the backends. These may simply be new fields in weston_renderer
that the backends override as necessary, or a new structure private to
the backends which the renderers may partially initialize.
This way we can replace this nice peace of code
if (c->use_pixman) {
if (x11_output_init_shm(c, output, width, height) < 0)
return NULL;
if (pixman_renderer_output_create(&output->base) < 0) {
x11_output_deinit_shm(c, output);
return NULL;
}
} else {
if (gl_renderer_output_create(&output->base, output->window) < 0)
return NULL;
}
with this
if (buff_manager->output_create(&output->base, output->window) < 0)
return NULL;
and generally reduce the number of branches. Which will help making
EGL/gl_renderer switchable.
On Tue, Jan 8, 2013 at 8:22 PM, Kristian Høgsberg <krh at bitplanet.net> wrote:
> On Tue, Jan 8, 2013 at 1:36 PM, John Kåre Alsaker
> <john.kare.alsaker at gmail.com> wrote:
>> I'd like to see this callback in a buffer manager abstraction which
>> could encapsulate EGL and X11/fb/kms buffer management and get rid of
>> duplicated code for the renderers. However if it's added to
>> weston_renderer, it should be destroyed by the compositor and not by
>> the backends.
>
> What code duplication do you see? There's very little left; only
> compositor-drm creates egl on kms buffers, only compositor-x11 creates
> X windows... for the pixman renderer, we'll need X shm buffers for X
> and kms dumb buffers for compsitor-drm so there's not a lot of
> commonality there either. Also, since the backend creates the
> renderer it only makes sense that it destroys it. The reason for the
> function pointer is that we can now have different renderers in play
> in one backend, and instead of doing #ifdef EGL if (using gl renderer)
> destroy_gl_renderer else #else blah blah, we can just call the
> renderer destroy vfunc. It's just a small pragmatic cleanup, not a
> brand new abstraction.
>
> Kristian
>
>> On Tue, Jan 8, 2013 at 5:09 PM, Vasily Khoruzhick <anarsoul at gmail.com> wrote:
>>> Signed-off-by: Vasily Khoruzhick <anarsoul at gmail.com>
>>> ---
>>> src/compositor-android.c | 6 +++---
>>> src/compositor-drm.c | 4 ++--
>>> src/compositor-headless.c | 2 +-
>>> src/compositor-rpi.c | 4 ++--
>>> src/compositor-wayland.c | 4 ++--
>>> src/compositor-x11.c | 10 ++--------
>>> src/compositor.h | 3 +--
>>> src/gl-renderer.c | 3 ++-
>>> src/gl-renderer.h | 2 --
>>> src/noop-renderer.c | 3 ++-
>>> src/pixman-renderer.c | 3 ++-
>>> src/pixman-renderer.h | 3 ---
>>> 12 files changed, 19 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/src/compositor-android.c b/src/compositor-android.c
>>> index 5e27071..4e4ff08 100644
>>> --- a/src/compositor-android.c
>>> +++ b/src/compositor-android.c
>>> @@ -299,7 +299,7 @@ android_init_egl(struct android_compositor *compositor,
>>>
>>> if (gl_renderer_output_create(&output->base,
>>> output->fb->native_window) < 0) {
>>> - gl_renderer_destroy(&compositor->base);
>>> + compositor->base.renderer->destroy(&compositor->base);
>>> return -1;
>>> }
>>>
>>> @@ -313,7 +313,7 @@ android_compositor_destroy(struct weston_compositor *base)
>>>
>>> android_seat_destroy(compositor->seat);
>>>
>>> - gl_renderer_destroy(base);
>>> + base->renderer->destroy(base);
>>>
>>> /* destroys outputs, too */
>>> weston_compositor_shutdown(&compositor->base);
>>> @@ -358,7 +358,7 @@ android_compositor_create(struct wl_display *display, int argc, char *argv[],
>>> return &compositor->base;
>>>
>>> err_gl:
>>> - gl_renderer_destroy(&compositor->base);
>>> + compositor->base.renderer->destroy(&compositor->base);
>>> err_output:
>>> android_output_destroy(&output->base);
>>> err_compositor:
>>> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
>>> index cc72c74..1c17bfd 100644
>>> --- a/src/compositor-drm.c
>>> +++ b/src/compositor-drm.c
>>> @@ -2007,7 +2007,7 @@ drm_destroy(struct weston_compositor *ec)
>>>
>>> weston_compositor_shutdown(ec);
>>>
>>> - gl_renderer_destroy(ec);
>>> + ec->renderer->destroy(ec);
>>>
>>> destroy_sprites(d);
>>> gbm_device_destroy(d->gbm);
>>> @@ -2307,7 +2307,7 @@ err_drm_source:
>>> wl_list_for_each_safe(weston_seat, next, &ec->base.seat_list, link)
>>> evdev_input_destroy(weston_seat);
>>> err_sprite:
>>> - gl_renderer_destroy(&ec->base);
>>> + ec->base.renderer->destroy(&ec->base);
>>> gbm_device_destroy(ec->gbm);
>>> destroy_sprites(ec);
>>> err_udev_dev:
>>> diff --git a/src/compositor-headless.c b/src/compositor-headless.c
>>> index d23ee0a..03bd0a4 100644
>>> --- a/src/compositor-headless.c
>>> +++ b/src/compositor-headless.c
>>> @@ -141,7 +141,7 @@ headless_destroy(struct weston_compositor *ec)
>>> {
>>> struct headless_compositor *c = (struct headless_compositor *) ec;
>>>
>>> - noop_renderer_destroy(ec);
>>> + ec->renderer->destroy(ec);
>>>
>>> weston_seat_release(&c->fake_seat);
>>> weston_compositor_shutdown(ec);
>>> diff --git a/src/compositor-rpi.c b/src/compositor-rpi.c
>>> index f169d3b..208271f 100644
>>> --- a/src/compositor-rpi.c
>>> +++ b/src/compositor-rpi.c
>>> @@ -1347,7 +1347,7 @@ rpi_compositor_destroy(struct weston_compositor *base)
>>> /* destroys outputs, too */
>>> weston_compositor_shutdown(&compositor->base);
>>>
>>> - gl_renderer_destroy(&compositor->base);
>>> + compositor->base.renderer->destroy(&compositor->base);
>>> tty_destroy(compositor->tty);
>>>
>>> bcm_host_deinit();
>>> @@ -1501,7 +1501,7 @@ rpi_compositor_create(struct wl_display *display, int argc, char *argv[],
>>> return &compositor->base;
>>>
>>> out_gl:
>>> - gl_renderer_destroy(&compositor->base);
>>> + compositor->base.renderer->destroy(&compositor->base);
>>>
>>> out_tty:
>>> tty_destroy(compositor->tty);
>>> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
>>> index 167fce6..0a3df03 100644
>>> --- a/src/compositor-wayland.c
>>> +++ b/src/compositor-wayland.c
>>> @@ -612,7 +612,7 @@ wayland_restore(struct weston_compositor *ec)
>>> static void
>>> wayland_destroy(struct weston_compositor *ec)
>>> {
>>> - gl_renderer_destroy(ec);
>>> + ec->renderer->destroy(ec);
>>>
>>> weston_compositor_shutdown(ec);
>>>
>>> @@ -686,7 +686,7 @@ wayland_compositor_create(struct wl_display *display,
>>> return &c->base;
>>>
>>> err_gl:
>>> - gl_renderer_destroy(&c->base);
>>> + c->base.renderer->destroy(&c->base);
>>> err_display:
>>> wl_display_disconnect(c->parent.wl_display);
>>> err_compositor:
>>> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
>>> index a5fc9ef..c5c4248 100644
>>> --- a/src/compositor-x11.c
>>> +++ b/src/compositor-x11.c
>>> @@ -1419,10 +1419,7 @@ x11_destroy(struct weston_compositor *ec)
>>>
>>> weston_compositor_shutdown(ec); /* destroys outputs, too */
>>>
>>> - if (compositor->use_pixman)
>>> - pixman_renderer_destroy(ec);
>>> - else
>>> - gl_renderer_destroy(ec);
>>> + ec->renderer->destroy(ec);
>>>
>>> XCloseDisplay(compositor->dpy);
>>> free(ec);
>>> @@ -1532,10 +1529,7 @@ x11_compositor_create(struct wl_display *display,
>>> err_x11_input:
>>> x11_input_destroy(c);
>>> err_renderer:
>>> - if (c->use_pixman)
>>> - pixman_renderer_destroy(&c->base);
>>> - else
>>> - gl_renderer_destroy(&c->base);
>>> + c->base.renderer->destroy(&c->base);
>>> err_xdisplay:
>>> XCloseDisplay(c->dpy);
>>> err_free:
>>> diff --git a/src/compositor.h b/src/compositor.h
>>> index 15d6939..544cf33 100644
>>> --- a/src/compositor.h
>>> +++ b/src/compositor.h
>>> @@ -273,6 +273,7 @@ struct weston_renderer {
>>> float red, float green,
>>> float blue, float alpha);
>>> void (*destroy_surface)(struct weston_surface *surface);
>>> + void (*destroy)(struct weston_compositor *ec);
>>> };
>>>
>>> struct weston_compositor {
>>> @@ -821,8 +822,6 @@ weston_output_switch_mode(struct weston_output *output, struct weston_mode *mode
>>>
>>> int
>>> noop_renderer_init(struct weston_compositor *ec);
>>> -void
>>> -noop_renderer_destroy(struct weston_compositor *ec);
>>>
>>> struct weston_compositor *
>>> backend_init(struct wl_display *display, int argc, char *argv[],
>>> diff --git a/src/gl-renderer.c b/src/gl-renderer.c
>>> index 8da111e..ee2763a 100644
>>> --- a/src/gl-renderer.c
>>> +++ b/src/gl-renderer.c
>>> @@ -1648,7 +1648,7 @@ gl_renderer_output_surface(struct weston_output *output)
>>> return get_output_state(output)->egl_surface;
>>> }
>>>
>>> -WL_EXPORT void
>>> +static void
>>> gl_renderer_destroy(struct weston_compositor *ec)
>>> {
>>> struct gl_renderer *gr = get_renderer(ec);
>>> @@ -1748,6 +1748,7 @@ gl_renderer_create(struct weston_compositor *ec, EGLNativeDisplayType display,
>>> gr->base.create_surface = gl_renderer_create_surface;
>>> gr->base.surface_set_color = gl_renderer_surface_set_color;
>>> gr->base.destroy_surface = gl_renderer_destroy_surface;
>>> + gr->base.destroy = gl_renderer_destroy;
>>>
>>> gr->egl_display = eglGetDisplay(display);
>>> if (gr->egl_display == EGL_NO_DISPLAY) {
>>> diff --git a/src/gl-renderer.h b/src/gl-renderer.h
>>> index 09c58c5..1186348 100644
>>> --- a/src/gl-renderer.h
>>> +++ b/src/gl-renderer.h
>>> @@ -42,8 +42,6 @@ gl_renderer_output_surface(struct weston_output *output);
>>> void
>>> gl_renderer_set_border(struct weston_compositor *ec, int32_t width, int32_t height, void *data,
>>> int32_t *edges);
>>> -void
>>> -gl_renderer_destroy(struct weston_compositor *ec);
>>>
>>> void
>>> gl_renderer_print_egl_error_state(void);
>>> diff --git a/src/noop-renderer.c b/src/noop-renderer.c
>>> index 24660ec..a0b1679 100644
>>> --- a/src/noop-renderer.c
>>> +++ b/src/noop-renderer.c
>>> @@ -68,7 +68,7 @@ noop_renderer_destroy_surface(struct weston_surface *surface)
>>> {
>>> }
>>>
>>> -WL_EXPORT void
>>> +static void
>>> noop_renderer_destroy(struct weston_compositor *ec)
>>> {
>>> free(ec->renderer);
>>> @@ -91,6 +91,7 @@ noop_renderer_init(struct weston_compositor *ec)
>>> renderer->create_surface = noop_renderer_create_surface;
>>> renderer->surface_set_color = noop_renderer_surface_set_color;
>>> renderer->destroy_surface = noop_renderer_destroy_surface;
>>> + renderer->destroy = noop_renderer_destroy;
>>> ec->renderer = renderer;
>>>
>>> return 0;
>>> diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c
>>> index 1747408..6c62965 100644
>>> --- a/src/pixman-renderer.c
>>> +++ b/src/pixman-renderer.c
>>> @@ -312,7 +312,7 @@ pixman_renderer_destroy_surface(struct weston_surface *surface)
>>> free(ps);
>>> }
>>>
>>> -WL_EXPORT void
>>> +static void
>>> pixman_renderer_destroy(struct weston_compositor *ec)
>>> {
>>> free(ec->renderer);
>>> @@ -335,6 +335,7 @@ pixman_renderer_init(struct weston_compositor *ec)
>>> renderer->create_surface = pixman_renderer_create_surface;
>>> renderer->surface_set_color = pixman_renderer_surface_set_color;
>>> renderer->destroy_surface = pixman_renderer_destroy_surface;
>>> + renderer->destroy = pixman_renderer_destroy;
>>> ec->renderer = renderer;
>>>
>>> return 0;
>>> diff --git a/src/pixman-renderer.h b/src/pixman-renderer.h
>>> index 9920999..77761ba 100644
>>> --- a/src/pixman-renderer.h
>>> +++ b/src/pixman-renderer.h
>>> @@ -22,9 +22,6 @@
>>>
>>> #include "compositor.h"
>>>
>>> -void
>>> -pixman_renderer_destroy(struct weston_compositor *ec);
>>> -
>>> int
>>> pixman_renderer_init(struct weston_compositor *ec);
>>>
>>> --
>>> 1.8.1
>>>
>>> _______________________________________________
>>> wayland-devel mailing list
>>> wayland-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>> _______________________________________________
>> 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