[PATCH] ui/sdl2: Allow high-dpi

Antonio Caggiano posted 1 patch 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230607090434.53682-1-quic._5Facaggian@quicinc.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>
ui/sdl2-gl.c | 4 ++--
ui/sdl2.c    | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
[PATCH] ui/sdl2: Allow high-dpi
Posted by Antonio Caggiano 11 months ago
Add the SDL_WINDOW_ALLOW_HIGHDPI flag when creating a window and get the
drawable size instead of the window size when setting up the framebuffer
and the viewport.

Signed-off-by: Antonio Caggiano <quic_acaggian@quicinc.com>
---
 ui/sdl2-gl.c | 4 ++--
 ui/sdl2.c    | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
index bbfa70eac3..251b7d56d6 100644
--- a/ui/sdl2-gl.c
+++ b/ui/sdl2-gl.c
@@ -53,7 +53,7 @@ static void sdl2_gl_render_surface(struct sdl2_console *scon)
     SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
     sdl2_set_scanout_mode(scon, false);
 
-    SDL_GetWindowSize(scon->real_window, &ww, &wh);
+    SDL_GL_GetDrawableSize(scon->real_window, &ww, &wh);
     surface_gl_setup_viewport(scon->gls, scon->surface, ww, wh);
 
     surface_gl_render_texture(scon->gls, scon->surface);
@@ -239,7 +239,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
 
     SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
 
-    SDL_GetWindowSize(scon->real_window, &ww, &wh);
+    SDL_GL_GetDrawableSize(scon->real_window, &ww, &wh);
     egl_fb_setup_default(&scon->win_fb, ww, wh);
     egl_fb_blit(&scon->win_fb, &scon->guest_fb, !scon->y0_top);
 
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9d703200bf..c9c83815ca 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -95,7 +95,7 @@ void sdl2_window_create(struct sdl2_console *scon)
     }
 #ifdef CONFIG_OPENGL
     if (scon->opengl) {
-        flags |= SDL_WINDOW_OPENGL;
+        flags |= SDL_WINDOW_OPENGL | SDL_WINDOW_ALLOW_HIGHDPI;
     }
 #endif
 
-- 
2.40.0
Re: [PATCH] ui/sdl2: Allow high-dpi
Posted by Marc-André Lureau 11 months ago
Hi Antonio

On Wed, Jun 7, 2023 at 1:05 PM Antonio Caggiano <quic_acaggian@quicinc.com>
wrote:

> Add the SDL_WINDOW_ALLOW_HIGHDPI flag when creating a window and get the
> drawable size instead of the window size when setting up the framebuffer
> and the viewport.
>
>
What does this actually change?
What about non-gl display, Mouse motion, and display resize?

thanks

Signed-off-by: Antonio Caggiano <quic_acaggian@quicinc.com>
> ---
>  ui/sdl2-gl.c | 4 ++--
>  ui/sdl2.c    | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
> index bbfa70eac3..251b7d56d6 100644
> --- a/ui/sdl2-gl.c
> +++ b/ui/sdl2-gl.c
> @@ -53,7 +53,7 @@ static void sdl2_gl_render_surface(struct sdl2_console
> *scon)
>      SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
>      sdl2_set_scanout_mode(scon, false);
>
> -    SDL_GetWindowSize(scon->real_window, &ww, &wh);
> +    SDL_GL_GetDrawableSize(scon->real_window, &ww, &wh);
>      surface_gl_setup_viewport(scon->gls, scon->surface, ww, wh);
>
>      surface_gl_render_texture(scon->gls, scon->surface);
> @@ -239,7 +239,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
>
>      SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
>
> -    SDL_GetWindowSize(scon->real_window, &ww, &wh);
> +    SDL_GL_GetDrawableSize(scon->real_window, &ww, &wh);
>      egl_fb_setup_default(&scon->win_fb, ww, wh);
>      egl_fb_blit(&scon->win_fb, &scon->guest_fb, !scon->y0_top);
>
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index 9d703200bf..c9c83815ca 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -95,7 +95,7 @@ void sdl2_window_create(struct sdl2_console *scon)
>      }
>  #ifdef CONFIG_OPENGL
>      if (scon->opengl) {
> -        flags |= SDL_WINDOW_OPENGL;
> +        flags |= SDL_WINDOW_OPENGL | SDL_WINDOW_ALLOW_HIGHDPI;
>      }
>  #endif
>
> --
> 2.40.0
>
>
Re: [PATCH] ui/sdl2: Allow high-dpi
Posted by Antonio Caggiano 11 months ago
Hi Marc-André,

On 07/06/2023 12:21, Marc-André Lureau wrote:
> Hi Antonio
> 
> On Wed, Jun 7, 2023 at 1:05 PM Antonio Caggiano <quic_acaggian@quicinc.com>
> wrote:
> 
>> Add the SDL_WINDOW_ALLOW_HIGHDPI flag when creating a window and get the
>> drawable size instead of the window size when setting up the framebuffer
>> and the viewport.
>>
>>
> What does this actually change?

The sdl2-gl backend does not work properly on high-DPI windows. [0]
My understanding is that by allowing high-DPI, SDL creates a framebuffer 
with the right size in pixels which might be different than the size of 
the window. [1]
I believe we just need to use the framebuffer size for GL viewport and 
texture, by retrieving it with SDL_GL_GetDrawableSize.

> What about non-gl display, Mouse motion, and display resize?

 From what I can see by testing the SDL2 renderer, it seems to handle 
all of this transparently, so there is nothing we need to do in sdl2-2d.

Same for mouse motion, which I believe is using window "screen coordinates".

Display resize is a bit weird for a bunch of frames, but then it fixes 
itself.

[0] 
https://user-images.githubusercontent.com/6058008/244368628-329241dc-267d-452f-b8ce-816ae1623131.png
[1] https://wiki.libsdl.org/SDL2/SDL_GetWindowSize#remarks

> 
> thanks
> 
> Signed-off-by: Antonio Caggiano <quic_acaggian@quicinc.com>
>> ---
>>   ui/sdl2-gl.c | 4 ++--
>>   ui/sdl2.c    | 2 +-
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
>> index bbfa70eac3..251b7d56d6 100644
>> --- a/ui/sdl2-gl.c
>> +++ b/ui/sdl2-gl.c
>> @@ -53,7 +53,7 @@ static void sdl2_gl_render_surface(struct sdl2_console
>> *scon)
>>       SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
>>       sdl2_set_scanout_mode(scon, false);
>>
>> -    SDL_GetWindowSize(scon->real_window, &ww, &wh);
>> +    SDL_GL_GetDrawableSize(scon->real_window, &ww, &wh);
>>       surface_gl_setup_viewport(scon->gls, scon->surface, ww, wh);
>>
>>       surface_gl_render_texture(scon->gls, scon->surface);
>> @@ -239,7 +239,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
>>
>>       SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
>>
>> -    SDL_GetWindowSize(scon->real_window, &ww, &wh);
>> +    SDL_GL_GetDrawableSize(scon->real_window, &ww, &wh);
>>       egl_fb_setup_default(&scon->win_fb, ww, wh);
>>       egl_fb_blit(&scon->win_fb, &scon->guest_fb, !scon->y0_top);
>>
>> diff --git a/ui/sdl2.c b/ui/sdl2.c
>> index 9d703200bf..c9c83815ca 100644
>> --- a/ui/sdl2.c
>> +++ b/ui/sdl2.c
>> @@ -95,7 +95,7 @@ void sdl2_window_create(struct sdl2_console *scon)
>>       }
>>   #ifdef CONFIG_OPENGL
>>       if (scon->opengl) {
>> -        flags |= SDL_WINDOW_OPENGL;
>> +        flags |= SDL_WINDOW_OPENGL | SDL_WINDOW_ALLOW_HIGHDPI;
>>       }
>>   #endif
>>
>> --
>> 2.40.0
>>
>>
> 

Cheers,
Antonio

Re: [PATCH] ui/sdl2: Allow high-dpi
Posted by Marc-André Lureau 10 months, 3 weeks ago
Hi Antonio

On Thu, Jun 8, 2023 at 1:33 PM Antonio Caggiano <quic_acaggian@quicinc.com>
wrote:

> Hi Marc-André,
>
> On 07/06/2023 12:21, Marc-André Lureau wrote:
> > Hi Antonio
> >
> > On Wed, Jun 7, 2023 at 1:05 PM Antonio Caggiano <
> quic_acaggian@quicinc.com>
> > wrote:
> >
> >> Add the SDL_WINDOW_ALLOW_HIGHDPI flag when creating a window and get the
> >> drawable size instead of the window size when setting up the framebuffer
> >> and the viewport.
> >>
> >>
> > What does this actually change?
>
> The sdl2-gl backend does not work properly on high-DPI windows. [0]
> My understanding is that by allowing high-DPI, SDL creates a framebuffer
> with the right size in pixels which might be different than the size of
> the window. [1]
> I believe we just need to use the framebuffer size for GL viewport and
> texture, by retrieving it with SDL_GL_GetDrawableSize.
>
> > What about non-gl display, Mouse motion, and display resize?
>
>  From what I can see by testing the SDL2 renderer, it seems to handle
> all of this transparently, so there is nothing we need to do in sdl2-2d.
>
> Same for mouse motion, which I believe is using window "screen
> coordinates".
>
> Display resize is a bit weird for a bunch of frames, but then it fixes
> itself.
>
> [0]
>
> https://user-images.githubusercontent.com/6058008/244368628-329241dc-267d-452f-b8ce-816ae1623131.png


I am using SDL2-2.26.3-1.fc38.x86_64 on wayland, and it doesn't have this
issue.

Your patch doesn't seem to change the behaviour.

Maybe there is something to investigate at SDL level instead.

btw, "allow hi-dpi" isn't a good description imho, since the guest still
doesn't receive real physical hi-dpi details of the client, and remains
scaled-up.


>
> [1] https://wiki.libsdl.org/SDL2/SDL_GetWindowSize#remarks
>
> >
> > thanks
> >
> > Signed-off-by: Antonio Caggiano <quic_acaggian@quicinc.com>
> >> ---
> >>   ui/sdl2-gl.c | 4 ++--
> >>   ui/sdl2.c    | 2 +-
> >>   2 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
> >> index bbfa70eac3..251b7d56d6 100644
> >> --- a/ui/sdl2-gl.c
> >> +++ b/ui/sdl2-gl.c
> >> @@ -53,7 +53,7 @@ static void sdl2_gl_render_surface(struct sdl2_console
> >> *scon)
> >>       SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
> >>       sdl2_set_scanout_mode(scon, false);
> >>
> >> -    SDL_GetWindowSize(scon->real_window, &ww, &wh);
> >> +    SDL_GL_GetDrawableSize(scon->real_window, &ww, &wh);
> >>       surface_gl_setup_viewport(scon->gls, scon->surface, ww, wh);
> >>
> >>       surface_gl_render_texture(scon->gls, scon->surface);
> >> @@ -239,7 +239,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener
> *dcl,
> >>
> >>       SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
> >>
> >> -    SDL_GetWindowSize(scon->real_window, &ww, &wh);
> >> +    SDL_GL_GetDrawableSize(scon->real_window, &ww, &wh);
> >>       egl_fb_setup_default(&scon->win_fb, ww, wh);
> >>       egl_fb_blit(&scon->win_fb, &scon->guest_fb, !scon->y0_top);
> >>
> >> diff --git a/ui/sdl2.c b/ui/sdl2.c
> >> index 9d703200bf..c9c83815ca 100644
> >> --- a/ui/sdl2.c
> >> +++ b/ui/sdl2.c
> >> @@ -95,7 +95,7 @@ void sdl2_window_create(struct sdl2_console *scon)
> >>       }
> >>   #ifdef CONFIG_OPENGL
> >>       if (scon->opengl) {
> >> -        flags |= SDL_WINDOW_OPENGL;
> >> +        flags |= SDL_WINDOW_OPENGL | SDL_WINDOW_ALLOW_HIGHDPI;
> >>       }
> >>   #endif
> >>
> >> --
> >> 2.40.0
> >>
> >>
> >
>
> Cheers,
> Antonio
>
>

-- 
Marc-André Lureau