[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 <elongbug at gmail.com> 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 <elongbug at gmail.com>
> ---
>  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.velikov at collabora.com>

-Emil