assembly: 0.980
other: 0.979
semantic: 0.979
graphic: 0.979
instruction: 0.973
vnc: 0.973
device: 0.972
socket: 0.961
KVM: 0.939
network: 0.933
boot: 0.931
mistranslation: 0.930

[regression] vnc tight png produces garbled output

VNC Tight PNG compression did work fine two or three month ago but don't anymore. Now when Tight PNG is used parts of the desktop are shown but they are scrambled together.
I have always tested this feature against QEMU git with noVNC by only allowing Tight PNG compression.

On Sat, Dec 8, 2012 at 12:46 PM, Tim Hardeck <email address hidden> wrote:
> Public bug reported:
>
> VNC Tight PNG compression did work fine two or three month ago but don't anymore. Now when Tight PNG is used parts of the desktop are shown but they are scrambled together.
> I have always tested this feature against QEMU git with noVNC by only allowing Tight PNG compression.

Hi Tim,
If you have a few minutes please use git-bisect(1) to identify the
commit that causes the regression.

The rough steps are:
1. Verify that qemu.git/master is broken and find an older commit
where it works.
2. Use git-bisect(1) to binary search the commit history between these
two points - it will leave you with the commit that caused the
regression.

Here some quick links to get you started:
http://git-scm.com/book/en/Git-Tools-Debugging-with-Git#Binary-Search
http://blog.evan.pro/getting-started-with-git-bisect-in-60-seconds

Stefan


47683d669f993308c2b84bed4ce64aafb5d7ced4 is the first bad commit
commit 47683d669f993308c2b84bed4ce64aafb5d7ced4
Author: Gerd Hoffmann <email address hidden>
Date:   Thu Oct 11 12:04:33 2012 +0200

    pixman/vnc: remove rgb_prepare_row* functions

    Let pixman do it instead.

    Signed-off-by: Gerd Hoffmann <email address hidden>

:040000 040000 653d58e66bf3a2d8240b2f9176979c44ccd720e1 6b6e367a8522cb58b42ad8f204387a354d3b3d00 M  ui


Just reverting this particular commit isn't enough thou but it is connected.

On Mon, Dec 10, 2012 at 03:54:58PM -0000, Tim Hardeck wrote:
> 47683d669f993308c2b84bed4ce64aafb5d7ced4 is the first bad commit
> commit 47683d669f993308c2b84bed4ce64aafb5d7ced4
> Author: Gerd Hoffmann <email address hidden>
> Date:   Thu Oct 11 12:04:33 2012 +0200
> 
>     pixman/vnc: remove rgb_prepare_row* functions
> 
>     Let pixman do it instead.
> 
>     Signed-off-by: Gerd Hoffmann <email address hidden>
> 
> :040000 040000 653d58e66bf3a2d8240b2f9176979c44ccd720e1
> 6b6e367a8522cb58b42ad8f204387a354d3b3d00 M  ui
> 
> 
> Just reverting this particular commit isn't enough thou but it is connected.

This suggests the conversion to pixman has introduced the bug.

Tim: can you provide steps to reproduce the bug?

Stefan


* make sure that qemu is compiled with --enable-vnc-png

* git clone git://github.com/kanaka/noVNC
* edit include/rfb.js at line 50 and comment out or remove all encodings above "['TIGHT_PNG',        -260 ],"
* open vnc.html in Firefox or Chrome

*apply either my patch to QEMU https://lists.nongnu.org/archive/html/qemu-devel/2012-12/msg00869.html or use Websockify https://github.com/kanaka/websockify to get Websocket support.

* in case of my patch run QEMU with  `-vnc :0,websocket` and connect with noVNC to port 5700.

* in case of Websockify run QEMU with `./websockify.py 5900 -- qemu-system-x86_64 -vnc :0` and connect to port 5900

On 12/11/12 09:57, Stefan Hajnoczi wrote:
> On Mon, Dec 10, 2012 at 03:54:58PM -0000, Tim Hardeck wrote:
>> 47683d669f993308c2b84bed4ce64aafb5d7ced4 is the first bad commit
>> commit 47683d669f993308c2b84bed4ce64aafb5d7ced4
>> Author: Gerd Hoffmann <email address hidden>
>> Date:   Thu Oct 11 12:04:33 2012 +0200
>>
>>     pixman/vnc: remove rgb_prepare_row* functions
>>
>>     Let pixman do it instead.
>>
>>     Signed-off-by: Gerd Hoffmann <email address hidden>
>>
>> :040000 040000 653d58e66bf3a2d8240b2f9176979c44ccd720e1
>> 6b6e367a8522cb58b42ad8f204387a354d3b3d00 M  ui
>>
>>
>> Just reverting this particular commit isn't enough thou but it is connected.
> 
> This suggests the conversion to pixman has introduced the bug.

Quite possible.  I've tested tight via 'vncviewer
PreferredEncoding=Tight', but that doesn't force png, so there is a
chance for issues I havn't seen.

> Tim: can you provide steps to reproduce the bug?

That will certainly help fixing.

Also: How garbled?  Totally messed up?  Just colors wrong?  Any other
visible pattern?

cheers,
  Gerd




On 12/11/12 11:09, Tim Hardeck wrote:
> * make sure that qemu is compiled with --enable-vnc-png
> 
> * git clone git://github.com/kanaka/noVNC
> * edit include/rfb.js at line 50 and comment out or remove all encodings above "['TIGHT_PNG',        -260 ],"
> * open vnc.html in Firefox or Chrome
> 
> *apply either my patch to QEMU https://lists.nongnu.org/archive/html
> /qemu-devel/2012-12/msg00869.html or use Websockify
> https://github.com/kanaka/websockify to get Websocket support.
> 
> * in case of my patch run QEMU with  `-vnc :0,websocket` and connect
> with noVNC to port 5700.
> 
> * in case of Websockify run QEMU with `./websockify.py 5900 -- qemu-
> system-x86_64 -vnc :0` and connect to port 5900

Hmm, doesn't reproduce here (using websockify proxy).

--- a/include/rfb.js
+++ b/include/rfb.js
@@ -48,8 +48,8 @@ var that           = {},  // Public API methods

     // In preference order
     encodings      = [
-        ['COPYRECT',         0x01 ],
-        ['TIGHT',            0x07 ],
+//        ['COPYRECT',         0x01 ],
+//        ['TIGHT',            0x07 ],
         ['TIGHT_PNG',        -260 ],
         ['HEXTILE',          0x05 ],
         ['RRE',              0x02 ],


cheers,
  Gerd


If you had opened vnc.html before the rfb.js might be still in the browser cache, at least I had a similar issue once.
I have tested this with Chrome but I think it happened in Firefox too.

This patch adds an x argument to qemu_pixman_linebuf_fill so it can
also be used to convert a partial scanline.  Then fix tight + png/jpeg
encoding by passing in the x+y offset, so the data is read from the
correct screen location instead of the upper left corner.

Cc: <email address hidden>
Cc: <email address hidden>
Reported-by: Tim Hardeneck <email address hidden>
Signed-off-by: Gerd Hoffmann <email address hidden>
---
 hw/vga.c           |    2 +-
 qemu-pixman.c      |    4 ++--
 qemu-pixman.h      |    2 +-
 ui/vnc-enc-tight.c |    4 ++--
 ui/vnc.c           |    2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index 568083a..22561a5 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2413,7 +2413,7 @@ void ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp)
     }
     linebuf = qemu_pixman_linebuf_create(PIXMAN_BE_r8g8b8, width);
     for (y = 0; y < height; y++) {
-        qemu_pixman_linebuf_fill(linebuf, ds->image, width, y);
+        qemu_pixman_linebuf_fill(linebuf, ds->image, width, 0, y);
         clearerr(f);
         ret = fwrite(pixman_image_get_data(linebuf), 1,
                      pixman_image_get_stride(linebuf), f);
diff --git a/qemu-pixman.c b/qemu-pixman.c
index e46e180..1473835 100644
--- a/qemu-pixman.c
+++ b/qemu-pixman.c
@@ -52,10 +52,10 @@ pixman_image_t *qemu_pixman_linebuf_create(pixman_format_code_t format,
 }
 
 void qemu_pixman_linebuf_fill(pixman_image_t *linebuf, pixman_image_t *fb,
-                              int width, int y)
+                              int width, int x, int y)
 {
     pixman_image_composite(PIXMAN_OP_SRC, fb, NULL, linebuf,
-                           0, y, 0, 0, 0, 0, width, 1);
+                           x, y, 0, 0, 0, 0, width, 1);
 }
 
 pixman_image_t *qemu_pixman_mirror_create(pixman_format_code_t format,
diff --git a/qemu-pixman.h b/qemu-pixman.h
index bee55eb..3c05c83 100644
--- a/qemu-pixman.h
+++ b/qemu-pixman.h
@@ -31,7 +31,7 @@ pixman_format_code_t qemu_pixman_get_format(PixelFormat *pf);
 pixman_image_t *qemu_pixman_linebuf_create(pixman_format_code_t format,
                                            int width);
 void qemu_pixman_linebuf_fill(pixman_image_t *linebuf, pixman_image_t *fb,
-                              int width, int y);
+                              int width, int x, int y);
 pixman_image_t *qemu_pixman_mirror_create(pixman_format_code_t format,
                                           pixman_image_t *image);
 void qemu_pixman_image_unref(pixman_image_t *image);
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 9ae4cab..62d0fde 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -1212,7 +1212,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality)
     buf = (uint8_t *)pixman_image_get_data(linebuf);
     row[0] = buf;
     for (dy = 0; dy < h; dy++) {
-        qemu_pixman_linebuf_fill(linebuf, vs->vd->server, w, dy);
+        qemu_pixman_linebuf_fill(linebuf, vs->vd->server, w, x, y + dy);
         jpeg_write_scanlines(&cinfo, row, 1);
     }
     qemu_pixman_image_unref(linebuf);
@@ -1356,7 +1356,7 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h,
         if (color_type == PNG_COLOR_TYPE_PALETTE) {
             memcpy(buf, vs->tight.tight.buffer + (dy * w), w);
         } else {
-            qemu_pixman_linebuf_fill(linebuf, vs->vd->server, w, dy);
+            qemu_pixman_linebuf_fill(linebuf, vs->vd->server, w, x, y + dy);
         }
         png_write_row(png_ptr, buf);
     }
diff --git a/ui/vnc.c b/ui/vnc.c
index f4486ad..4b15dd4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2570,7 +2570,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
             uint8_t *server_ptr;
 
             if (vd->guest.format != VNC_SERVER_FB_FORMAT) {
-                qemu_pixman_linebuf_fill(tmpbuf, vd->guest.fb, width, y);
+                qemu_pixman_linebuf_fill(tmpbuf, vd->guest.fb, width, 0, y);
                 guest_ptr = (uint8_t *)pixman_image_get_data(tmpbuf);
             } else {
                 guest_ptr = guest_row;
-- 
1.7.1



This patch adds an x argument to qemu_pixman_linebuf_fill so it can
also be used to convert a partial scanline.  Then fix tight + png/jpeg
encoding by passing in the x+y offset, so the data is read from the
correct screen location instead of the upper left corner.

Cc: <email address hidden>
Cc: <email address hidden>
Reported-by: Tim Hardeneck <email address hidden>
Signed-off-by: Gerd Hoffmann <email address hidden>
---
 hw/vga.c           |    2 +-
 qemu-pixman.c      |    4 ++--
 qemu-pixman.h      |    2 +-
 ui/vnc-enc-tight.c |    4 ++--
 ui/vnc.c           |    2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index 2b0200a..c266161 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2413,7 +2413,7 @@ void ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp)
     }
     linebuf = qemu_pixman_linebuf_create(PIXMAN_BE_r8g8b8, width);
     for (y = 0; y < height; y++) {
-        qemu_pixman_linebuf_fill(linebuf, ds->image, width, y);
+        qemu_pixman_linebuf_fill(linebuf, ds->image, width, 0, y);
         clearerr(f);
         ret = fwrite(pixman_image_get_data(linebuf), 1,
                      pixman_image_get_stride(linebuf), f);
diff --git a/qemu-pixman.c b/qemu-pixman.c
index 79e175b..e7263fb 100644
--- a/qemu-pixman.c
+++ b/qemu-pixman.c
@@ -52,10 +52,10 @@ pixman_image_t *qemu_pixman_linebuf_create(pixman_format_code_t format,
 }
 
 void qemu_pixman_linebuf_fill(pixman_image_t *linebuf, pixman_image_t *fb,
-                              int width, int y)
+                              int width, int x, int y)
 {
     pixman_image_composite(PIXMAN_OP_SRC, fb, NULL, linebuf,
-                           0, y, 0, 0, 0, 0, width, 1);
+                           x, y, 0, 0, 0, 0, width, 1);
 }
 
 pixman_image_t *qemu_pixman_mirror_create(pixman_format_code_t format,
diff --git a/qemu-pixman.h b/qemu-pixman.h
index bee55eb..3c05c83 100644
--- a/qemu-pixman.h
+++ b/qemu-pixman.h
@@ -31,7 +31,7 @@ pixman_format_code_t qemu_pixman_get_format(PixelFormat *pf);
 pixman_image_t *qemu_pixman_linebuf_create(pixman_format_code_t format,
                                            int width);
 void qemu_pixman_linebuf_fill(pixman_image_t *linebuf, pixman_image_t *fb,
-                              int width, int y);
+                              int width, int x, int y);
 pixman_image_t *qemu_pixman_mirror_create(pixman_format_code_t format,
                                           pixman_image_t *image);
 void qemu_pixman_image_unref(pixman_image_t *image);
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 9ae4cab..62d0fde 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -1212,7 +1212,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality)
     buf = (uint8_t *)pixman_image_get_data(linebuf);
     row[0] = buf;
     for (dy = 0; dy < h; dy++) {
-        qemu_pixman_linebuf_fill(linebuf, vs->vd->server, w, dy);
+        qemu_pixman_linebuf_fill(linebuf, vs->vd->server, w, x, y + dy);
         jpeg_write_scanlines(&cinfo, row, 1);
     }
     qemu_pixman_image_unref(linebuf);
@@ -1356,7 +1356,7 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h,
         if (color_type == PNG_COLOR_TYPE_PALETTE) {
             memcpy(buf, vs->tight.tight.buffer + (dy * w), w);
         } else {
-            qemu_pixman_linebuf_fill(linebuf, vs->vd->server, w, dy);
+            qemu_pixman_linebuf_fill(linebuf, vs->vd->server, w, x, y + dy);
         }
         png_write_row(png_ptr, buf);
     }
diff --git a/ui/vnc.c b/ui/vnc.c
index ba30362..04afcff 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2569,7 +2569,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
             uint8_t *server_ptr;
 
             if (vd->guest.format != VNC_SERVER_FB_FORMAT) {
-                qemu_pixman_linebuf_fill(tmpbuf, vd->guest.fb, width, y);
+                qemu_pixman_linebuf_fill(tmpbuf, vd->guest.fb, width, 0, y);
                 guest_ptr = (uint8_t *)pixman_image_get_data(tmpbuf);
             } else {
                 guest_ptr = guest_row;
-- 
1.7.1



The patch does fix the issue for me, thanks.

Gerd's patch had been included here:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=bc210eb163b162ff2
==> Setting status to "Fix released"