egl: deduplicate allocations of local buffer over each platform backend (v2)

platform_drm, platform_wayland and platform_android have similiar local buffer
allocation routines. For deduplicating, it unifies dri2_egl_surface's
local buffer allocation routines. And it polishes inconsistent indentations.

Note that as dri2_wl_get_buffers_with_format() have not make a __DRI_BUFFER_BACK_LEFT
attachment buffer for local_buffers, new helper function, dri2_egl_surface_free_local_buffers(),
will drop the __DRI_BUFFER_BACK_LEFT check.
So if other platforms use new helper functions, we have to ensure not to make
__DRI_BUFFER_BACK_LEFT attachment buffer for local_buffers.

v2: Fixes from Emil's review:
   a) Make local_buffers variable, dri2_egl_surface_alloc_local_buffer() and
      dri2_egl_surface_free_local_buffers() unconditionally.
   b) Preserve the original codeflow for error_path and normal_path.
   c) Add note on commit messages for dropping of __DRI_BUFFER_BACK_LEFT check.
   c) Rollback the unrelated whitespace changes.
   d) Add a missing blank line.

Signed-off-by: Mun Gwan-gyeong <elongbug@gmail.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>

5 files changed, 71 insertions, 115 deletions

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 125943333be..aa6f02a34b5 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1011,6 +1011,40 @@ dri2_display_destroy(_EGLDisplay *disp)
    disp->DriverData = NULL;
 }
 
+__DRIbuffer *
+dri2_egl_surface_alloc_local_buffer(struct dri2_egl_surface *dri2_surf,
+                                    unsigned int att, unsigned int format)
+{
+   struct dri2_egl_display *dri2_dpy =
+      dri2_egl_display(dri2_surf->base.Resource.Display);
+
+   if (att >= ARRAY_SIZE(dri2_surf->local_buffers))
+      return NULL;
+
+   if (!dri2_surf->local_buffers[att]) {
+      dri2_surf->local_buffers[att] =
+         dri2_dpy->dri2->allocateBuffer(dri2_dpy->dri_screen, att, format,
+                                        dri2_surf->base.Width, dri2_surf->base.Height);
+   }
+
+   return dri2_surf->local_buffers[att];
+}
+
+void
+dri2_egl_surface_free_local_buffers(struct dri2_egl_surface *dri2_surf)
+{
+   struct dri2_egl_display *dri2_dpy =
+      dri2_egl_display(dri2_surf->base.Resource.Display);
+
+   for (int i = 0; i < ARRAY_SIZE(dri2_surf->local_buffers); i++) {
+      if (dri2_surf->local_buffers[i]) {
+         dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
+                                       dri2_surf->local_buffers[i]);
+         dri2_surf->local_buffers[i] = NULL;
+      }
+   }
+}
+
 /**
  * Called via eglTerminate(), drv->API.Terminate().
  *
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index f471561ed0c..4a52b490a87 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -285,8 +285,10 @@ struct dri2_egl_surface
    struct gbm_dri_surface *gbm_surf;
 #endif
 
+   /* EGL-owned buffers */
+   __DRIbuffer           *local_buffers[__DRI_BUFFER_COUNT];
+
 #if defined(HAVE_WAYLAND_PLATFORM) || defined(HAVE_DRM_PLATFORM)
-   __DRIbuffer           *dri_buffers[__DRI_BUFFER_COUNT];
    struct {
 #ifdef HAVE_WAYLAND_PLATFORM
       struct wl_buffer   *wl_buffer;
@@ -311,9 +313,6 @@ struct dri2_egl_surface
    __DRIimage *dri_image_back;
    __DRIimage *dri_image_front;
 
-   /* EGL-owned buffers */
-   __DRIbuffer           *local_buffers[__DRI_BUFFER_COUNT];
-
    /* 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.
@@ -456,4 +455,11 @@ dri2_set_WL_bind_wayland_display(_EGLDriver *drv, _EGLDisplay *disp)
 void
 dri2_display_destroy(_EGLDisplay *disp);
 
+__DRIbuffer *
+dri2_egl_surface_alloc_local_buffer(struct dri2_egl_surface *dri2_surf,
+                                    unsigned int att, unsigned int format);
+
+void
+dri2_egl_surface_free_local_buffers(struct dri2_egl_surface *dri2_surf);
+
 #endif /* EGL_DRI2_INCLUDED */
diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
index beb474025f7..a864a2af3d7 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -271,40 +271,6 @@ droid_window_cancel_buffer(struct dri2_egl_surface *dri2_surf)
    }
 }
 
-static __DRIbuffer *
-droid_alloc_local_buffer(struct dri2_egl_surface *dri2_surf,
-                         unsigned int att, unsigned int format)
-{
-   struct dri2_egl_display *dri2_dpy =
-      dri2_egl_display(dri2_surf->base.Resource.Display);
-
-   if (att >= ARRAY_SIZE(dri2_surf->local_buffers))
-      return NULL;
-
-   if (!dri2_surf->local_buffers[att]) {
-      dri2_surf->local_buffers[att] =
-         dri2_dpy->dri2->allocateBuffer(dri2_dpy->dri_screen, att, format,
-               dri2_surf->base.Width, dri2_surf->base.Height);
-   }
-
-   return dri2_surf->local_buffers[att];
-}
-
-static void
-droid_free_local_buffers(struct dri2_egl_surface *dri2_surf)
-{
-   struct dri2_egl_display *dri2_dpy =
-      dri2_egl_display(dri2_surf->base.Resource.Display);
-
-   for (int i = 0; i < ARRAY_SIZE(dri2_surf->local_buffers); i++) {
-      if (dri2_surf->local_buffers[i]) {
-         dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
-               dri2_surf->local_buffers[i]);
-         dri2_surf->local_buffers[i] = NULL;
-      }
-   }
-}
-
 static _EGLSurface *
 droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
 		    _EGLConfig *conf, void *native_window,
@@ -400,7 +366,7 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
    struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
    struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
 
-   droid_free_local_buffers(dri2_surf);
+   dri2_egl_surface_free_local_buffers(dri2_surf);
 
    if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
       if (dri2_surf->buffer)
@@ -447,7 +413,7 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
    /* free outdated buffers and update the surface size */
    if (dri2_surf->base.Width != dri2_surf->buffer->width ||
        dri2_surf->base.Height != dri2_surf->buffer->height) {
-      droid_free_local_buffers(dri2_surf);
+      dri2_egl_surface_free_local_buffers(dri2_surf);
       dri2_surf->base.Width = dri2_surf->buffer->width;
       dri2_surf->base.Height = dri2_surf->buffer->height;
    }
@@ -970,7 +936,7 @@ droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf,
       case __DRI_BUFFER_ACCUM:
       case __DRI_BUFFER_DEPTH_STENCIL:
       case __DRI_BUFFER_HIZ:
-         local = droid_alloc_local_buffer(dri2_surf,
+         local = dri2_egl_surface_alloc_local_buffer(dri2_surf,
                attachments[i], attachments[i + 1]);
 
          if (local) {
diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/platform_drm.c
index ee75a9b0d9f..259b1cd5194 100644
--- a/src/egl/drivers/dri2/platform_drm.c
+++ b/src/egl/drivers/dri2/platform_drm.c
@@ -176,11 +176,7 @@ dri2_drm_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
 	 gbm_bo_destroy(dri2_surf->color_buffers[i].bo);
    }
 
-   for (unsigned i = 0; i < __DRI_BUFFER_COUNT; i++) {
-      if (dri2_surf->dri_buffers[i])
-         dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
-                                       dri2_surf->dri_buffers[i]);
-   }
+   dri2_egl_surface_free_local_buffers(dri2_surf);
 
    free(surf);
 
@@ -271,39 +267,18 @@ back_bo_to_dri_buffer(struct dri2_egl_surface *dri2_surf, __DRIbuffer *buffer)
    buffer->flags = 0;
 }
 
-static int
-get_aux_bo(struct dri2_egl_surface *dri2_surf,
-	   unsigned int attachment, unsigned int format, __DRIbuffer *buffer)
-{
-   struct dri2_egl_display *dri2_dpy =
-      dri2_egl_display(dri2_surf->base.Resource.Display);
-   __DRIbuffer *b = dri2_surf->dri_buffers[attachment];
-
-   if (b == NULL) {
-      b = dri2_dpy->dri2->allocateBuffer(dri2_dpy->dri_screen,
-					 attachment, format,
-					 dri2_surf->base.Width,
-					 dri2_surf->base.Height);
-      dri2_surf->dri_buffers[attachment] = b;
-   }
-   if (b == NULL)
-      return -1;
-
-   memcpy(buffer, b, sizeof *buffer);
-
-   return 0;
-}
-
 static __DRIbuffer *
 dri2_drm_get_buffers_with_format(__DRIdrawable *driDrawable,
-			     int *width, int *height,
-			     unsigned int *attachments, int count,
-			     int *out_count, void *loaderPrivate)
+                                 int *width, int *height,
+                                 unsigned int *attachments, int count,
+                                 int *out_count, void *loaderPrivate)
 {
    struct dri2_egl_surface *dri2_surf = loaderPrivate;
    int i, j;
 
    for (i = 0, j = 0; i < 2 * count; i += 2, j++) {
+      __DRIbuffer *local;
+
       assert(attachments[i] < __DRI_BUFFER_COUNT);
       assert(j < ARRAY_SIZE(dri2_surf->buffers));
 
@@ -316,11 +291,14 @@ dri2_drm_get_buffers_with_format(__DRIdrawable *driDrawable,
          back_bo_to_dri_buffer(dri2_surf, &dri2_surf->buffers[j]);
 	 break;
       default:
-	 if (get_aux_bo(dri2_surf, attachments[i], attachments[i + 1],
-			&dri2_surf->buffers[j]) < 0) {
-	    _eglError(EGL_BAD_ALLOC, "failed to allocate aux buffer");
+	 local = dri2_egl_surface_alloc_local_buffer(dri2_surf, attachments[i],
+                                                     attachments[i + 1]);
+
+	 if (!local) {
+	    _eglError(EGL_BAD_ALLOC, "failed to allocate local buffer");
 	    return NULL;
 	 }
+	 dri2_surf->buffers[j] = *local;
 	 break;
       }
    }
diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
index 3535c4bef08..15dc9390cb3 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -277,13 +277,8 @@ dri2_wl_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
                 dri2_surf->color_buffers[i].data_size);
    }
 
-   if (dri2_dpy->dri2) {
-      for (int i = 0; i < __DRI_BUFFER_COUNT; i++)
-         if (dri2_surf->dri_buffers[i] &&
-             dri2_surf->dri_buffers[i]->attachment != __DRI_BUFFER_BACK_LEFT)
-            dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
-                                          dri2_surf->dri_buffers[i]);
-   }
+   if (dri2_dpy->dri2)
+      dri2_egl_surface_free_local_buffers(dri2_surf);
 
    if (dri2_surf->throttle_callback)
       wl_callback_destroy(dri2_surf->throttle_callback);
@@ -330,13 +325,8 @@ dri2_wl_release_buffers(struct dri2_egl_surface *dri2_surf)
       dri2_surf->color_buffers[i].locked = false;
    }
 
-   if (dri2_dpy->dri2) {
-      for (int i = 0; i < __DRI_BUFFER_COUNT; i++)
-         if (dri2_surf->dri_buffers[i] &&
-             dri2_surf->dri_buffers[i]->attachment != __DRI_BUFFER_BACK_LEFT)
-            dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
-                                          dri2_surf->dri_buffers[i]);
-   }
+   if (dri2_dpy->dri2)
+      dri2_egl_surface_free_local_buffers(dri2_surf);
 }
 
 static int
@@ -491,29 +481,6 @@ back_bo_to_dri_buffer(struct dri2_egl_surface *dri2_surf, __DRIbuffer *buffer)
 }
 
 static int
-get_aux_bo(struct dri2_egl_surface *dri2_surf,
-           unsigned int attachment, unsigned int format, __DRIbuffer *buffer)
-{
-   struct dri2_egl_display *dri2_dpy =
-      dri2_egl_display(dri2_surf->base.Resource.Display);
-   __DRIbuffer *b = dri2_surf->dri_buffers[attachment];
-
-   if (b == NULL) {
-      b = dri2_dpy->dri2->allocateBuffer(dri2_dpy->dri_screen,
-                                         attachment, format,
-                                         dri2_surf->base.Width,
-                                         dri2_surf->base.Height);
-      dri2_surf->dri_buffers[attachment] = b;
-   }
-   if (b == NULL)
-      return -1;
-
-   memcpy(buffer, b, sizeof *buffer);
-
-   return 0;
-}
-
-static int
 update_buffers(struct dri2_egl_surface *dri2_surf)
 {
    struct dri2_egl_display *dri2_dpy =
@@ -567,16 +534,21 @@ dri2_wl_get_buffers_with_format(__DRIdrawable * driDrawable,
       return NULL;
 
    for (i = 0, j = 0; i < 2 * count; i += 2, j++) {
+      __DRIbuffer *local;
+
       switch (attachments[i]) {
       case __DRI_BUFFER_BACK_LEFT:
          back_bo_to_dri_buffer(dri2_surf, &dri2_surf->buffers[j]);
          break;
       default:
-         if (get_aux_bo(dri2_surf, attachments[i], attachments[i + 1],
-                        &dri2_surf->buffers[j]) < 0) {
-            _eglError(EGL_BAD_ALLOC, "failed to allocate aux buffer");
+         local = dri2_egl_surface_alloc_local_buffer(dri2_surf, attachments[i],
+                                                     attachments[i + 1]);
+
+         if (!local) {
+            _eglError(EGL_BAD_ALLOC, "failed to allocate local buffer");
             return NULL;
          }
+         dri2_surf->buffers[j] = *local;
          break;
       }
    }