Re: [PATCH 2/4] ui: Remove unnecessary checks

[Alexander Orzechowski]

On 12/21/21 02:40, Marc-André Lureau wrote:

> Hi
>
> On Sun, Dec 19, 2021 at 6:32 AM Alexander Orzechowski 
> <orzechowski.alexander@gmail.com> wrote:
>
>     These conditionals should never be false as scale_x and scale_y should
>     scale the fbw and fbh variables such that the ww and wh variables
>     always
>     have a greater magnitude.
>
>     Signed-off-by: Alexander Orzechowski <orzechowski.alexander@gmail.com>
>
>
> I don't understand how you reached that conclusion.
>
> scale_x/scale_y can have various values, from 0.25 manually, or pretty 
> much anything in freescale.
>
> Just adding a breakpoint/debug there and you can see they can be false.
>
>     ---
>      ui/gtk.c | 27 ++++++---------------------
>      1 file changed, 6 insertions(+), 21 deletions(-)
>
>     diff --git a/ui/gtk.c b/ui/gtk.c
>     index 824334ff3d..f2d74b253d 100644
>     --- a/ui/gtk.c
>     +++ b/ui/gtk.c
>     @@ -416,13 +416,8 @@ static void gd_update(DisplayChangeListener *dcl,
>          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) {
>     -        mx = (ww - fbw) / 2;
>     -    }
>     -    if (wh > fbh) {
>     -        my = (wh - fbh) / 2;
>     -    }
>     +    mx = (ww - fbw) / 2;
>     +    my = (wh - fbh) / 2;
>
>          gtk_widget_queue_draw_area(vc->gfx.drawing_area,
>                                     mx + x1, my + y1, (x2 - x1), (y2 -
>     y1));
>     @@ -801,13 +796,8 @@ static gboolean gd_draw_event(GtkWidget
>     *widget, cairo_t *cr, void *opaque)
>          fbw *= vc->gfx.scale_x;
>          fbh *= vc->gfx.scale_y;
>
>     -    mx = my = 0;
>     -    if (ww > fbw) {
>     -        mx = (ww - fbw) / 2;
>     -    }
>     -    if (wh > fbh) {
>     -        my = (wh - fbh) / 2;
>     -    }
>     +    mx = (ww - fbw) / 2;
>     +    my = (wh - fbh) / 2;
>
>          cairo_rectangle(cr, 0, 0, ww, wh);
>
>     @@ -850,13 +840,8 @@ static gboolean gd_motion_event(GtkWidget
>     *widget, GdkEventMotion *motion,
>          ws = gdk_window_get_scale_factor(
>      gtk_widget_get_window(vc->gfx.drawing_area));
>
>     -    mx = my = 0;
>     -    if (ww > fbw) {
>     -        mx = (ww - fbw) / 2;
>     -    }
>     -    if (wh > fbh) {
>     -        my = (wh - fbh) / 2;
>     -    }
>     +    mx = (ww - fbw) / 2;
>     +    my = (wh - fbh) / 2;
>
>          x = (motion->x - mx) / vc->gfx.scale_x * ws;
>          y = (motion->y - my) / vc->gfx.scale_y * ws;
>     -- 
>     2.34.1
>
>
>
>
> --
> Marc-André Lureau

Thanks for the thorough review. I didn't realize you could set the scale 
manually, but

it was under my impression that qemu would set the GDK_HINT_MIN_SIZE 
property[1]

to try to disallow the user from resizing the window any smaller than 
the size of the

virtual console. Qemu provides no mechanism to change the translation of 
the virtual

console within the window in this case where the window is smaller than 
the virtual

console would normally allow, I consider this invalid state. The mx and 
my variables

are only used to translate the virtual console within its render 
surface. Thus, if qemu

were to enter this "invalid" state as I've called it, the view would 
show the middle of

the virtual console and obscure the edges. Without this patch it would 
show the top

left, obscuring the bottom right. I am happy to drop this patch for v2. 
Please let me

know your thoughts.


[1]: See ui/gtk.c line 279 with the patches applied.

--

Alexander Orzechowski