[Mesa-dev] [PATCH v2 1/8] egl: add dri2_egl_surface_free_outdated_buffers_and_update_size() helper (v2)

[Emil Velikov]

On 17 October 2017 at 21:38, Gurchetan Singh
<gurchetansingh at chromium.org> wrote:
> The naming is verbose and somewhat inconsistent.  We have:
>
> dri2_init_surface
> dri2_fini_surface
> dri2_egl_surface_alloc_local_buffer
> dri2_egl_surface_free_local_buffers
>
> I suggest you implement the following convention:
>
> dri2_surface_init
> dri2_surface_fini
> dri2_surface_alloc_attachment (instead of 'local_buffers')
> dri2_surface_free_attachments  (instead of 'local_buffers')
>
Suggestions seems great, although I'm a bit unsure on the naming
convention - dri2_$object_$action vs dri2_$action_$object.
Most of src/egl/drivers/dri2/ alongside all of src/egl/main/ use the latter.

Then again, I'd suggest keeping that as separate series. These patches
started as a way to minimise the duplication we have in drivers/dri2.
So that new platforms such as Tizen do not need to copy the lot, again.

> and instead of dri2_egl_surface_free_outdated_buffers_and_update_size, we
> can just have:
>
> dri2_surface_update
>
Modulo naming convention (aka dri2_update_surface) I like the name.

> And can you wrap these functions around the:
>
> #if defined(HAVE_WAYLAND_PLATFORM) || defined(HAVE_DRM_PLATFORM) ||
> defined(HAVE_ANDROID_PLATFORM)
>
> pre-processors checks just to make clear what platforms use the attachment
> (aka 'local_buffers') functionality.
>
While technically correct, I'd opt against this. Sort of a secondary
goal the series is to a) remove a handful of the ifdef spaghetti and
b) unify the diverging platforms.
Of which surfaceless and android being the [rather] odd ones out.

We could continue to minimise the diversion as time goes by, and this
steers us in the right direction.

Thanks
Emil