[PATCH] ui/cocoa: Fix stride resolution of pixman image

[PATCH] ui/cocoa: Fix stride resolution of pixman image

[Akihiko Odaki]

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 ui/cocoa.m | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 0ef5fdf3b7a..2de72155dea 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -450,19 +450,19 @@ - (void) drawRect:(NSRect) rect
         int w = pixman_image_get_width(pixman_image);
         int h = pixman_image_get_height(pixman_image);
         int bitsPerPixel = PIXMAN_FORMAT_BPP(pixman_image_get_format(pixman_image));
-        int bitsPerComponent = DIV_ROUND_UP(bitsPerPixel, 8) * 2;
+        int stride = pixman_image_get_stride(pixman_image);
         CGDataProviderRef dataProviderRef = CGDataProviderCreateWithData(
             NULL,
             pixman_image_get_data(pixman_image),
-            w * 4 * h,
+            stride * h,
             NULL
         );
         CGImageRef imageRef = CGImageCreate(
             w, //width
             h, //height
-            bitsPerComponent, //bitsPerComponent
+            DIV_ROUND_UP(bitsPerPixel, 8) * 2, //bitsPerComponent
             bitsPerPixel, //bitsPerPixel
-            (w * (bitsPerComponent/2)), //bytesPerRow
+            stride, //bytesPerRow
 #ifdef __LITTLE_ENDIAN__
             CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace for OS X >= 10.4
             kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst,
-- 
2.24.3 (Apple Git-128)

Re: [PATCH] ui/cocoa: Fix stride resolution of pixman image

[Peter Maydell]

On Sun, 21 Feb 2021 at 17:31, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>

In what situations does this change make a difference?
Obviously most of the time it works fine, or we'd have noticed
before now.

(This is the kind of detail that it's useful to provide in
the commit message.)

thanks
-- PMM

[PATCH v2] ui/cocoa: Fix stride resolution of pixman image

[Akihiko Odaki]

A display can receive an image which its stride is greater than its
width. In fact, when a guest requests virtio-gpu to scan out a
smaller part of an image, virtio-gpu passes it to a display as an
image which its width represents the one of the part and its stride
equals to the one of the whole image.

This change makes ui/cocoa to cover such cases.

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 ui/cocoa.m | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 0ef5fdf3b7a..2de72155dea 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -450,19 +450,19 @@ - (void) drawRect:(NSRect) rect
         int w = pixman_image_get_width(pixman_image);
         int h = pixman_image_get_height(pixman_image);
         int bitsPerPixel = PIXMAN_FORMAT_BPP(pixman_image_get_format(pixman_image));
-        int bitsPerComponent = DIV_ROUND_UP(bitsPerPixel, 8) * 2;
+        int stride = pixman_image_get_stride(pixman_image);
         CGDataProviderRef dataProviderRef = CGDataProviderCreateWithData(
             NULL,
             pixman_image_get_data(pixman_image),
-            w * 4 * h,
+            stride * h,
             NULL
         );
         CGImageRef imageRef = CGImageCreate(
             w, //width
             h, //height
-            bitsPerComponent, //bitsPerComponent
+            DIV_ROUND_UP(bitsPerPixel, 8) * 2, //bitsPerComponent
             bitsPerPixel, //bitsPerPixel
-            (w * (bitsPerComponent/2)), //bytesPerRow
+            stride, //bytesPerRow
 #ifdef __LITTLE_ENDIAN__
             CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace for OS X >= 10.4
             kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst,
-- 
2.24.3 (Apple Git-128)

Re: [PATCH v2] ui/cocoa: Fix stride resolution of pixman image

[Gerd Hoffmann]

On Mon, Feb 22, 2021 at 11:40:12PM +0900, Akihiko Odaki wrote:
> A display can receive an image which its stride is greater than its
> width. In fact, when a guest requests virtio-gpu to scan out a
> smaller part of an image, virtio-gpu passes it to a display as an
> image which its width represents the one of the part and its stride
> equals to the one of the whole image.

Probably not limited to virtio-gpu.  Wayland rounds display framebuffers
to the next multiple of 64, so when running -- for example -- 800x600
wayland will create an image 832 pixels wide.  Other UIs had simliar
issues.

Patch added to UI patch queue.

thanks,
  Gerd

Re: [PATCH v2] ui/cocoa: Fix stride resolution of pixman image

[Peter Maydell]

On Wed, 24 Feb 2021 at 11:23, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Mon, Feb 22, 2021 at 11:40:12PM +0900, Akihiko Odaki wrote:
> > A display can receive an image which its stride is greater than its
> > width. In fact, when a guest requests virtio-gpu to scan out a
> > smaller part of an image, virtio-gpu passes it to a display as an
> > image which its width represents the one of the part and its stride
> > equals to the one of the whole image.
>
> Probably not limited to virtio-gpu.  Wayland rounds display framebuffers
> to the next multiple of 64, so when running -- for example -- 800x600
> wayland will create an image 832 pixels wide.  Other UIs had simliar
> issues.
>
> Patch added to UI patch queue.

Could you add Akihiko's explanation to the commit message
for the patch in your queue, please?

thanks
-- PMM

Re: [PATCH v2] ui/cocoa: Fix stride resolution of pixman image

[Gerd Hoffmann]

On Wed, Feb 24, 2021 at 01:08:22PM +0000, Peter Maydell wrote:
> On Wed, 24 Feb 2021 at 11:23, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Mon, Feb 22, 2021 at 11:40:12PM +0900, Akihiko Odaki wrote:
> > > A display can receive an image which its stride is greater than its
> > > width. In fact, when a guest requests virtio-gpu to scan out a
> > > smaller part of an image, virtio-gpu passes it to a display as an
> > > image which its width represents the one of the part and its stride
> > > equals to the one of the whole image.
> >
> > Probably not limited to virtio-gpu.  Wayland rounds display framebuffers
> > to the next multiple of 64, so when running -- for example -- 800x600
> > wayland will create an image 832 pixels wide.  Other UIs had simliar
> > issues.
> >
> > Patch added to UI patch queue.
> 
> Could you add Akihiko's explanation to the commit message
> for the patch in your queue, please?

That _is_ the (v2) commit message ;)

Akihiko: new versions of a patch should be sent as new thread, not as
reply.  It is less confusing for both people and tools like b4
(https://pypi.org/project/b4/) which help with patch processing.

take care,
  Gerd

Re: [PATCH v2] ui/cocoa: Fix stride resolution of pixman image

[Akihiko Odaki]

2021年2月25日(木) 17:02 Gerd Hoffmann <kraxel@redhat.com>:
>
> On Wed, Feb 24, 2021 at 01:08:22PM +0000, Peter Maydell wrote:
> > On Wed, 24 Feb 2021 at 11:23, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > > On Mon, Feb 22, 2021 at 11:40:12PM +0900, Akihiko Odaki wrote:
> > > > A display can receive an image which its stride is greater than its
> > > > width. In fact, when a guest requests virtio-gpu to scan out a
> > > > smaller part of an image, virtio-gpu passes it to a display as an
> > > > image which its width represents the one of the part and its stride
> > > > equals to the one of the whole image.
> > >
> > > Probably not limited to virtio-gpu.  Wayland rounds display framebuffers
> > > to the next multiple of 64, so when running -- for example -- 800x600
> > > wayland will create an image 832 pixels wide.  Other UIs had simliar
> > > issues.
> > >
> > > Patch added to UI patch queue.
> >
> > Could you add Akihiko's explanation to the commit message
> > for the patch in your queue, please?
>
> That _is_ the (v2) commit message ;)
>
> Akihiko: new versions of a patch should be sent as new thread, not as
> reply.  It is less confusing for both people and tools like b4
> (https://pypi.org/project/b4/) which help with patch processing.

I didn't know that. Thanks for telling me that. I'll do so next time.

Regards,
Akihiko Odaki

>
> take care,
>   Gerd
>