Hi
On Thu, Aug 4, 2022 at 12:11 PM Mark Cave-Ayland
<mark.cave-ayl...@ilande.co.uk> wrote:
>
> On 25/07/2022 17:35, Mark Cave-Ayland wrote:
>
> > On 25/07/2022 12:58, marcandre.lur...@redhat.com wrote:
> >
> >> From: Marc-André Lureau <marcandre.lur...@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-ayl...@ilande.co.uk>
> >> Signed-off-by: Marc-André Lureau <marcandre.lur...@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?
I don't know how often it will change, but it seems a bit fragile to
me. I can add the commit comment along.
> >
> > 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-ayl...@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.
Gerd, could you review the patch and let me send a MR ? (or do you
have other UI patches queued already and take it?)
