Re: [Qemu-devel] [PATCH v2 7/7] sdl2: add support for display rendering using opengl.

[Gerd Hoffmann]

  Hi,

> > +void sdl2_gl_switch(DisplayChangeListener *dcl,
> > +                    DisplaySurface *new_surface)
> > +{
> > +    struct sdl2_console *scon = container_of(dcl, struct sdl2_console, 
> > dcl);
> > +    DisplaySurface *old_surface = scon->surface;
> > +
> > +    assert(scon->opengl);
> > +
> > +    SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
> > +    surface_gl_destroy_texture(scon->gls, scon->surface);
> 
> Same question as for v1: Can a surface be in use by multiple DCLs?

Oops, I remember that comment for the last series, forgot to answer ...

Yes.  Old surface is released after notifying all DCLs about the new one
(see dpy_gfx_replace_surface() in console.c), so the old is still valid
at this point.  That way refcounting should not be needed in most cases.

If a DCL needs the old surface image stay around after it returns from
the switch callback (say due to threads still holding pointers to it) it
can grab a reference on the pixman image backing the surface:
pixman_image_ref(surface->image).  The surfaces itself are not reference
counted.

> > +    } else if (old_surface &&
> > +               ((surface_width(old_surface)  != 
> > surface_width(new_surface)) ||
> > +                (surface_height(old_surface) != 
> > surface_height(new_surface)))) {
> 
> And as in v1: If the window is scaled, this will reset the scaling to 
> 100 %, which is fine. However, if the new surface has the same 
> dimensions as the old surface, the window will not be scaled. That would 
> seem strange to me (why is the scaling reset for some surface changes 
> but not for others?).

Yea, there are some corner cases.  But they are not specific to opengl,
they happen in sdl2-2d mode too, so that is something for another patch
series ...

cheers,
  Gerd