Re: [Mesa-dev] [PATCH v2 3/8] egl: add dri2_egl_surface_record_buffers_and_update_back_buffer() helper (v2)

[Emil Velikov]

Hi Gwan-gyeong,

There's a small nit but otherwise looks good.

On 6 October 2017 at 22:38, Gwan-gyeong Mun  wrote:
> To share common record buffers and update back buffer code.
> This records all the buffers created by each platform's native window and
> update back buffer for updating buffer's age in swap_buffers.
>
> In preparation to adding of new platform which uses this helper.
>
> v2:
>  - Remove unnedded ifdef magic
s/unnedded/unneeded/

>  - Fixes from Eric's review:
>a) Split out series of refactor for helpers to a separate series.
>b) Add the new helper function and use them to replace the old code in the
>   same patch.
>
> Signed-off-by: Mun Gwan-gyeong 
> ---
>  src/egl/drivers/dri2/egl_dri2.c | 32 
>  src/egl/drivers/dri2/egl_dri2.h |  5 +
>  src/egl/drivers/dri2/platform_android.c | 25 ++---
>  3 files changed, 39 insertions(+), 23 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 3c4e525040..3622d18a24 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -1068,6 +1068,38 @@ 
> dri2_egl_surface_free_outdated_buffers_and_update_size(struct 
> dri2_egl_surface *
> }
>  }
>
> +void
> +dri2_egl_surface_record_buffers_and_update_back_buffer(struct 
> dri2_egl_surface *dri2_surf,
> +   void *buffer)
> +{
> +   /* Record all the buffers created by each platform's native window and
> +* update back buffer for updating buffer's age in swap_buffers.
> +*/
> +   EGLBoolean updated = EGL_FALSE;
> +
> +   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].native_buffer = buffer;
> + dri2_surf->color_buffers[i].age = 0;
"age" seems like a new addition. It's correct to have it, but I'll
keep that as separate patch.

Also I would set "updated" and bail out at this point. There's no
point in continuing to loop.
If you want to address that (it's optional) please keep as separate patch.

With the age line dropped and typo fixed
Reviewed-by: Emil Velikov 

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 3/8] egl: add dri2_egl_surface_record_buffers_and_update_back_buffer() helper (v2)

[Gwan-gyeong Mun]

To share common record buffers and update back buffer code.
This records all the buffers created by each platform's native window and
update back buffer for updating buffer's age in swap_buffers.

In preparation to adding of new platform which uses this helper.

v2:
 - Remove unnedded ifdef magic
 - Fixes from Eric's review:
   a) Split out series of refactor for helpers to a separate series.
   b) Add the new helper function and use them to replace the old code in the
  same patch.

Signed-off-by: Mun Gwan-gyeong 
---
 src/egl/drivers/dri2/egl_dri2.c | 32 
 src/egl/drivers/dri2/egl_dri2.h |  5 +
 src/egl/drivers/dri2/platform_android.c | 25 ++---
 3 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 3c4e525040..3622d18a24 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1068,6 +1068,38 @@ 
dri2_egl_surface_free_outdated_buffers_and_update_size(struct dri2_egl_surface *
}
 }
 
+void
+dri2_egl_surface_record_buffers_and_update_back_buffer(struct dri2_egl_surface 
*dri2_surf,
+   void *buffer)
+{
+   /* Record all the buffers created by each platform's native window and
+* update back buffer for updating buffer's age in swap_buffers.
+*/
+   EGLBoolean updated = EGL_FALSE;
+
+   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].native_buffer = buffer;
+ dri2_surf->color_buffers[i].age = 0;
+  }
+  if (dri2_surf->color_buffers[i].native_buffer == buffer) {
+ dri2_surf->back = _surf->color_buffers[i];
+ updated = EGL_TRUE;
+ break;
+  }
+   }
+
+   if (!updated) {
+  /* In case of all the buffers were recreated, reset the color_buffers */
+  for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
+ dri2_surf->color_buffers[i].native_buffer = NULL;
+ dri2_surf->color_buffers[i].age = 0;
+  }
+  dri2_surf->color_buffers[0].native_buffer = buffer;
+  dri2_surf->back = _surf->color_buffers[0];
+   }
+}
+
 /**
  * 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 08ccf24410..67d7b7f863 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -463,6 +463,11 @@ dri2_egl_surface_free_local_buffers(struct 
dri2_egl_surface *dri2_surf);
 void
 dri2_egl_surface_free_outdated_buffers_and_update_size(struct dri2_egl_surface 
*dri2_surf,
int width, int height);
+
+void
+dri2_egl_surface_record_buffers_and_update_back_buffer(struct dri2_egl_surface 
*dri2_surf,
+   void *buffer);
+
 EGLBoolean
 dri2_init_surface(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
 _EGLConfig *conf, const EGLint *attrib_list, EGLBoolean 
enable_out_fence);
diff --git a/src/egl/drivers/dri2/platform_android.c 
b/src/egl/drivers/dri2/platform_android.c
index 67e739c1fc..202db9a883 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -191,29 +191,8 @@ droid_window_dequeue_buffer(struct dri2_egl_surface 
*dri2_surf)
/* Record all the buffers created by ANativeWindow and update back buffer
 * for updating buffer's age in swap_buffers.
 */
-   EGLBoolean updated = EGL_FALSE;
-   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].native_buffer = (void *)dri2_surf->buffer;
-  }
-  if (dri2_surf->color_buffers[i].native_buffer == (void 
*)dri2_surf->buffer) {
- dri2_surf->back = _surf->color_buffers[i];
- updated = EGL_TRUE;
- break;
-  }
-   }
-
-   if (!updated) {
-  /* In case of all the buffers were recreated by ANativeWindow, reset
-   * the color_buffers
-   */
-  for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
- dri2_surf->color_buffers[i].native_buffer = NULL;
- dri2_surf->color_buffers[i].age = 0;
-  }
-  dri2_surf->color_buffers[0].native_buffer = (void *)dri2_surf->buffer;
-  dri2_surf->back = _surf->color_buffers[0];
-   }
+   dri2_egl_surface_record_buffers_and_update_back_buffer(dri2_surf,
+  (void 
*)dri2_surf->buffer);
 
return EGL_TRUE;
 }
-- 
2.14.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev