[Mesa-dev] [PATCH] egl: refactor color_buffers structure for deduplicating (v2)

Gwan-gyeong Mun elongbug at gmail.com
Wed Nov 15 15:27:09 UTC 2017


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.

v2: Fixes from Emil's review
  a) change ifdef macro of "HAVE_WAYLAND_PLATFORM or HAVE_DRM_PLATFORM" to
     "HAVE_ANDROID_PLATFORM" for COLOR_BUFFERS_SIZE.
  b) drop the unneeded indentation of comment.
  c) drop ifdef macro of HAVE_WAYLAND_PLATFORM from color_buffers structure
     for more generic and widespread helpers.
  d) drop unneeded $native_type -> void * cast and viceversa.
  e) create the local native_buffer of $native_type and cast on assignment.

Signed-off-by: Mun Gwan-gyeong <elongbug at gmail.com>
---
 src/egl/drivers/dri2/egl_dri2.h         | 32 ++++++++----------
 src/egl/drivers/dri2/platform_android.c | 10 +++---
 src/egl/drivers/dri2/platform_drm.c     | 60 ++++++++++++++++++---------------
 src/egl/drivers/dri2/platform_wayland.c | 41 +++++++++++-----------
 4 files changed, 73 insertions(+), 70 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index 0ec8f44dce..cbeedadd4b 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 */
 
+#ifdef HAVE_ANDROID_PLATFORM
+/* Usually Android uses at most triple buffers in ANativeWindow so hardcode
+ * the number of color_buffers to 3.
+ */
+#define COLOR_BUFFERS_SIZE 3
+#else
+#define COLOR_BUFFERS_SIZE 4
+#endif
+
 #include "eglconfig.h"
 #include "eglcontext.h"
 #include "egldisplay.h"
@@ -280,39 +289,26 @@ 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 {
-#ifdef HAVE_WAYLAND_PLATFORM
-      struct wl_buffer   *wl_buffer;
+      void *native_buffer; // aka wl_buffer/gbm_bo/ANativeWindowBuffer
       __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 63223e9a69..24e5ddebf9 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 = dri2_surf->buffer;
       }
-      if (dri2_surf->color_buffers[i].buffer == dri2_surf->buffer) {
+      if (dri2_surf->color_buffers[i].native_buffer == 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 = 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..57d1ea4417 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 = 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 == 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(dri2_surf->color_buffers[i].native_buffer);
    }
 
    dri2_egl_surface_free_local_buffers(dri2_surf);
@@ -204,24 +204,28 @@ 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) {
+      struct gbm_bo *bo;
+
       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);
+         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);
       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);
+         bo = gbm_bo_create(&dri2_dpy->gbm_dri->base,
+                            surf->base.width,
+                            surf->base.height,
+                            surf->base.format,
+                            surf->base.flags);
+
+      if (bo == NULL)
+         return -1;
 
+      dri2_surf->back->native_buffer = bo;
    }
-   if (dri2_surf->back->bo == NULL)
-      return -1;
 
    return 0;
 }
@@ -238,11 +242,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 = 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 +262,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 = 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 +366,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 = dri2_surf->back->native_buffer;
    buffers->image_mask = __DRI_IMAGE_BUFFER_BACK;
    buffers->back = bo->image;
 
@@ -496,7 +502,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 +547,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 7010dfdcb2..3a52971f54 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -76,7 +76,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 == buffer)
          break;
 
    if (i == ARRAY_SIZE(dri2_surf->color_buffers)) {
@@ -266,8 +266,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(dri2_surf->color_buffers[i].native_buffer);
       if (dri2_surf->color_buffers[i].dri_image)
          dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].dri_image);
       if (dri2_surf->color_buffers[i].linear_copy)
@@ -308,9 +308,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(dri2_surf->color_buffers[i].native_buffer);
       if (dri2_surf->color_buffers[i].dri_image)
          dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].dri_image);
       if (dri2_surf->color_buffers[i].linear_copy)
@@ -319,7 +319,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;
@@ -513,12 +513,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(dri2_surf->color_buffers[i].native_buffer);
          dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].dri_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;
       }
@@ -875,7 +875,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)
@@ -883,15 +883,15 @@ dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
       else
          image = dri2_surf->current->dri_image;
 
-      dri2_surf->current->wl_buffer =
+      dri2_surf->current->native_buffer =
          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,
+                     dri2_surf->current->native_buffer,
                      dri2_surf->dx, dri2_surf->dy);
 
    dri2_surf->wl_win->attached_width  = dri2_surf->base.Width;
@@ -1612,7 +1612,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;
       }
@@ -1623,17 +1623,18 @@ swrast_update_buffers(struct dri2_egl_surface *dri2_surf)
       for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
          if (!dri2_surf->color_buffers[i].locked) {
              dri2_surf->back = &dri2_surf->color_buffers[i];
+             struct wl_buffer *bo = dri2_surf->back->native_buffer;
              if (!dri2_wl_swrast_allocate_buffer(dri2_surf,
                                                  dri2_surf->format,
                                                  dri2_surf->base.Width,
                                                  dri2_surf->base.Height,
                                                  &dri2_surf->back->data,
                                                  &dri2_surf->back->data_size,
-                                                 &dri2_surf->back->wl_buffer)) {
+                                                 &bo)) {
                 _eglError(EGL_BAD_ALLOC, "failed to allocate color buffer");
                  return -1;
              }
-             wl_buffer_add_listener(dri2_surf->back->wl_buffer,
+             wl_buffer_add_listener(dri2_surf->back->native_buffer,
                                     &wl_buffer_listener, dri2_surf);
              break;
          }
@@ -1652,11 +1653,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(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;
       }
    }
@@ -1702,7 +1703,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,
+                     dri2_surf->current->native_buffer,
                      dri2_surf->dx, dri2_surf->dy);
 
    dri2_surf->wl_win->attached_width  = dri2_surf->base.Width;
-- 
2.15.0



More information about the mesa-dev mailing list