[PATCH v2 0/3] UI fixups

[PATCH v2 0/3] UI fixups

[Alexander Orzechowski]

Some UI fixups including one revert.

I have some things that I want to have a discussion about. Fullscreen mode
will always distort the virtual console. Is this behavior we really want? If we
get rid of this, we can combine scale_x and scale_y into a single variable
and simplify things a little bit.

Also just from looking around a little bit in ui/gtk-egl.c the code
seems super broken regarding the virtual console rendering in the correct place.
I'm unable to test those code paths so I haven't touched it for now.

Changes in v2:
 - Dropped patch 2, I couldn't figure out what to colour the bike shed
 - Fixed incorrect rendering when gtk/gl is using scanout_mode

Alexander Orzechowski (3):
  ui: Use allocated size instead of window size
  ui: Revert: "fix incorrect pointer position on highdpi with gtk"
  ui: Fix gtk/gl when the scaled virtual console does not fit the window

 ui/gtk-gl-area.c | 34 +++++++++++++++++++++++++++++-----
 ui/gtk.c         | 30 +++++++++++-------------------
 2 files changed, 40 insertions(+), 24 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] ui: Use allocated size instead of window size

[Alexander Orzechowski]

In these cases, we only care about the size of the virtual console
itself. Previously, these calculations were made using the size of
the entire window, which would include the size of the virtual console
plus all the ui elements around it.

Signed-off-by: Alexander Orzechowski <orzechowski.alexander@gmail.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/gtk.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 428f02f2df..824334ff3d 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -340,8 +340,8 @@ static void gd_update_full_redraw(VirtualConsole *vc)
 {
     GtkWidget *area = vc->gfx.drawing_area;
     int ww, wh;
-    ww = gdk_window_get_width(gtk_widget_get_window(area));
-    wh = gdk_window_get_height(gtk_widget_get_window(area));
+    ww = gtk_widget_get_allocated_width(vc->gfx.drawing_area);
+    wh = gtk_widget_get_allocated_height(vc->gfx.drawing_area);
 #if defined(CONFIG_OPENGL)
     if (vc->gfx.gls && gtk_use_gl_area) {
         gtk_gl_area_queue_render(GTK_GL_AREA(vc->gfx.drawing_area));
@@ -387,7 +387,6 @@ static void gd_update(DisplayChangeListener *dcl,
                       int x, int y, int w, int h)
 {
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
-    GdkWindow *win;
     int x1, x2, y1, y2;
     int mx, my;
     int fbw, fbh;
@@ -414,12 +413,8 @@ static void gd_update(DisplayChangeListener *dcl,
     fbw = surface_width(vc->gfx.ds) * vc->gfx.scale_x;
     fbh = surface_height(vc->gfx.ds) * vc->gfx.scale_y;
 
-    win = gtk_widget_get_window(vc->gfx.drawing_area);
-    if (!win) {
-        return;
-    }
-    ww = gdk_window_get_width(win);
-    wh = gdk_window_get_height(win);
+    ww = gtk_widget_get_allocated_width(vc->gfx.drawing_area);
+    wh = gtk_widget_get_allocated_height(vc->gfx.drawing_area);
 
     mx = my = 0;
     if (ww > fbw) {
@@ -788,8 +783,8 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
     fbw = surface_width(vc->gfx.ds);
     fbh = surface_height(vc->gfx.ds);
 
-    ww = gdk_window_get_width(gtk_widget_get_window(widget));
-    wh = gdk_window_get_height(gtk_widget_get_window(widget));
+    ww = gtk_widget_get_allocated_width(vc->gfx.drawing_area);
+    wh = gtk_widget_get_allocated_height(vc->gfx.drawing_area); 
 
     if (s->full_screen) {
         vc->gfx.scale_x = (double)ww / fbw;
@@ -838,7 +833,6 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
 {
     VirtualConsole *vc = opaque;
     GtkDisplayState *s = vc->s;
-    GdkWindow *window;
     int x, y;
     int mx, my;
     int fbh, fbw;
@@ -851,10 +845,10 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
     fbw = surface_width(vc->gfx.ds) * vc->gfx.scale_x;
     fbh = surface_height(vc->gfx.ds) * vc->gfx.scale_y;
 
-    window = gtk_widget_get_window(vc->gfx.drawing_area);
-    ww = gdk_window_get_width(window);
-    wh = gdk_window_get_height(window);
-    ws = gdk_window_get_scale_factor(window);
+    ww = gtk_widget_get_allocated_width(vc->gfx.drawing_area);
+    wh = gtk_widget_get_allocated_height(vc->gfx.drawing_area);
+    ws = gdk_window_get_scale_factor(
+            gtk_widget_get_window(vc->gfx.drawing_area));
 
     mx = my = 0;
     if (ww > fbw) {
-- 
2.34.1

[PATCH v2 2/3] ui: Revert: "fix incorrect pointer position on highdpi with gtk"

[Alexander Orzechowski]

This reverts commit f14aab420c58b57e07189d6d9e6d3fbfab4761a6.

This commit was originally tested on gtk/gl which corrected behavior
there. Turns out, the OpenGL texture representing the virtual console
was being rendered in the incorrect place and not that the cursor
was incorrectly being handled.

Signed-off-by: Alexander Orzechowski <orzechowski.alexander@gmail.com>
---
 ui/gtk.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 824334ff3d..b7f296fac7 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -836,7 +836,7 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
     int x, y;
     int mx, my;
     int fbh, fbw;
-    int ww, wh, ws;
+    int ww, wh;
 
     if (!vc->gfx.ds) {
         return TRUE;
@@ -847,8 +847,6 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
 
     ww = gtk_widget_get_allocated_width(vc->gfx.drawing_area);
     wh = gtk_widget_get_allocated_height(vc->gfx.drawing_area);
-    ws = gdk_window_get_scale_factor(
-            gtk_widget_get_window(vc->gfx.drawing_area));
 
     mx = my = 0;
     if (ww > fbw) {
@@ -858,8 +856,8 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
         my = (wh - fbh) / 2;
     }
 
-    x = (motion->x - mx) / vc->gfx.scale_x * ws;
-    y = (motion->y - my) / vc->gfx.scale_y * ws;
+    x = (motion->x - mx) / vc->gfx.scale_x;
+    y = (motion->y - my) / vc->gfx.scale_y;
 
     if (qemu_input_is_absolute()) {
         if (x < 0 || y < 0 ||
-- 
2.34.1

[PATCH v2 3/3] ui: Fix gtk/gl when the scaled virtual console does not fit the window

[Alexander Orzechowski]

gtk/gl was incorrectly always rendering as if the 'Zoom to Fit' was
always checked even if it wasn't. This is now using logic closer
to what is being used for the existing cairo code paths.

Signed-off-by: Alexander Orzechowski <orzechowski.alexander@gmail.com>
---
 ui/gtk-gl-area.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 01e4e74ee3..f4f2dac882 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -41,16 +41,40 @@ void gd_gl_area_draw(VirtualConsole *vc)
 #ifdef CONFIG_GBM
     QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
 #endif
+    GtkDisplayState *s = vc->s;
     int ww, wh, ws, y1, y2;
+    int mx, my;
+    int fbh, fbw;
 
     if (!vc->gfx.gls) {
         return;
     }
 
     gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
+
+    fbw = surface_width(vc->gfx.ds);
+    fbh = surface_height(vc->gfx.ds);
+
     ws = gdk_window_get_scale_factor(gtk_widget_get_window(vc->gfx.drawing_area));
-    ww = gtk_widget_get_allocated_width(vc->gfx.drawing_area) * ws;
-    wh = gtk_widget_get_allocated_height(vc->gfx.drawing_area) * ws;
+    ww = gtk_widget_get_allocated_width(vc->gfx.drawing_area);
+    wh = gtk_widget_get_allocated_height(vc->gfx.drawing_area);
+
+    if (s->full_screen) {
+        vc->gfx.scale_x = (double)ww / fbw;
+        vc->gfx.scale_y = (double)wh / fbh;
+    } else if (s->free_scale) {
+        double sx, sy;
+
+        sx = (double)ww / fbw;
+        sy = (double)wh / fbh;
+
+        vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
+    }
+
+    fbw *= vc->gfx.scale_x * ws;
+    fbh *= vc->gfx.scale_y * ws;
+    mx = (ww * ws - fbw) / 2;
+    my = (wh * ws - fbh) / 2;
 
     if (vc->gfx.scanout_mode) {
         if (!vc->gfx.guest_fb.framebuffer) {
@@ -70,11 +94,11 @@ void gd_gl_area_draw(VirtualConsole *vc)
         glBindFramebuffer(GL_READ_FRAMEBUFFER, vc->gfx.guest_fb.framebuffer);
         /* GtkGLArea sets GL_DRAW_FRAMEBUFFER for us */
 
-        glViewport(0, 0, ww, wh);
+        glViewport(mx, my, fbw, fbh);
         y1 = vc->gfx.y0_top ? 0 : vc->gfx.h;
         y2 = vc->gfx.y0_top ? vc->gfx.h : 0;
         glBlitFramebuffer(0, y1, vc->gfx.w, y2,
-                          0, 0, ww, wh,
+                          mx, my, fbw + mx, fbh + my,
                           GL_COLOR_BUFFER_BIT, GL_NEAREST);
 #ifdef CONFIG_GBM
         if (dmabuf) {
@@ -98,7 +122,7 @@ void gd_gl_area_draw(VirtualConsole *vc)
         }
         gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
 
-        surface_gl_setup_viewport(vc->gfx.gls, vc->gfx.ds, ww, wh);
+        glViewport(mx, my, fbw, fbh);
         surface_gl_render_texture(vc->gfx.gls, vc->gfx.ds);
     }
 
-- 
2.34.1

Re: [PATCH v2 3/3] ui: Fix gtk/gl when the scaled virtual console does not fit the window

[Marc-André Lureau]

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

Hi

On Wed, Dec 22, 2021 at 4:11 AM Alexander Orzechowski <
orzechowski.alexander@gmail.com> wrote:

> gtk/gl was incorrectly always rendering as if the 'Zoom to Fit' was
> always checked even if it wasn't. This is now using logic closer
> to what is being used for the existing cairo code paths.
>
> Signed-off-by: Alexander Orzechowski <orzechowski.alexander@gmail.com>
>

This is definitely an improvement for me (on hidpi), but I have some
questions and I don't have much interest in working on qemu GTK3 code.

(fwiw, I hope the future will be around the gtk4 widget I work on at
https://gitlab.com/marcandre.lureau/qemu-display/)

The 2nd patch should come after this one imho, to avoid intermediary extra
regressions.

---
>  ui/gtk-gl-area.c | 34 +++++++++++++++++++++++++++++-----
>  1 file changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> index 01e4e74ee3..f4f2dac882 100644
> --- a/ui/gtk-gl-area.c
> +++ b/ui/gtk-gl-area.c
> @@ -41,16 +41,40 @@ void gd_gl_area_draw(VirtualConsole *vc)
>  #ifdef CONFIG_GBM
>      QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
>  #endif
> +    GtkDisplayState *s = vc->s;
>      int ww, wh, ws, y1, y2;
> +    int mx, my;
> +    int fbh, fbw;
>
>      if (!vc->gfx.gls) {
>          return;
>      }
>
>      gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
> +
> +    fbw = surface_width(vc->gfx.ds);
> +    fbh = surface_height(vc->gfx.ds);
> +
>      ws =
> gdk_window_get_scale_factor(gtk_widget_get_window(vc->gfx.drawing_area));
> -    ww = gtk_widget_get_allocated_width(vc->gfx.drawing_area) * ws;
> -    wh = gtk_widget_get_allocated_height(vc->gfx.drawing_area) * ws;
> +    ww = gtk_widget_get_allocated_width(vc->gfx.drawing_area);
> +    wh = gtk_widget_get_allocated_height(vc->gfx.drawing_area);
> +
> +    if (s->full_screen) {
> +        vc->gfx.scale_x = (double)ww / fbw;
> +        vc->gfx.scale_y = (double)wh / fbh;
>

Why not keep aspect-ratio in full-screen?

(and hopefully the pointer follows the same size logic, as well as the
2d/cairo & gtk-x11/egl code paths..)

+    } else if (s->free_scale) {
> +        double sx, sy;
> +
> +        sx = (double)ww / fbw;
> +        sy = (double)wh / fbh;
> +
> +        vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
> +    }
> +
> +    fbw *= vc->gfx.scale_x * ws;
> +    fbh *= vc->gfx.scale_y * ws;
> +    mx = (ww * ws - fbw) / 2;
> +    my = (wh * ws - fbh) / 2;
>
>      if (vc->gfx.scanout_mode) {
>          if (!vc->gfx.guest_fb.framebuffer) {
> @@ -70,11 +94,11 @@ void gd_gl_area_draw(VirtualConsole *vc)
>          glBindFramebuffer(GL_READ_FRAMEBUFFER,
> vc->gfx.guest_fb.framebuffer);
>          /* GtkGLArea sets GL_DRAW_FRAMEBUFFER for us */
>
> -        glViewport(0, 0, ww, wh);
> +        glViewport(mx, my, fbw, fbh);
>          y1 = vc->gfx.y0_top ? 0 : vc->gfx.h;
>          y2 = vc->gfx.y0_top ? vc->gfx.h : 0;
>          glBlitFramebuffer(0, y1, vc->gfx.w, y2,
> -                          0, 0, ww, wh,
> +                          mx, my, fbw + mx, fbh + my,
>                            GL_COLOR_BUFFER_BIT, GL_NEAREST);
>  #ifdef CONFIG_GBM
>          if (dmabuf) {
> @@ -98,7 +122,7 @@ void gd_gl_area_draw(VirtualConsole *vc)
>          }
>          gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
>
> -        surface_gl_setup_viewport(vc->gfx.gls, vc->gfx.ds, ww, wh);
> +        glViewport(mx, my, fbw, fbh);
>

 We should be able to reuse and share the surface_gl_setup_viewport()
logic, no?

-- 
Marc-André Lureau

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