Re: [RFC PATCH xserver] xwayland: Fix non-argb cursor conversion

[Adam Jackson]

On Tue, 2017-12-05 at 10:51 +, Daniel Stone wrote:
> On 27 September 2017 at 17:01, Olivier Fourdan 
> wrote:
> > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=103012
> > Signed-off-by: Olivier Fourdan 
> 
> Reviewed-by: Daniel Stone 

remote: I: patch #179214 updated using rev 
97ac59b1ed3624f7c04e54dd3e3dadfa46a8f170.
remote: I: 1 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   a4c1e29082..97ac59b1ed  master -> master

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [RFC PATCH xserver] xwayland: Fix non-argb cursor conversion

[Daniel Stone]

On 27 September 2017 at 17:01, Olivier Fourdan  wrote:
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=103012
> Signed-off-by: Olivier Fourdan 

Reviewed-by: Daniel Stone 
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [RFC PATCH xserver] xwayland: Fix non-argb cursor conversion

[Emil Velikov]

Hi Olivier,

On 27 September 2017 at 17:01, Olivier Fourdan  wrote:
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=103012
> Signed-off-by: Olivier Fourdan 
> ---
>  Note: I am not familiar with this so I have no idea whether or not the
>  fix is correct (thus the RFC), but it does fix the test case provided
>  in bug 103012.
>
>  hw/xwayland/xwayland-cursor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
> index c95f4e830..cf8395f1d 100644
> --- a/hw/xwayland/xwayland-cursor.c
> +++ b/hw/xwayland/xwayland-cursor.c
> @@ -42,7 +42,7 @@ expand_source_and_mask(CursorPtr cursor, CARD32 *data)
>  (cursor->foreGreen & 0xff00) | (cursor->foreGreen >> 8);
>  bg = ((cursor->backRed & 0xff00) << 8) |
>  (cursor->backGreen & 0xff00) | (cursor->backGreen >> 8);
> -stride = (bits->width / 8 + 3) & ~3;
> +stride = BitmapBytePad(bits->width);

Bit of a disclaimer - I'm far from expert in the area.

The pattern seems like a common mistake, where one would align after
the division instead of the other way around.
As width is != 32 the code will provide larger than needed stride,
which seems to be exactly what the commit summary says.

One might mention exactly what's happening the commit message, since
it's not immediately obvious.

With the above disclaimer in mind, patch is
Reviewed-by: Emil Velikov 

-Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [RFC PATCH xserver] xwayland: Fix non-argb cursor conversion

[Olivier Fourdan]

Hi all,

Anyone to review/comment on this patch below?

It seems to fix the issue wit hte reproducer from the bug, but I dunno if this 
is right fix...

Thanks
Olivier

> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=103012
> Signed-off-by: Olivier Fourdan 
> ---
>  Note: I am not familiar with this so I have no idea whether or not the
>  fix is correct (thus the RFC), but it does fix the test case provided
>  in bug 103012.
> 
>  hw/xwayland/xwayland-cursor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
> index c95f4e830..cf8395f1d 100644
> --- a/hw/xwayland/xwayland-cursor.c
> +++ b/hw/xwayland/xwayland-cursor.c
> @@ -42,7 +42,7 @@ expand_source_and_mask(CursorPtr cursor, CARD32 *data)
>  (cursor->foreGreen & 0xff00) | (cursor->foreGreen >> 8);
>  bg = ((cursor->backRed & 0xff00) << 8) |
>  (cursor->backGreen & 0xff00) | (cursor->backGreen >> 8);
> -stride = (bits->width / 8 + 3) & ~3;
> +stride = BitmapBytePad(bits->width);
>  for (y = 0; y < bits->height; y++)
>  for (x = 0; x < bits->width; x++) {
>  i = y * stride + x / 8;
> --
> 2.13.5
> 
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[RFC PATCH xserver] xwayland: Fix non-argb cursor conversion

[Olivier Fourdan]

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=103012
Signed-off-by: Olivier Fourdan 
---
 Note: I am not familiar with this so I have no idea whether or not the
 fix is correct (thus the RFC), but it does fix the test case provided
 in bug 103012.

 hw/xwayland/xwayland-cursor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
index c95f4e830..cf8395f1d 100644
--- a/hw/xwayland/xwayland-cursor.c
+++ b/hw/xwayland/xwayland-cursor.c
@@ -42,7 +42,7 @@ expand_source_and_mask(CursorPtr cursor, CARD32 *data)
 (cursor->foreGreen & 0xff00) | (cursor->foreGreen >> 8);
 bg = ((cursor->backRed & 0xff00) << 8) |
 (cursor->backGreen & 0xff00) | (cursor->backGreen >> 8);
-stride = (bits->width / 8 + 3) & ~3;
+stride = BitmapBytePad(bits->width);
 for (y = 0; y < bits->height; y++)
 for (x = 0; x < bits->width; x++) {
 i = y * stride + x / 8;
-- 
2.13.5

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel