Re: [PATCH] ui/console: fix qemu_console_resize() regression

[Mark Cave-Ayland]

On 25/07/2022 17:35, Mark Cave-Ayland wrote:

> On 25/07/2022 12:58, marcandre.lureau@redhat.com wrote:
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > The display may be corrupted when changing screen colour depth in
> > qemu-system-ppc/MacOS since 7.0.
>
> Is it worth being more specific here? Whilst MacOS with its NDRV driver exhibits the 
> issue, it's really only because MacOS has separate selections for depth and 
> resolution which allows one to be set without updating the other. I did a quick play 
> with the Forth reproducer, and even with current git master the issue goes away if 
> you also change the width/height at the same time as the depth.
>
> > Do not short-cut qemu_console_resize() if the surface is backed by vga
> > vram. When the scanout isn't set, or it is already allocated, or opengl,
> > and the size is fitting, we still avoid the reallocation & replace path.
> >
> > Fixes: commit cb8962c1 ("ui: do not create a surface when resizing a GL 
> > scanout")
> >
> > Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   ui/console.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/ui/console.c b/ui/console.c
> > index e139f7115e1f..765892f84f1c 100644
> > --- a/ui/console.c
> > +++ b/ui/console.c
> > @@ -2575,11 +2575,13 @@ static void vc_chr_open(Chardev *chr,
> >   void qemu_console_resize(QemuConsole *s, int width, int height)
> >   {
> > -    DisplaySurface *surface;
> > +    DisplaySurface *surface = qemu_console_surface(s);
> >       assert(s->console_type == GRAPHIC_CONSOLE);
> > -    if (qemu_console_get_width(s, -1) == width &&
> > +    if ((s->scanout.kind != SCANOUT_SURFACE ||
> > +         (surface && surface->flags & QEMU_ALLOCATED_FLAG)) &&
> > +        qemu_console_get_width(s, -1) == width &&
> >           qemu_console_get_height(s, -1) == height) {
> >           return;
> >       }
>
> The criteria listed for the short-cut in the commit message are quite handy, so is it 
> worth adding a comment along the same lines as a reminder? Or is this logic touched 
> so rarely that it isn't worthwhile?
>
> Regardless of the above, thanks for coming up with the patch and I can confirm that 
> it fixes both the Forth reproducer and the changing of the Monitor colour depth in 
> MacOS itself:
>
> Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Hi Marc-André,

Are you planning to submit this as a fix for 7.1? I think it would certainly qualify 
as a bug fix, and would likely affect users other than just qemu-system-ppc/MacOS.


ATB,

Mark.