[Mesa-dev] [PATCH v3 10/10] egl/wayland: add dri2_wl_free_buffers() helper (v2)

[Gwan-gyeong Mun]

This deduplicates free routines of color_buffers array.

v2:
 - Add clear_all argument to check clearing all of color_buffers or not.
 - Fixes from Eric's review:
   a) polish check routine of check_lock and color_buffers[i].locked
   b) move 'native_buffer = NULL' to avoid leaking locked buffers
 - Fixes from Emil's review:
   a) drop the unneeded cast
   b) apply dri2_wl_free_buffers to update_buffers() and swrast_update_buffers()

Signed-off-by: Mun Gwan-gyeong <elongbug at gmail.com>
---
 src/egl/drivers/dri2/platform_wayland.c | 92 ++++++++++++++-------------------
 1 file changed, 38 insertions(+), 54 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
index 21037b4547..ad72641478 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -252,6 +252,40 @@ dri2_wl_create_pixmap_surface(_EGLDriver *drv, _EGLDisplay *disp,
    return NULL;
 }
 
+static void
+dri2_wl_free_buffers(struct dri2_egl_surface *dri2_surf, bool check_lock,
+                     bool clear_all)
+{
+   struct dri2_egl_display *dri2_dpy =
+      dri2_egl_display(dri2_surf->base.Resource.Display);
+
+   for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
+      bool clear_buffer = false;
+
+      if (dri2_surf->color_buffers[i].native_buffer &&
+          (!check_lock || !dri2_surf->color_buffers[i].locked)) {
+         wl_buffer_destroy(dri2_surf->color_buffers[i].native_buffer);
+         dri2_surf->color_buffers[i].native_buffer = NULL;
+         dri2_surf->color_buffers[i].locked = false;
+         clear_buffer = true;
+      }
+
+      if (clear_all || clear_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)
+            dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].linear_copy);
+         if (dri2_surf->color_buffers[i].data)
+            munmap(dri2_surf->color_buffers[i].data,
+                   dri2_surf->color_buffers[i].data_size);
+
+         dri2_surf->color_buffers[i].dri_image = NULL;
+         dri2_surf->color_buffers[i].linear_copy = NULL;
+         dri2_surf->color_buffers[i].data = NULL;
+      }
+   }
+}
+
 /**
  * Called via eglDestroySurface(), drv->API.DestroySurface().
  */
@@ -265,17 +299,7 @@ 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].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)
-         dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].linear_copy);
-      if (dri2_surf->color_buffers[i].data)
-         munmap(dri2_surf->color_buffers[i].data,
-                dri2_surf->color_buffers[i].data_size);
-   }
+   dri2_wl_free_buffers(dri2_surf, false, true);
 
    if (dri2_dpy->dri2)
       dri2_egl_surface_free_local_buffers(dri2_surf);
@@ -307,24 +331,7 @@ dri2_wl_release_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->color_buffers); i++) {
-      if (dri2_surf->color_buffers[i].native_buffer &&
-          !dri2_surf->color_buffers[i].locked)
-         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)
-         dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].linear_copy);
-      if (dri2_surf->color_buffers[i].data)
-         munmap(dri2_surf->color_buffers[i].data,
-                dri2_surf->color_buffers[i].data_size);
-
-      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;
-      dri2_surf->color_buffers[i].locked = false;
-   }
+   dri2_wl_free_buffers(dri2_surf, true, true);
 
    if (dri2_dpy->dri2)
       dri2_egl_surface_free_local_buffers(dri2_surf);
@@ -489,9 +496,6 @@ back_bo_to_dri_buffer(struct dri2_egl_surface *dri2_surf, __DRIbuffer *buffer)
 static int
 update_buffers(struct dri2_egl_surface *dri2_surf)
 {
-   struct dri2_egl_display *dri2_dpy =
-      dri2_egl_display(dri2_surf->base.Resource.Display);
-
    if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
        dri2_surf->base.Height != dri2_surf->wl_win->height) {
 
@@ -511,18 +515,7 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
    /* If we have an extra unlocked buffer at this point, we had to do triple
     * buffering for a while, but now can go back to just double buffering.
     * 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].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].native_buffer = NULL;
-         dri2_surf->color_buffers[i].dri_image = NULL;
-         dri2_surf->color_buffers[i].linear_copy = NULL;
-      }
-   }
+   dri2_wl_free_buffers(dri2_surf, true, false);
 
    return 0;
 }
@@ -1623,16 +1616,7 @@ swrast_update_buffers(struct dri2_egl_surface *dri2_surf)
    /* If we have an extra unlocked buffer at this point, we had to do triple
     * buffering for a while, but now can go back to just double buffering.
     * 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].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].native_buffer = NULL;
-         dri2_surf->color_buffers[i].data = NULL;
-      }
-   }
+   dri2_wl_free_buffers(dri2_surf, true, false);
 
    return 0;
 }
-- 
2.14.2