[rocko][PATCH] weston: add patch to set pitch correctly for subsampled textures

[rocko][PATCH] weston: add patch to set pitch correctly for subsampled textures

[Andrey Konovalov]

This fixes display issue with YUV420/I420 and NV12 formats, that
can result in crash of weston.

The master branch has this fix as part of commit 148920f3971d "weston:
Bump version to 3.0.0". The patch has been rebased to apply cleanly
to weston 2.0.0.

Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
---
 ...t-pitch-correctly-for-subsampled-textures.patch | 55 ++++++++++++++++++++++
 meta/recipes-graphics/wayland/weston_2.0.0.bb      |  1 +
 2 files changed, 56 insertions(+)
 create mode 100644 meta/recipes-graphics/wayland/weston/weston-gl-renderer-Set-pitch-correctly-for-subsampled-textures.patch

diff --git a/meta/recipes-graphics/wayland/weston/weston-gl-renderer-Set-pitch-correctly-for-subsampled-textures.patch b/meta/recipes-graphics/wayland/weston/weston-gl-renderer-Set-pitch-correctly-for-subsampled-textures.patch
new file mode 100644
index 0000000..c188018
--- /dev/null
+++ b/meta/recipes-graphics/wayland/weston/weston-gl-renderer-Set-pitch-correctly-for-subsampled-textures.patch
@@ -0,0 +1,55 @@
+Multi-plane sub-sampled textures have partial width/height, e.g.
+YUV420/I420 has a full-size Y plane, followed by a half-width/height U
+plane, and a half-width/height V plane.
+
+zwp_linux_dmabuf_v1 allows clients to pass an explicit pitch for each
+plane, but for wl_shm this must be inferred. gl-renderer was correctly
+accounting for the width and height when subsampling, but the pitch was
+being taken as the pitch for the first plane.
+
+This does not match the requirements for GStreamer's waylandsink, in
+particular, as well as other clients. Fix the SHM upload path to
+correctly set the pitch for each plane, according to subsampling.
+
+Tested with:
+  $ gst-launch-1.0 videotestsrc ! waylandsink
+
+Upstream-Status: Backport [https://patchwork.freedesktop.org/patch/180767/]
+
+Signed-off-by: Daniel Stone <daniels@collabora.com>
+Fixes: fdeefe42418 ("gl-renderer: add support of WL_SHM_FORMAT_YUV420")
+Reported-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
+Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103063
+
+---
+ libweston/gl-renderer.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
+index 244ce309..40bf0bb6 100644
+--- a/libweston/gl-renderer.c
++++ b/libweston/gl-renderer.c
+@@ -1285,14 +1285,13 @@ gl_renderer_flush_damage(struct weston_surface *surface)
+ 		goto done;
+ 	}
+ 
+-	glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, gs->pitch);
+-
+ 	if (gs->needs_full_upload) {
+ 		glPixelStorei(GL_UNPACK_SKIP_PIXELS_EXT, 0);
+ 		glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, 0);
+ 		wl_shm_buffer_begin_access(buffer->shm_buffer);
+ 		for (j = 0; j < gs->num_textures; j++) {
+ 			glBindTexture(GL_TEXTURE_2D, gs->textures[j]);
++			glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, gs->pitch / gs->hsub[j]);
+ 			glTexImage2D(GL_TEXTURE_2D, 0,
+ 				     gs->gl_format[j],
+ 				     gs->pitch / gs->hsub[j],
+@@ -1317,6 +1316,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
+ 		glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, r.y1);
+ 		for (j = 0; j < gs->num_textures; j++) {
+ 			glBindTexture(GL_TEXTURE_2D, gs->textures[j]);
++			glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, gs->pitch / gs->hsub[j]);
+ 			glTexSubImage2D(GL_TEXTURE_2D, 0,
+ 					r.x1 / gs->hsub[j],
+ 					r.y1 / gs->vsub[j],
diff --git a/meta/recipes-graphics/wayland/weston_2.0.0.bb b/meta/recipes-graphics/wayland/weston_2.0.0.bb
index 54b07bd..063494c 100644
--- a/meta/recipes-graphics/wayland/weston_2.0.0.bb
+++ b/meta/recipes-graphics/wayland/weston_2.0.0.bb
@@ -12,6 +12,7 @@ SRC_URI = "https://wayland.freedesktop.org/releases/${BPN}-${PV}.tar.xz \
            file://0001-configure.ac-Fix-wayland-protocols-path.patch \
            file://xwayland.weston-start \
            file://0001-weston-launch-Provide-a-default-version-that-doesn-t.patch \
+           file://weston-gl-renderer-Set-pitch-correctly-for-subsampled-textures.patch \
 "
 SRC_URI[md5sum] = "15f38945942bf2a91fe2687145fb4c7d"
 SRC_URI[sha256sum] = "b4e446ac27f118196f1609dab89bb3cb3e81652d981414ad860e733b355365d8"
-- 
2.7.4

Re: [rocko][PATCH] weston: add patch to set pitch correctly for subsampled textures

[Andrey Konovalov]

This exact patch also applies to pyro.
Should I send the separate "[OE-core][pyro][PATCH] weston: add patch to set pitch correctly for subsampled textures" email,
or just asking to merge it into pyro as well is enough?

Thanks,
Andrey

On 24.11.2017 22:47, Andrey Konovalov wrote:
> This fixes display issue with YUV420/I420 and NV12 formats, that
> can result in crash of weston.
> 
> The master branch has this fix as part of commit 148920f3971d "weston:
> Bump version to 3.0.0". The patch has been rebased to apply cleanly
> to weston 2.0.0.
> 
> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
> ---
>   ...t-pitch-correctly-for-subsampled-textures.patch | 55 ++++++++++++++++++++++
>   meta/recipes-graphics/wayland/weston_2.0.0.bb      |  1 +
>   2 files changed, 56 insertions(+)
>   create mode 100644 meta/recipes-graphics/wayland/weston/weston-gl-renderer-Set-pitch-correctly-for-subsampled-textures.patch
> 
> diff --git a/meta/recipes-graphics/wayland/weston/weston-gl-renderer-Set-pitch-correctly-for-subsampled-textures.patch b/meta/recipes-graphics/wayland/weston/weston-gl-renderer-Set-pitch-correctly-for-subsampled-textures.patch
> new file mode 100644
> index 0000000..c188018
> --- /dev/null
> +++ b/meta/recipes-graphics/wayland/weston/weston-gl-renderer-Set-pitch-correctly-for-subsampled-textures.patch
> @@ -0,0 +1,55 @@
> +Multi-plane sub-sampled textures have partial width/height, e.g.
> +YUV420/I420 has a full-size Y plane, followed by a half-width/height U
> +plane, and a half-width/height V plane.
> +
> +zwp_linux_dmabuf_v1 allows clients to pass an explicit pitch for each
> +plane, but for wl_shm this must be inferred. gl-renderer was correctly
> +accounting for the width and height when subsampling, but the pitch was
> +being taken as the pitch for the first plane.
> +
> +This does not match the requirements for GStreamer's waylandsink, in
> +particular, as well as other clients. Fix the SHM upload path to
> +correctly set the pitch for each plane, according to subsampling.
> +
> +Tested with:
> +  $ gst-launch-1.0 videotestsrc ! waylandsink
> +
> +Upstream-Status: Backport [https://patchwork.freedesktop.org/patch/180767/]
> +
> +Signed-off-by: Daniel Stone <daniels@collabora.com>
> +Fixes: fdeefe42418 ("gl-renderer: add support of WL_SHM_FORMAT_YUV420")
> +Reported-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> +Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103063
> +
> +---
> + libweston/gl-renderer.c | 4 ++--
> + 1 file changed, 2 insertions(+), 2 deletions(-)
> +
> +diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
> +index 244ce309..40bf0bb6 100644
> +--- a/libweston/gl-renderer.c
> ++++ b/libweston/gl-renderer.c
> +@@ -1285,14 +1285,13 @@ gl_renderer_flush_damage(struct weston_surface *surface)
> + 		goto done;
> + 	}
> +
> +-	glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, gs->pitch);
> +-
> + 	if (gs->needs_full_upload) {
> + 		glPixelStorei(GL_UNPACK_SKIP_PIXELS_EXT, 0);
> + 		glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, 0);
> + 		wl_shm_buffer_begin_access(buffer->shm_buffer);
> + 		for (j = 0; j < gs->num_textures; j++) {
> + 			glBindTexture(GL_TEXTURE_2D, gs->textures[j]);
> ++			glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, gs->pitch / gs->hsub[j]);
> + 			glTexImage2D(GL_TEXTURE_2D, 0,
> + 				     gs->gl_format[j],
> + 				     gs->pitch / gs->hsub[j],
> +@@ -1317,6 +1316,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
> + 		glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, r.y1);
> + 		for (j = 0; j < gs->num_textures; j++) {
> + 			glBindTexture(GL_TEXTURE_2D, gs->textures[j]);
> ++			glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, gs->pitch / gs->hsub[j]);
> + 			glTexSubImage2D(GL_TEXTURE_2D, 0,
> + 					r.x1 / gs->hsub[j],
> + 					r.y1 / gs->vsub[j],
> diff --git a/meta/recipes-graphics/wayland/weston_2.0.0.bb b/meta/recipes-graphics/wayland/weston_2.0.0.bb
> index 54b07bd..063494c 100644
> --- a/meta/recipes-graphics/wayland/weston_2.0.0.bb
> +++ b/meta/recipes-graphics/wayland/weston_2.0.0.bb
> @@ -12,6 +12,7 @@ SRC_URI = "https://wayland.freedesktop.org/releases/${BPN}-${PV}.tar.xz \
>              file://0001-configure.ac-Fix-wayland-protocols-path.patch \
>              file://xwayland.weston-start \
>              file://0001-weston-launch-Provide-a-default-version-that-doesn-t.patch \
> +           file://weston-gl-renderer-Set-pitch-correctly-for-subsampled-textures.patch \
>   "
>   SRC_URI[md5sum] = "15f38945942bf2a91fe2687145fb4c7d"
>   SRC_URI[sha256sum] = "b4e446ac27f118196f1609dab89bb3cb3e81652d981414ad860e733b355365d8"
> 

Re: [rocko][PATCH] weston: add patch to set pitch correctly for subsampled textures

[Leonardo Sandoval]

On Fri, 24 Nov 2017 22:58:20 +0300
Andrey Konovalov <andrey.konovalov@linaro.org> wrote:

> This exact patch also applies to pyro.
> Should I send the separate "[OE-core][pyro][PATCH] weston: add patch to set pitch correctly for subsampled textures" email,
> or just asking to merge it into pyro as well is enough?

better if you send another patch with that subject so the non-master maintainer can pick it up.

Re: [rocko][PATCH] weston: add patch to set pitch correctly for subsampled textures

[Andrey Konovalov]

On 24.11.2017 23:34, Leonardo Sandoval wrote:
> On Fri, 24 Nov 2017 22:58:20 +0300
> Andrey Konovalov <andrey.konovalov@linaro.org> wrote:
> 
>> This exact patch also applies to pyro.
>> Should I send the separate "[OE-core][pyro][PATCH] weston: add patch to set pitch correctly for subsampled textures" email,
>> or just asking to merge it into pyro as well is enough?
> 
> better if you send another patch with that subject so the non-master maintainer can pick it up.

OK, have just sent the pyro version of this patch.

Looks like it confuses the patchwork system a bit:
> This is a system generated Comment: Patch 146165 was automatically marked as superseded by patch 146166.
To be clear: the same patch should be applied to both pyro and rocko. Patch 146165 was posted for rocko (with [rocko] in the subject line),
and patch 146166 is for pyro (has [pyro] in the subject line).

Thanks,
Andrey

Re: [rocko][PATCH] weston: add patch to set pitch correctly for subsampled textures

[Leonardo Sandoval]

On Sat, 25 Nov 2017 22:59:36 +0300
Andrey Konovalov <andrey.konovalov@linaro.org> wrote:

> On 24.11.2017 23:34, Leonardo Sandoval wrote:
> > On Fri, 24 Nov 2017 22:58:20 +0300
> > Andrey Konovalov <andrey.konovalov@linaro.org> wrote:
> >   
> >> This exact patch also applies to pyro.
> >> Should I send the separate "[OE-core][pyro][PATCH] weston: add patch to set pitch correctly for subsampled textures" email,
> >> or just asking to merge it into pyro as well is enough?  
> > 
> > better if you send another patch with that subject so the non-master maintainer can pick it up.  
> 
> OK, have just sent the pyro version of this patch.
> 
> Looks like it confuses the patchwork system a bit:

Right. Past weeks there were some issues for those series that were intended for non-master branches. This is a new enhancement and now is working as expected.


> > This is a system generated Comment: Patch 146165 was automatically marked as superseded by patch 146166.  
> To be clear: the same patch should be applied to both pyro and rocko. Patch 146165 was posted for rocko (with [rocko] in the subject line),
> and patch 146166 is for pyro (has [pyro] in the subject line).

Perfect, thanks for this extra work.

> 
> Thanks,
> Andrey
> 

✗ patchtest: failure for weston: add patch to set pitch correctly for subsampled textures

[Patchwork]

== Series Details ==

Series: weston: add patch to set pitch correctly for subsampled textures
Revision: 1
URL   : https://patchwork.openembedded.org/series/9949/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             New patch path missing in SRC_URI [test_src_uri_path_not_updated] 
  Suggested fix    Add the patch path to the recipe's SRC_URI
  New patch(es)    weston-gl-renderer-Set-pitch-correctly-for-subsampled-textures.patch



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Re: ✗ patchtest: failure for weston: add patch to set pitch correctly for subsampled textures

[Leonardo Sandoval]

On Fri, 24 Nov 2017 20:04:08 -0000
Patchwork <patchwork@patchwork.openembedded.org> wrote:

> == Series Details ==
> 
> Series: weston: add patch to set pitch correctly for subsampled textures
> Revision: 1
> URL   : https://patchwork.openembedded.org/series/9949/
> State : failure
> 
> == Summary ==
> 
> 
> Thank you for submitting this patch series to OpenEmbedded Core. This is
> an automated response. Several tests have been executed on the proposed
> series by patchtest resulting in the following failures:
> 
> 
> 
> * Issue             New patch path missing in SRC_URI [test_src_uri_path_not_updated] 
>   Suggested fix    Add the patch path to the recipe's SRC_URI
>   New patch(es)    weston-gl-renderer-Set-pitch-correctly-for-subsampled-textures.patch

I believe this is a false-positive from the system of a check that was just added [1]

I will revert it until fix it.

[1]
http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe/commit/?id=49201c19cfe4cadd127b112d2858d5b28db49c20

> 
> 
> 
> If you believe any of these test results are incorrect, please reply to the
> mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
> Otherwise we would appreciate you correcting the issues and submitting a new
> version of the patchset if applicable. Please ensure you add/increment the
> version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
> [PATCH v3] -> ...).
> 
> ---
> Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
> Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
> Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core