On Tue, 2025-05-13 at 20:01 +0000, Kim, Dongwon wrote:
> > Hi Dongwon,
>
> > On Tue, 2025-05-13 at 01:26 +0000, Kim, Dongwon wrote:
> > > Hi,
> > >
> > > > Subject: [PATCH 3/9] gtk/ui: Introduce helper gd_update_scale
> > > >
> > > > The code snippet updating scale_x/scale_y is general and will be
> > > > used in next
> > > > patch. Make it a function.
> > > >
> > > > Signed-off-by: Weifeng Liu <[email protected]>
> > > > ---
> > > > include/ui/gtk.h | 2 ++
> > > > ui/gtk.c | 30 +++++++++++++++++++-----------
> > > > 2 files changed, 21 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/include/ui/gtk.h b/include/ui/gtk.h index
> > > > aa3d637029..d3944046db 100644
> > > > --- a/include/ui/gtk.h
> > > > +++ b/include/ui/gtk.h
> > > > @@ -224,4 +224,6 @@ int gd_gl_area_make_current(DisplayGLCtx *dgc,
> > > > /* gtk-clipboard.c */
> > > > void gd_clipboard_init(GtkDisplayState *gd);
> > > >
> > > > +void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw,
> > > > int
> > > > +fbh);
> > > > +
> > > > #endif /* UI_GTK_H */
> > > > diff --git a/ui/gtk.c b/ui/gtk.c
> > > > index 8f5bb4b62e..47af49e387 100644
> > > > --- a/ui/gtk.c
> > > > +++ b/ui/gtk.c
> > > > @@ -801,6 +801,24 @@ void
> > > > gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget
> > > > *widget)
> > > > #endif }
> > > >
> > > > +void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw,
> > > > int
> > > > +fbh) {
> > > > + if (!vc) {
> > > > + return;
> > > > + }
> > > > +
> > > > + if (vc->s->full_screen) {
> > > > + vc->gfx.scale_x = (double)ww / fbw;
> > > > + vc->gfx.scale_y = (double)wh / fbh;
> > > > + } else if (vc->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);
> > >
> > > I assume you are trying to keep the w/h ratio same here in case free-
> > > scale == true.
> > > Why would we do that? We can easily stretch the host window to any
> > > direction then the scale-x and scale-y
> > > could be different any time.
> > >
> > Currently, the code doesn't clarify how we should handle aspect ratios.
> > However, I noticed that in the gd_draw_event function, when free-
> > scale=true, it preserves a fixed aspect ratio. I believe this is a
> > reasonable approach (in my humble opinion, it's unlikely that people
> > want to see distorted images), so I've decided to retain this behavior
> > for now and align other parts to follow the same logic, ensuring a
> > consistent experience for users.
>
> Yeah, I didn't realize it does keep the ratio in case of gl=off in the
> original code.
> The reason I brought up this is because as you may Have noticed, scale_x and
> scale_y
> have been individually configured based on width and height of the current
> window
> and guest surface in gtk-egl and gtk-gl-area, which is being changed with
> your patches.
> I am not sure which one is the best either but this would definitely require
> some discussion.
>
IMO, neither preserving nor ignoring the aspect ratio is objectively
“better” — both have its own supporters. I suggest we start by defining
a reasonable default and making all implementations conform to it (even
if it means changing the behaviors of a few backends). Once that’s in
place, I’ll add an option like "keep-aspect" or similar (per my
discussion with Balaton in the other thread) so users can select the
behavior they prefer. Does that sound acceptable?
> > Best regards,
> > Weifeng
>
> > > > + }
> > > > +}
> > > > /**
> > > > * DOC: Coordinate handling.
> > > > *
> > > > @@ -908,17 +926,7 @@ static gboolean gd_draw_event(GtkWidget
> > > > *widget,
> > > > cairo_t *cr, void *opaque)
> > > > ww_widget =
> > > > gdk_window_get_width(gtk_widget_get_window(widget));
> > > > wh_widget =
> > > > gdk_window_get_height(gtk_widget_get_window(widget));
> > > >
> > > > - if (s->full_screen) {
> > > > - vc->gfx.scale_x = (double)ww_widget / fbw;
> > > > - vc->gfx.scale_y = (double)wh_widget / fbh;
> > > > - } else if (s->free_scale) {
> > > > - double sx, sy;
> > > > -
> > > > - sx = (double)ww_widget / fbw;
> > > > - sy = (double)wh_widget / fbh;
> > > > -
> > > > - vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
> > > > - }
> > > > + gd_update_scale(vc, ww_widget, wh_widget, fbw, fbh);
> > > >
> > > > ww_surface = fbw * vc->gfx.scale_x;
> > > > wh_surface = fbh * vc->gfx.scale_y;
> > > > --
> > > > 2.49.0
> > > >
