[PATCH weston 2/2] gl-renderer,simple-dmabuf-v4l: fix dmabuf y-invert

[Pekka Paalanen]

On Mon,  4 Jul 2016 16:25:16 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> Invert the Y_INVERT flag for the EGL import fo dmabufs. This fixes
> weston-simple-dmabuf-intel to show the same image on both GL-composited
> and with direct scanout on a hardware plane. Before, the image would
> y-flip when switching between these two cases. Now the orientation also
> matches the color values written in simple-dmabuf-intel.c.
> 
> The GL-renderer uses the OpenGL convention of texture coordinates, where
> the origin is at the bottom-left of an image. This can be observed in
> texture_region() where the texcoords are inverted if y_invert is false,
> since the surface coordinates have origin at top-left.  Both wl_shm and
> dmabuf buffers have origin at the top-left.
> 
> When wl_shm buffer is imported with glTexImage2D, it gets inverted
> because glTexImage2D is defined to read in the bottom row first. The shm
> data is top row first. This incidentally also means, that buffer pixel
> 0,0 ends up at texture coordinates 0,0. This is now inverted compared to
> the GL coordinate convention, and therefore gl_renderer_attach_shm()
> sets y_inverted to true. This causes texture_region() to NOT invert the
> texcoords. Wayland surface coordinates have origin at top-left, hence
> the double-inversion.
> 
> Dmabuf buffers also have the origin at top-left. However, they are
> imported via EGL to GL, where they should get the GL oriented
> coordinates but they do not. It is as if pixel 0,0 ends up at texcoords
> 0,0 - the same thing as with wl_shm buffers. Therefore we need to invert
> the invert flag.
> 
> Too bad EGL_EXT_image_dma_buf_import does not seem to specify the image
> orientation. The GL spec implied result seems to conflict with the
> reality in Mesa 11.2.2.
> 
> I asked about this in the Mesa developer mailing list. The question with
> no answers:
> https://lists.freedesktop.org/archives/mesa-dev/2016-June/120249.html
> and the thread I hijacked to get some answers:
> https://lists.freedesktop.org/archives/mesa-dev/2016-June/120733.html
> which culminated to the conclusion:
> https://lists.freedesktop.org/archives/mesa-dev/2016-June/120955.html
> that supports this patch.
> 
> simple-dmabuf-v4l is equally fixed to not add Y_INVERT. There is no
> rational reason to have it, and removing is necessary together with the
> GL-renderer change to keep the image the right way up. This has been
> tested with VIVID.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

Hi,

forgot to add that this patch is a partial fix to
https://phabricator.freedesktop.org/T7475


Thanks,
pq

> ---
>  clients/simple-dmabuf-v4l.c | 7 ++++++-
>  libweston/gl-renderer.c     | 8 +++++++-
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/clients/simple-dmabuf-v4l.c b/clients/simple-dmabuf-v4l.c
> index 060b4fa..6ef0eb3 100644
> --- a/clients/simple-dmabuf-v4l.c
> +++ b/clients/simple-dmabuf-v4l.c
> @@ -353,7 +353,12 @@ create_dmabuf_buffer(struct display *display, struct buffer *buffer)
>  	unsigned i;
>  
>  	modifier = 0;
> -	flags = ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT;
> +	flags = 0;
> +
> +	/* XXX: apparently some webcams may actually provide y-inverted images,
> +	 * in which case we should set
> +	 * flags = ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT
> +	 */
>  
>  	params = zwp_linux_dmabuf_v1_create_params(display->dmabuf);
>  	for (i = 0; i < display->format.num_planes; ++i)
> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
> index 28c0b50..a404e7e 100644
> --- a/libweston/gl-renderer.c
> +++ b/libweston/gl-renderer.c
> @@ -1837,8 +1837,14 @@ gl_renderer_attach_dmabuf(struct weston_surface *surface,
>  
>  	buffer->width = dmabuf->attributes.width;
>  	buffer->height = dmabuf->attributes.height;
> +
> +	/*
> +	 * GL-renderer uses the OpenGL convention of texture coordinates, where
> +	 * the origin is at bottom-left. Because dmabuf buffers have the origin
> +	 * at top-left, we must invert the Y_INVERT flag to get the image right.
> +	 */
>  	buffer->y_inverted =
> -		!!(dmabuf->attributes.flags & ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT);
> +		!(dmabuf->attributes.flags & ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT);
>  
>  	for (i = 0; i < gs->num_images; i++)
>  		egl_image_unref(gs->images[i]);