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

[Quentin Glidic]

On 04/07/2016 15:40, Pekka Paalanen wrote:
> 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

It is a well-explained fix, and it works. I think we can fix the rest as 
follow-up patches.

Pushed both patches:
2eda27b..319397e  master -> master

Cheers,


>> ---
>>  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]);
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>


-- 

Quentin “Sardem FF7” Glidic