Corrupt VNC display for 1366x768

Corrupt VNC display for 1366x768

[Simon Rowe]

[-- Attachment #1: Type: text/plain, Size: 677 bytes --]

I've been trying to track down the cause of a glitch that affects guest VNC consoles when the resolution is set to 1366x768. This results in a "stair case" effect where each successive row is offset to the right by a handful of pixels. I believe this is related to the fact that the horizontal resolution only divisible by 2, not 16 which most others are.

There was a similar report many years ago, with a proposed patch

https://lists.gnu.org/archive/html/qemu-devel/2013-03/msg02732.html

but this was never committed and the code has been significantly reworked in the meantime.

Could anyone give me any pointers as to where the problem may lie?

Thanks
Simon

[-- Attachment #2: Type: text/html, Size: 2787 bytes --]

Re: Corrupt VNC display for 1366x768

[Daniel P. Berrangé]

On Wed, Oct 02, 2024 at 11:09:13AM +0000, Simon Rowe wrote:
> I've been trying to track down the cause of a glitch that affects guest VNC consoles when the resolution is set to 1366x768. This results in a "stair case" effect where each successive row is offset to the right by a handful of pixels. I believe this is related to the fact that the horizontal resolution only divisible by 2, not 16 which most others are.
> 
> There was a similar report many years ago, with a proposed patch
> 
> https://lists.gnu.org/archive/html/qemu-devel/2013-03/msg02732.html
> 
> but this was never committed and the code has been significantly reworked in the meantime.
> 
> Could anyone give me any pointers as to where the problem may lie?

There's a newer bug report here, but not real progress:

  https://gitlab.com/qemu-project/qemu/-/issues/90

1366 is particularly problematic as it apparently can't be represented
exactly in EDID which needs a x8 multiple.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: Corrupt VNC display for 1366x768

[Simon Rowe]

[-- Attachment #1: Type: text/plain, Size: 587 bytes --]

On 02/10/2024, 13:01, "Daniel P. Berrangé" <berrange@redhat.com> wrote:

> There's a newer bug report here, but not real progress:
>
> https://gitlab.com/qemu-project/qemu/-/issues/90
>
> 1366 is particularly problematic as it apparently can't be represented
> exactly in EDID which needs a x8 multiple.

Thanks for the pointers. My failing VM is UEFI so I'm not sure all the mentions of legacy display emulation are significant. I was hoping it was just a processing problem in the VNC client update code (the display looks OK (sorta) apart from the tearing).
Regards
Simon

[-- Attachment #2: Type: text/html, Size: 2826 bytes --]

Re: Corrupt VNC display for 1366x768

[Simon Rowe]

[-- Attachment #1: Type: text/plain, Size: 2331 bytes --]

Looking at the trace output it seems that the displaysurface has been rounded from the start
vnc_client_connect VNC client connect state=0x556dce1c1b20 ioc=0x556dce9e1e70
displaysurface_create_from surface=0x556dce104b30, 1360x768, format 0x20020888
vnc_server_dpy_recreate VNC server dpy recreate dpy=0x7faa59026010 size=1360x768 fmt=537004168
vnc_client_throttle_threshold VNC client throttle threshold state=0x556dce1c1b20 ioc=0x556dce9e1e70 oldoffset=0 newoffset=1048576 width=0 height=0 bpp=0 audio=(nil)
displaysurface_free surface=0x556dcebf10d0
vnc_auth_start VNC client auth start state=0x556dce1c1b20 method=1
vnc_auth_pass VNC client auth passed state=0x556dce1c1b20 method=1
vnc_client_throttle_threshold VNC client throttle threshold state=0x556dce1c1b20 ioc=0x556dce9e1e70 oldoffset=1048576 newoffset=4177920 width=1360 height=768 bpp=4 audio=(nil)
displaysurface_create_from surface=0x556dce140de0, 1360x768, format 0x20020888
vnc_server_dpy_pageflip VNC server dpy pageflip dpy=0x7faa59026010 size=1360x768 fmt=537004168
displaysurface_free surface=0x556dce104b30
vnc_client_throttle_threshold VNC client throttle threshold state=0x556dce1c1b20 ioc=0x556dce9e1e70 oldoffset=4177920 newoffset=1048576 width=1360 height=768 bpp=1 audio=(nil)
vnc_job_add_rect VNC add rect state=0x556dce1c1b20 job=0x556dce0ea1e0 offset=0,0 size=1360x768
vnc_job_clamp_rect VNC job clamp rect state=0x7faa42de53a0 job=0x556dce0ea1e0 offset=0,0 size=1360x768
vnc_job_clamped_rect VNC job clamp rect state=0x7faa42de53a0 job=0x556dce0ea1e0 offset=0,0 size=1360x768
vnc_job_nrects VNC job state=0x7faa42de53a0 job=0x556dce0ea1e0 nrects=1
vnc_client_unthrottle_forced VNC client unthrottle forced offset state=0x556dce1c1b20 ioc=0x556dce9e1e70
vnc_job_add_rect VNC add rect state=0x556dce1c1b20 job=0x556dce217810 offset=0,0 size=1360x13
vnc_job_clamp_rect VNC job clamp rect state=0x7faa42de53a0 job=0x556dce217810 offset=0,0 size=1360x13
vnc_job_clamped_rect VNC job clamp rect state=0x7faa42de53a0 job=0x556dce217810 offset=0,0 size=1360x13
vnc_job_nrects VNC job state=0x7faa42de53a0 job=0x556dce217810 nrects=1
vnc_client_unthrottle_forced VNC client unthrottle forced offset state=0x556dce1c1b20 ioc=0x556dce9e1e70

I'm currently struggling to follow where the width parameter is taken from.
Regards
Simon

[-- Attachment #2: Type: text/html, Size: 7443 bytes --]

Re: Corrupt VNC display for 1366x768

[Daniel P. Berrangé]

On Thu, Oct 03, 2024 at 01:01:04PM +0000, Simon Rowe wrote:
> Looking at the trace output it seems that the displaysurface has been rounded from the start
> vnc_client_connect VNC client connect state=0x556dce1c1b20 ioc=0x556dce9e1e70
> displaysurface_create_from surface=0x556dce104b30, 1360x768, format 0x20020888
> vnc_server_dpy_recreate VNC server dpy recreate dpy=0x7faa59026010 size=1360x768 fmt=537004168
> vnc_client_throttle_threshold VNC client throttle threshold state=0x556dce1c1b20 ioc=0x556dce9e1e70 oldoffset=0 newoffset=1048576 width=0 height=0 bpp=0 audio=(nil)
> displaysurface_free surface=0x556dcebf10d0
> vnc_auth_start VNC client auth start state=0x556dce1c1b20 method=1
> vnc_auth_pass VNC client auth passed state=0x556dce1c1b20 method=1
> vnc_client_throttle_threshold VNC client throttle threshold state=0x556dce1c1b20 ioc=0x556dce9e1e70 oldoffset=1048576 newoffset=4177920 width=1360 height=768 bpp=4 audio=(nil)
> displaysurface_create_from surface=0x556dce140de0, 1360x768, format 0x20020888
> vnc_server_dpy_pageflip VNC server dpy pageflip dpy=0x7faa59026010 size=1360x768 fmt=537004168
> displaysurface_free surface=0x556dce104b30
> vnc_client_throttle_threshold VNC client throttle threshold state=0x556dce1c1b20 ioc=0x556dce9e1e70 oldoffset=4177920 newoffset=1048576 width=1360 height=768 bpp=1 audio=(nil)
> vnc_job_add_rect VNC add rect state=0x556dce1c1b20 job=0x556dce0ea1e0 offset=0,0 size=1360x768
> vnc_job_clamp_rect VNC job clamp rect state=0x7faa42de53a0 job=0x556dce0ea1e0 offset=0,0 size=1360x768
> vnc_job_clamped_rect VNC job clamp rect state=0x7faa42de53a0 job=0x556dce0ea1e0 offset=0,0 size=1360x768
> vnc_job_nrects VNC job state=0x7faa42de53a0 job=0x556dce0ea1e0 nrects=1
> vnc_client_unthrottle_forced VNC client unthrottle forced offset state=0x556dce1c1b20 ioc=0x556dce9e1e70
> vnc_job_add_rect VNC add rect state=0x556dce1c1b20 job=0x556dce217810 offset=0,0 size=1360x13
> vnc_job_clamp_rect VNC job clamp rect state=0x7faa42de53a0 job=0x556dce217810 offset=0,0 size=1360x13
> vnc_job_clamped_rect VNC job clamp rect state=0x7faa42de53a0 job=0x556dce217810 offset=0,0 size=1360x13
> vnc_job_nrects VNC job state=0x7faa42de53a0 job=0x556dce217810 nrects=1
> vnc_client_unthrottle_forced VNC client unthrottle forced offset state=0x556dce1c1b20 ioc=0x556dce9e1e70
> 
> I'm currently struggling to follow where the width parameter is taken from.

The QEMU VNC code has logic which rounds up display sizes to a multiple
of 16:

  static int vnc_width(VncDisplay *vd)
  {
      return MIN(VNC_MAX_WIDTH, ROUND_UP(surface_width(vd->ds),
                                         VNC_DIRTY_PIXELS_PER_BIT));
  }

Separately, it also tracks the "true" width, but untangling which it
uses where & the implications is hard to do. ie i'm not going to try
to explain it further, as I don't know what's going on without spending
some hours to trace through it all :-)

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: Corrupt VNC display for 1366x768

[Simon Rowe]

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

On 03/10/2024, 14:05, "Daniel P. Berrangé" <berrange@redhat.com> wrote:

> The QEMU VNC code has logic which rounds up display sizes to a multiple
> of 16:
>
>  static int vnc_width(VncDisplay *vd)
>  {
>      return MIN(VNC_MAX_WIDTH, ROUND_UP(surface_width(vd->ds),
>                                         VNC_DIRTY_PIXELS_PER_BIT));
>  }
>
> Separately, it also tracks the "true" width, but untangling which it
> uses where & the implications is hard to do. ie i'm not going to try
> to explain it further, as I don't know what's going on without spending
> some hours to trace through it all :-)

On a hunch I revisited Frediano's patch (mentioned in the original post). I had focused on the change to the VNC functions but he also had a small change to the masking of the display width in the VGA code. I've applied a similar change and … I now get a proper display!

Patch is simply

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 892fedc8dc..ea659e2812 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -581,14 +581,14 @@ static void vbe_fixup_regs(VGACommonState *s)
     }

     /* check width */
-    r[VBE_DISPI_INDEX_XRES] &= ~7u;
+    r[VBE_DISPI_INDEX_XRES] &= ~1u;
     if (r[VBE_DISPI_INDEX_XRES] == 0) {
         r[VBE_DISPI_INDEX_XRES] = 8;
     }
     if (r[VBE_DISPI_INDEX_XRES] > VBE_DISPI_MAX_XRES) {
         r[VBE_DISPI_INDEX_XRES] = VBE_DISPI_MAX_XRES;
     }
-    r[VBE_DISPI_INDEX_VIRT_WIDTH] &= ~7u;
+    r[VBE_DISPI_INDEX_VIRT_WIDTH] &= ~1u;
     if (r[VBE_DISPI_INDEX_VIRT_WIDTH] > VBE_DISPI_MAX_XRES) {
         r[VBE_DISPI_INDEX_VIRT_WIDTH] = VBE_DISPI_MAX_XRES;
     }

I don't know how functional correct this is.
Regards
Simon

[-- Attachment #2: Type: text/html, Size: 6159 bytes --]