[Spice-devel] [PATCH v2 spice-gtk] Adjust to window scaling

[Snir Sheriber]

On 3/18/19 12:27 PM, Frediano Ziglio wrote:
>> When GDK_SCALE is != 1 and egl is used, the image presented does not
>> fit to the window (scale of 2 is often used with hidpi monitors).
>> Usually this is not a problem since all components are adjusted by
>> gdk/gtk but with egl, pixel-based data is not being scaled. In this
>> case window's scale value can be used in order to determine whether
>> to use a pixel resource with higher resolution data.
>>
>> In order to reproduce the problem set spice with virgl/Intel-vGPU
>> and run spice-gtk with GDK_SCALE=2
> I still didn't try. So from what I understand the sizes and coordinated
> from GTK when GDK_SCALE == 2 (for instance) are half of the pixel sizes,
> right? So for instance width == 1024 means 2048 pixels.


yes, gtk/gdk related sizes will be half of the actual pixels size


>
>> ---
>> Changes from v1:
>> -commit msg
>> -replace var naming (ws with win_scale)
>>
>>
>> This patch is kind of RFC, it fixes the issue, but it's a bit hacky
>> and specific. I didn't come across other scale issues but it is likely
>> that more of these exist and better and generic fix is needed.
>>
>> ---
>>   src/spice-widget-egl.c  | 15 +++++++++++++--
>>   src/spice-widget-priv.h |  1 +
>>   2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c
>> index 43fccd7..600c87a 100644
>> --- a/src/spice-widget-egl.c
>> +++ b/src/spice-widget-egl.c
>> @@ -326,6 +326,8 @@ static gboolean spice_widget_init_egl_win(SpiceDisplay
>> *display, GdkWindow *win,
>>       if (d->egl.surface)
>>           return TRUE;
>>   
>> +    d->egl.scale = gdk_window_get_scale_factor(win);
>> +
>>   #ifdef GDK_WINDOWING_X11
>>       if (GDK_IS_X11_WINDOW(win)) {
>>           native = (EGLNativeWindowType)GDK_WINDOW_XID(win);
> This factor is only read on initialization, what would happen changing
> resolution while the client is open or attaching a new HiDPI monitor?
>
> Can't we keep the "win" and avoid caching it? It should not take long
> to read the factor.


Yes, possibly would be better to read it every time. I'll try.


>
>> @@ -431,15 +433,17 @@ void spice_egl_resize_display(SpiceDisplay *display,
>> int w, int h)
>>   {
>>       SpiceDisplayPrivate *d = display->priv;
>>       int prog;
>> +    gint win_scale;
>>   
>>       if (!gl_make_current(display, NULL))
>>           return;
>>   
>> +    win_scale = d->egl.scale;
>>       glGetIntegerv(GL_CURRENT_PROGRAM, &prog);
>>   
>>       glUseProgram(d->egl.prog);
>> -    apply_ortho(d->egl.mproj, 0, w, 0, h, -1, 1);
>> -    glViewport(0, 0, w, h);
>> +    apply_ortho(d->egl.mproj, 0, w * win_scale , 0, h * win_scale, -1, 1);
>> +    glViewport(0, 0, w * win_scale, h * win_scale);
>>   
> Why not multiplying w and h by d->egl.scale like in other function?


Just because it was longer, but after i changed ws to win_scale it's
indeed not really significant


>
>>       if (d->ready)
>>           spice_egl_update_display(display);
>> @@ -559,6 +563,13 @@ void spice_egl_update_display(SpiceDisplay *display)
>>   
>>       spice_display_get_scaling(display, &s, &x, &y, &w, &h);
>>   
>> +    // Adjust to gdk scale
>> +    s *= d->egl.scale;
>> +    x *= d->egl.scale;
>> +    y *= d->egl.scale;
>> +    w *= d->egl.scale;
>> +    h *= d->egl.scale;
>> +
>>       glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
>>       glClear(GL_COLOR_BUFFER_BIT);
>>   
>> diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h
>> index 65eb404..8f110ac 100644
>> --- a/src/spice-widget-priv.h
>> +++ b/src/spice-widget-priv.h
>> @@ -149,6 +149,7 @@ struct _SpiceDisplayPrivate {
>>           EGLImageKHR         image;
>>           gboolean            call_draw_done;
>>           SpiceGlScanout      scanout;
>> +        gint                scale;
> Why in egl? It's not a property related to egl.


It is under egl since the egl related sizes are the only ones (if i 
didn't miss anything)
which don't scale automatically so i put it there


>
>>       } egl;
>>   #endif // HAVE_EGL
>>       double scroll_delta_y;
> Frediano