On 6 October 2017 at 22:38, Gwan-gyeong Mun <elong...@gmail.com> wrote:
> This is added for preventing adding of new color buffers structure and back*
> when new platform backend is added.
> This refactoring separates out the common and platform specific bits.
> This makes odd casting in the platform_foo.c but it prevents adding of new
> ifdef magic.
> Because of color_buffers array size is different on android and wayland /drm,
> it adds COLOR_BUFFERS_SIZE macro.
> - android's color buffers array size is 3.
> drm & wayland's color buffers array size is 4.
>
> Fixes from Rob's review:
> - refactor to separate out the common and platform specific bits.
>
> Fixes from Emil's review:
> - use suggested color buffers structure shape.
> it makes a buffer pointer of each platform to void pointer type.
>
> Signed-off-by: Mun Gwan-gyeong <elong...@gmail.com>
> ---
> src/egl/drivers/dri2/egl_dri2.h | 30 +++++++++---------
> src/egl/drivers/dri2/platform_android.c | 10 +++---
> src/egl/drivers/dri2/platform_drm.c | 55
> +++++++++++++++++----------------
> src/egl/drivers/dri2/platform_wayland.c | 46 +++++++++++++--------------
> 4 files changed, 71 insertions(+), 70 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> index 017895f0d9..08ccf24410 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -65,6 +65,15 @@ struct zwp_linux_dmabuf_v1;
>
> #endif /* HAVE_ANDROID_PLATFORM */
>
> +#if defined(HAVE_WAYLAND_PLATFORM) || defined(HAVE_DRM_PLATFORM)
The guard should be if Android ->3, 4 otherwise.
> +#define COLOR_BUFFERS_SIZE 4
> +#else
> + /* Usually Android uses at most triple buffers in ANativeWindow
> + * so hardcode the number of color_buffers to 3.
> + */
Props for keeping the comment around. Please drop the indentation.
> +#define COLOR_BUFFERS_SIZE 3
> +#endif
> +
> #include "eglconfig.h"
> #include "eglcontext.h"
> #include "egldisplay.h"
> @@ -286,39 +295,28 @@ struct dri2_egl_surface
> /* EGL-owned buffers */
> __DRIbuffer *local_buffers[__DRI_BUFFER_COUNT];
>
> -#if defined(HAVE_WAYLAND_PLATFORM) || defined(HAVE_DRM_PLATFORM)
> + /* Used to record all the buffers created by each platform's native window
> + * and their ages.
> + */
> struct {
> + void *native_buffer; // aka wl_buffer/gbm_bo/ANativeWindowBuffer
> #ifdef HAVE_WAYLAND_PLATFORM
I would drop this guard. Sure it will make the struct tiny bit larger,
but it will allow us to have a more generic and widespread helpers.
The rest of the patch should use a handful of:
- drop unneeded $native_type -> void * cast
- create the local native_buffer of $native_type and cast on assignment
Some partial examples follow:
> --- a/src/egl/drivers/dri2/platform_drm.c
> +++ b/src/egl/drivers/dri2/platform_drm.c
> @@ -53,7 +53,7 @@ lock_front_buffer(struct gbm_surface *_surf)
> return NULL;
> }
>
> - bo = dri2_surf->current->bo;
> + bo = (struct gbm_bo *)dri2_surf->current->native_buffer;
>
Unneeded cast?
> for (unsigned i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> - if (dri2_surf->color_buffers[i].bo)
> - gbm_bo_destroy(dri2_surf->color_buffers[i].bo);
> + if (dri2_surf->color_buffers[i].native_buffer)
> + gbm_bo_destroy((struct gbm_bo
> *)dri2_surf->color_buffers[i].native_buffer);
Ditto.
> }
>
> dri2_egl_surface_free_local_buffers(dri2_surf);
> @@ -204,23 +204,24 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
>
> if (dri2_surf->back == NULL)
> return -1;
> - if (dri2_surf->back->bo == NULL) {
> + if (dri2_surf->back->native_buffer == NULL) {
> if (surf->base.modifiers)
> - dri2_surf->back->bo =
> gbm_bo_create_with_modifiers(&dri2_dpy->gbm_dri->base,
> - surf->base.width,
> -
> surf->base.height,
> -
> surf->base.format,
> -
> surf->base.modifiers,
> -
> surf->base.count);
> + dri2_surf->back->native_buffer =
> + (void *)gbm_bo_create_with_modifiers(&dri2_dpy->gbm_dri->base,
> + surf->base.width,
> + surf->base.height,
> + surf->base.format,
> + surf->base.modifiers,
> + surf->base.count);
> else
> - dri2_surf->back->bo = gbm_bo_create(&dri2_dpy->gbm_dri->base,
> - surf->base.width,
> - surf->base.height,
> - surf->base.format,
> - surf->base.flags);
> + dri2_surf->back->native_buffer = (void
> *)gbm_bo_create(&dri2_dpy->gbm_dri->base,
> +
> surf->base.width,
> +
> surf->base.height,
> +
> surf->base.format,
> +
> surf->base.flags);
>
struct gbm_bo *bo;
if (foo)
bo = gbm_bo...()
else
bo = gbm_bo...()
if (bo == NULL)
return -1;
dri2_surf->back->native_buffer = (void *)bo;
> @@ -1521,7 +1521,7 @@ static EGLBoolean
> dri2_wl_swrast_allocate_buffer(struct dri2_egl_surface *dri2_surf,
> int format, int w, int h,
> void **data, int *size,
> - struct wl_buffer **buffer)
> + void **buffer)
No need to change the type?
-Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
