[Mesa-dev] [PATCH v2 2/8] egl: refactor color_buffers structure for deduplicating

Gurchetan Singh gurchetansingh at chromium.org
Tue Oct 17 20:49:07 UTC 2017


Can you wrap color_buffers around:

#if defined(HAVE_WAYLAND_PLATFORM) || defined(HAVE_DRM_PLATFORM)
|| defined(HAVE_ANDROID_PLATFORM)

flags?  This is because platform_surfaceless has no native surfaces and
it's good to be explicit in that regard.

On Fri, Oct 6, 2017 at 2:38 PM, Gwan-gyeong Mun <elongbug at gmail.com> wrote:

> This is added for preventing adding of new color buffers structure and
> back*
> when new platform backend is added.
> This refactoring separates out the common and platform specific bits.
> This makes odd casting in the platform_foo.c but it prevents adding of new
> ifdef magic.
> Because of color_buffers array size is different on android and wayland
> /drm,
> it adds COLOR_BUFFERS_SIZE macro.
>  - android's color buffers array size is 3.
>    drm & wayland's color buffers array size is 4.
>
> Fixes from Rob's review:
>  - refactor to separate out the common and platform specific bits.
>
> Fixes from Emil's review:
>  - use suggested color buffers structure shape.
>    it makes a buffer pointer of each platform to void pointer type.
>
> Signed-off-by: Mun Gwan-gyeong <elongbug at gmail.com>
> ---
>  src/egl/drivers/dri2/egl_dri2.h         | 30 +++++++++---------
>  src/egl/drivers/dri2/platform_android.c | 10 +++---
>  src/egl/drivers/dri2/platform_drm.c     | 55
> +++++++++++++++++----------------
>  src/egl/drivers/dri2/platform_wayland.c | 46 +++++++++++++--------------
>  4 files changed, 71 insertions(+), 70 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.h
> b/src/egl/drivers/dri2/egl_dri2.h
> index 017895f0d9..08ccf24410 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -65,6 +65,15 @@ struct zwp_linux_dmabuf_v1;
>
>  #endif /* HAVE_ANDROID_PLATFORM */
>
> +#if defined(HAVE_WAYLAND_PLATFORM) || defined(HAVE_DRM_PLATFORM)
> +#define COLOR_BUFFERS_SIZE 4
> +#else
> +       /* Usually Android uses at most triple buffers in ANativeWindow
> +        * so hardcode the number of color_buffers to 3.
> +        */
> +#define COLOR_BUFFERS_SIZE 3
> +#endif
> +
>  #include "eglconfig.h"
>  #include "eglcontext.h"
>  #include "egldisplay.h"
> @@ -286,39 +295,28 @@ struct dri2_egl_surface
>     /* EGL-owned buffers */
>     __DRIbuffer           *local_buffers[__DRI_BUFFER_COUNT];
>
> -#if defined(HAVE_WAYLAND_PLATFORM) || defined(HAVE_DRM_PLATFORM)
> +   /* Used to record all the buffers created by each platform's native
> window
> +    * and their ages.
> +    */
>     struct {
> +      void *native_buffer; // aka wl_buffer/gbm_bo/ANativeWindowBuffer
>  #ifdef HAVE_WAYLAND_PLATFORM
> -      struct wl_buffer   *wl_buffer;
>        __DRIimage         *dri_image;
>        /* for is_different_gpu case. NULL else */
>        __DRIimage         *linear_copy;
>        /* for swrast */
>        void *data;
>        int data_size;
> -#endif
> -#ifdef HAVE_DRM_PLATFORM
> -      struct gbm_bo       *bo;
>  #endif
>        bool                locked;
>        int                 age;
> -   } color_buffers[4], *back, *current;
> -#endif
> +   } color_buffers[COLOR_BUFFERS_SIZE], *back, *current;
>
>  #ifdef HAVE_ANDROID_PLATFORM
>     struct ANativeWindow *window;
>     struct ANativeWindowBuffer *buffer;
>     __DRIimage *dri_image_back;
>     __DRIimage *dri_image_front;
> -
> -   /* Used to record all the buffers created by ANativeWindow and their
> ages.
> -    * Usually Android uses at most triple buffers in ANativeWindow
> -    * so hardcode the number of color_buffers to 3.
> -    */
> -   struct {
> -      struct ANativeWindowBuffer *buffer;
> -      int age;
> -   } color_buffers[3], *back;
>  #endif
>
>  #if defined(HAVE_SURFACELESS_PLATFORM)
> diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> index 0acbb38bd8..67e739c1fc 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -193,10 +193,10 @@ droid_window_dequeue_buffer(struct dri2_egl_surface
> *dri2_surf)
>      */
>     EGLBoolean updated = EGL_FALSE;
>     for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> -      if (!dri2_surf->color_buffers[i].buffer) {
> -         dri2_surf->color_buffers[i].buffer = dri2_surf->buffer;
> +      if (!dri2_surf->color_buffers[i].native_buffer) {
> +         dri2_surf->color_buffers[i].native_buffer = (void
> *)dri2_surf->buffer;
>        }
> -      if (dri2_surf->color_buffers[i].buffer == dri2_surf->buffer) {
> +      if (dri2_surf->color_buffers[i].native_buffer == (void
> *)dri2_surf->buffer) {
>           dri2_surf->back = &dri2_surf->color_buffers[i];
>           updated = EGL_TRUE;
>           break;
> @@ -208,10 +208,10 @@ droid_window_dequeue_buffer(struct dri2_egl_surface
> *dri2_surf)
>         * the color_buffers
>         */
>        for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> -         dri2_surf->color_buffers[i].buffer = NULL;
> +         dri2_surf->color_buffers[i].native_buffer = NULL;
>           dri2_surf->color_buffers[i].age = 0;
>        }
> -      dri2_surf->color_buffers[0].buffer = dri2_surf->buffer;
> +      dri2_surf->color_buffers[0].native_buffer = (void
> *)dri2_surf->buffer;
>        dri2_surf->back = &dri2_surf->color_buffers[0];
>     }
>
> diff --git a/src/egl/drivers/dri2/platform_drm.c
> b/src/egl/drivers/dri2/platform_drm.c
> index 9005f5dd9e..3527352bab 100644
> --- a/src/egl/drivers/dri2/platform_drm.c
> +++ b/src/egl/drivers/dri2/platform_drm.c
> @@ -53,7 +53,7 @@ lock_front_buffer(struct gbm_surface *_surf)
>        return NULL;
>     }
>
> -   bo = dri2_surf->current->bo;
> +   bo = (struct gbm_bo *)dri2_surf->current->native_buffer;
>
>     if (device->dri2) {
>        dri2_surf->current->locked = true;
> @@ -70,7 +70,7 @@ release_buffer(struct gbm_surface *_surf, struct gbm_bo
> *bo)
>     struct dri2_egl_surface *dri2_surf = surf->dri_private;
>
>     for (unsigned i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> -      if (dri2_surf->color_buffers[i].bo == bo) {
> +      if (dri2_surf->color_buffers[i].native_buffer == (void *)bo) {
>          dri2_surf->color_buffers[i].locked = false;
>          break;
>        }
> @@ -172,8 +172,8 @@ dri2_drm_destroy_surface(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *surf)
>     dri2_dpy->core->destroyDrawable(dri2_surf->dri_drawable);
>
>     for (unsigned i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> -      if (dri2_surf->color_buffers[i].bo)
> -        gbm_bo_destroy(dri2_surf->color_buffers[i].bo);
> +      if (dri2_surf->color_buffers[i].native_buffer)
> +        gbm_bo_destroy((struct gbm_bo *)dri2_surf->color_buffers[i].
> native_buffer);
>     }
>
>     dri2_egl_surface_free_local_buffers(dri2_surf);
> @@ -204,23 +204,24 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
>
>     if (dri2_surf->back == NULL)
>        return -1;
> -   if (dri2_surf->back->bo == NULL) {
> +   if (dri2_surf->back->native_buffer == NULL) {
>        if (surf->base.modifiers)
> -         dri2_surf->back->bo = gbm_bo_create_with_modifiers(&
> dri2_dpy->gbm_dri->base,
> -
> surf->base.width,
> -
> surf->base.height,
> -
> surf->base.format,
> -
> surf->base.modifiers,
> -
> surf->base.count);
> +         dri2_surf->back->native_buffer =
> +            (void *)gbm_bo_create_with_modifiers
> (&dri2_dpy->gbm_dri->base,
> +                                                 surf->base.width,
> +                                                 surf->base.height,
> +                                                 surf->base.format,
> +                                                 surf->base.modifiers,
> +                                                 surf->base.count);
>        else
> -         dri2_surf->back->bo = gbm_bo_create(&dri2_dpy->gbm_dri->base,
> -                                             surf->base.width,
> -                                             surf->base.height,
> -                                             surf->base.format,
> -                                             surf->base.flags);
> +         dri2_surf->back->native_buffer = (void
> *)gbm_bo_create(&dri2_dpy->gbm_dri->base,
> +
> surf->base.width,
> +
> surf->base.height,
> +
> surf->base.format,
> +
> surf->base.flags);
>
>     }
> -   if (dri2_surf->back->bo == NULL)
> +   if (dri2_surf->back->native_buffer == NULL)
>        return -1;
>
>     return 0;
> @@ -238,11 +239,13 @@ get_swrast_front_bo(struct dri2_egl_surface
> *dri2_surf)
>        dri2_surf->current = &dri2_surf->color_buffers[0];
>     }
>
> -   if (dri2_surf->current->bo == NULL)
> -      dri2_surf->current->bo = gbm_bo_create(&dri2_dpy->gbm_dri->base,
> -                                             surf->base.width,
> surf->base.height,
> -                                             surf->base.format,
> surf->base.flags);
> -   if (dri2_surf->current->bo == NULL)
> +   if (dri2_surf->current->native_buffer == NULL)
> +      dri2_surf->current->native_buffer = (void
> *)gbm_bo_create(&dri2_dpy->gbm_dri->base,
> +
> surf->base.width,
> +
> surf->base.height,
> +
> surf->base.format,
> +
> surf->base.flags);
> +   if (dri2_surf->current->native_buffer == NULL)
>        return -1;
>
>     return 0;
> @@ -256,7 +259,7 @@ back_bo_to_dri_buffer(struct dri2_egl_surface
> *dri2_surf, __DRIbuffer *buffer)
>     struct gbm_dri_bo *bo;
>     int name, pitch;
>
> -   bo = (struct gbm_dri_bo *) dri2_surf->back->bo;
> +   bo = (struct gbm_dri_bo *) dri2_surf->back->native_buffer;
>
>     dri2_dpy->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_NAME,
> &name);
>     dri2_dpy->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_STRIDE,
> &pitch);
> @@ -360,7 +363,7 @@ dri2_drm_image_get_buffers(__DRIdrawable *driDrawable,
>     if (get_back_bo(dri2_surf) < 0)
>        return 0;
>
> -   bo = (struct gbm_dri_bo *) dri2_surf->back->bo;
> +   bo = (struct gbm_dri_bo *) dri2_surf->back->native_buffer;
>     buffers->image_mask = __DRI_IMAGE_BUFFER_BACK;
>     buffers->back = bo->image;
>
> @@ -496,7 +499,7 @@ swrast_put_image2(__DRIdrawable *driDrawable,
>     if (get_swrast_front_bo(dri2_surf) < 0)
>        return;
>
> -   bo = gbm_dri_bo(dri2_surf->current->bo);
> +   bo = gbm_dri_bo(dri2_surf->current->native_buffer);
>
>     bpp = gbm_bo_get_bpp(&bo->base);
>     if (bpp == 0)
> @@ -541,7 +544,7 @@ swrast_get_image(__DRIdrawable *driDrawable,
>     if (get_swrast_front_bo(dri2_surf) < 0)
>        return;
>
> -   bo = gbm_dri_bo(dri2_surf->current->bo);
> +   bo = gbm_dri_bo(dri2_surf->current->native_buffer);
>
>     bpp = gbm_bo_get_bpp(&bo->base);
>     if (bpp == 0)
> diff --git a/src/egl/drivers/dri2/platform_wayland.c
> b/src/egl/drivers/dri2/platform_wayland.c
> index 14db55ca74..1518a24b7c 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -77,7 +77,7 @@ wl_buffer_release(void *data, struct wl_buffer *buffer)
>     int i;
>
>     for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); ++i)
> -      if (dri2_surf->color_buffers[i].wl_buffer == buffer)
> +      if (dri2_surf->color_buffers[i].native_buffer == (void *)buffer)
>           break;
>
>     if (i == ARRAY_SIZE(dri2_surf->color_buffers)) {
> @@ -267,8 +267,8 @@ dri2_wl_destroy_surface(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *surf)
>     dri2_dpy->core->destroyDrawable(dri2_surf->dri_drawable);
>
>     for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> -      if (dri2_surf->color_buffers[i].wl_buffer)
> -         wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
> +      if (dri2_surf->color_buffers[i].native_buffer)
> +         wl_buffer_destroy((struct wl_buffer
> *)dri2_surf->color_buffers[i].native_buffer);
>        if (dri2_surf->color_buffers[i].dri_image)
>           dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].dr
> i_image);
>        if (dri2_surf->color_buffers[i].linear_copy)
> @@ -309,9 +309,9 @@ dri2_wl_release_buffers(struct dri2_egl_surface
> *dri2_surf)
>        dri2_egl_display(dri2_surf->base.Resource.Display);
>
>     for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> -      if (dri2_surf->color_buffers[i].wl_buffer &&
> +      if (dri2_surf->color_buffers[i].native_buffer &&
>            !dri2_surf->color_buffers[i].locked)
> -         wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
> +         wl_buffer_destroy((struct wl_buffer
> *)dri2_surf->color_buffers[i].native_buffer);
>        if (dri2_surf->color_buffers[i].dri_image)
>           dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].dr
> i_image);
>        if (dri2_surf->color_buffers[i].linear_copy)
> @@ -320,7 +320,7 @@ dri2_wl_release_buffers(struct dri2_egl_surface
> *dri2_surf)
>           munmap(dri2_surf->color_buffers[i].data,
>                  dri2_surf->color_buffers[i].data_size);
>
> -      dri2_surf->color_buffers[i].wl_buffer = NULL;
> +      dri2_surf->color_buffers[i].native_buffer = NULL;
>        dri2_surf->color_buffers[i].dri_image = NULL;
>        dri2_surf->color_buffers[i].linear_copy = NULL;
>        dri2_surf->color_buffers[i].data = NULL;
> @@ -514,12 +514,12 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
>      * That means we can free any unlocked buffer now. */
>     for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
>        if (!dri2_surf->color_buffers[i].locked &&
> -          dri2_surf->color_buffers[i].wl_buffer) {
> -         wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
> +          dri2_surf->color_buffers[i].native_buffer) {
> +         wl_buffer_destroy((struct wl_buffer
> *)dri2_surf->color_buffers[i].native_buffer);
>           dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].dr
> i_image);
>           if (dri2_dpy->is_different_gpu)
>              dri2_dpy->image->destroyImage(dri2_surf->color_
> buffers[i].linear_copy);
> -         dri2_surf->color_buffers[i].wl_buffer = NULL;
> +         dri2_surf->color_buffers[i].native_buffer = NULL;
>           dri2_surf->color_buffers[i].dri_image = NULL;
>           dri2_surf->color_buffers[i].linear_copy = NULL;
>        }
> @@ -848,7 +848,7 @@ dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
>     dri2_surf->current = dri2_surf->back;
>     dri2_surf->back = NULL;
>
> -   if (!dri2_surf->current->wl_buffer) {
> +   if (!dri2_surf->current->native_buffer) {
>        __DRIimage *image;
>
>        if (dri2_dpy->is_different_gpu)
> @@ -856,15 +856,15 @@ dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
>        else
>           image = dri2_surf->current->dri_image;
>
> -      dri2_surf->current->wl_buffer =
> -         create_wl_buffer(dri2_dpy, dri2_surf, image);
> +      dri2_surf->current->native_buffer =
> +         (void *)create_wl_buffer(dri2_dpy, dri2_surf, image);
>
> -      wl_buffer_add_listener(dri2_surf->current->wl_buffer,
> +      wl_buffer_add_listener(dri2_surf->current->native_buffer,
>                               &wl_buffer_listener, dri2_surf);
>     }
>
>     wl_surface_attach(dri2_surf->wl_surface_wrapper,
> -                     dri2_surf->current->wl_buffer,
> +                     (struct wl_buffer *)dri2_surf->current->native_b
> uffer,
>                       dri2_surf->dx, dri2_surf->dy);
>
>     dri2_surf->wl_win->attached_width  = dri2_surf->base.Width;
> @@ -1521,7 +1521,7 @@ static EGLBoolean
>  dri2_wl_swrast_allocate_buffer(struct dri2_egl_surface *dri2_surf,
>                                 int format, int w, int h,
>                                 void **data, int *size,
> -                               struct wl_buffer **buffer)
> +                               void **buffer)
>  {
>     struct dri2_egl_display *dri2_dpy =
>        dri2_egl_display(dri2_surf->base.Resource.Display);
> @@ -1546,7 +1546,7 @@ dri2_wl_swrast_allocate_buffer(struct
> dri2_egl_surface *dri2_surf,
>     /* Share it in a wl_buffer */
>     pool = wl_shm_create_pool(dri2_dpy->wl_shm, fd, size_map);
>     wl_proxy_set_queue((struct wl_proxy *)pool, dri2_surf->wl_queue);
> -   *buffer = wl_shm_pool_create_buffer(pool, 0, w, h, stride, format);
> +   *buffer = (void *)wl_shm_pool_create_buffer(pool, 0, w, h, stride,
> format);
>     wl_shm_pool_destroy(pool);
>     close(fd);
>
> @@ -1585,7 +1585,7 @@ swrast_update_buffers(struct dri2_egl_surface
> *dri2_surf)
>     /* try get free buffer already created */
>     for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
>        if (!dri2_surf->color_buffers[i].locked &&
> -          dri2_surf->color_buffers[i].wl_buffer) {
> +          dri2_surf->color_buffers[i].native_buffer) {
>            dri2_surf->back = &dri2_surf->color_buffers[i];
>            break;
>        }
> @@ -1602,11 +1602,11 @@ swrast_update_buffers(struct dri2_egl_surface
> *dri2_surf)
>                                                   dri2_surf->base.Height,
>                                                   &dri2_surf->back->data,
>
> &dri2_surf->back->data_size,
> -
>  &dri2_surf->back->wl_buffer)) {
> +
>  &dri2_surf->back->native_buffer)) {
>                  _eglError(EGL_BAD_ALLOC, "failed to allocate color
> buffer");
>                   return -1;
>               }
> -             wl_buffer_add_listener(dri2_surf->back->wl_buffer,
> +             wl_buffer_add_listener((struct wl_buffer
> *)dri2_surf->back->native_buffer,
>                                      &wl_buffer_listener, dri2_surf);
>               break;
>           }
> @@ -1625,11 +1625,11 @@ swrast_update_buffers(struct dri2_egl_surface
> *dri2_surf)
>      * That means we can free any unlocked buffer now. */
>     for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
>        if (!dri2_surf->color_buffers[i].locked &&
> -          dri2_surf->color_buffers[i].wl_buffer) {
> -         wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
> +          dri2_surf->color_buffers[i].native_buffer) {
> +         wl_buffer_destroy((struct wl_buffer
> *)dri2_surf->color_buffers[i].native_buffer);
>           munmap(dri2_surf->color_buffers[i].data,
>                  dri2_surf->color_buffers[i].data_size);
> -         dri2_surf->color_buffers[i].wl_buffer = NULL;
> +         dri2_surf->color_buffers[i].native_buffer = NULL;
>           dri2_surf->color_buffers[i].data = NULL;
>        }
>     }
> @@ -1675,7 +1675,7 @@ dri2_wl_swrast_commit_backbuffer(struct
> dri2_egl_surface *dri2_surf)
>     dri2_surf->back = NULL;
>
>     wl_surface_attach(dri2_surf->wl_surface_wrapper,
> -                     dri2_surf->current->wl_buffer,
> +                     (struct wl_buffer *)dri2_surf->current->native_b
> uffer,
>                       dri2_surf->dx, dri2_surf->dy);
>
>     dri2_surf->wl_win->attached_width  = dri2_surf->base.Width;
> --
> 2.14.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171017/9085a76b/attachment-0001.html>


More information about the mesa-dev mailing list