SDL: wayland: Fix some incorrect buffer scale calculations

From 9d9721cd4c8d1c4f066310cf3bf501a3ae413555 Mon Sep 17 00:00:00 2001
From: Frank Praznik <[EMAIL REDACTED]>
Date: Thu, 12 Sep 2024 13:36:08 -0400
Subject: [PATCH] wayland: Fix some incorrect buffer scale calculations

Use doubles and apply an offset to account for rounding errors due to Wayland scale increments being in units of 1/120. This fixes the backbuffer size calculations with certain combinations of size/scale values, and future-proofs the Wayland backend, as 32-bit floats become increasingly error-prone with larger dimensions and/or scale factors.

The conversion formula is now point->pixel->point round trip safe as well.
---
 src/video/wayland/SDL_waylandevents.c | 20 ++++-----
 src/video/wayland/SDL_waylandmouse.c  | 20 ++++-----
 src/video/wayland/SDL_waylandvideo.c  |  8 ++--
 src/video/wayland/SDL_waylandvideo.h  |  2 +-
 src/video/wayland/SDL_waylandwindow.c | 60 ++++++++++++---------------
 src/video/wayland/SDL_waylandwindow.h |  6 +--
 6 files changed, 53 insertions(+), 63 deletions(-)

diff --git a/src/video/wayland/SDL_waylandevents.c b/src/video/wayland/SDL_waylandevents.c
index e5b9fd03e3a33..b209116d01627 100644
--- a/src/video/wayland/SDL_waylandevents.c
+++ b/src/video/wayland/SDL_waylandevents.c
@@ -521,8 +521,8 @@ static void pointer_handle_motion(void *data, struct wl_pointer *pointer,
     input->sx_w = sx_w;
     input->sy_w = sy_w;
     if (input->pointer_focus) {
-        float sx = (float)(wl_fixed_to_double(sx_w) * window_data->pointer_scale.x);
-        float sy = (float)(wl_fixed_to_double(sy_w) * window_data->pointer_scale.y);
+        const float sx = (float)(wl_fixed_to_double(sx_w) * window_data->pointer_scale.x);
+        const float sy = (float)(wl_fixed_to_double(sy_w) * window_data->pointer_scale.y);
         SDL_SendMouseMotion(Wayland_GetPointerTimestamp(input, time), window_data->sdlwindow, input->pointer_id, false, sx, sy);
     }
 
@@ -2590,12 +2590,8 @@ static void tablet_tool_handle_motion(void *data, struct zwp_tablet_tool_v2 *too
     SDL_Window *window = sdltool->tool_focus;
     if (window) {
         const SDL_WindowData *windowdata = window->internal;
-        const float sx_f = (float)wl_fixed_to_double(sx_w);
-        const float sy_f = (float)wl_fixed_to_double(sy_w);
-        const float sx = sx_f * windowdata->pointer_scale.x;
-        const float sy = sy_f * windowdata->pointer_scale.y;
-        sdltool->x = sx;
-        sdltool->y = sy;
+        sdltool->x = (float)(wl_fixed_to_double(sx_w) * windowdata->pointer_scale.x);
+        sdltool->y = (float)(wl_fixed_to_double(sy_w) * windowdata->pointer_scale.y);
         sdltool->frame_motion_set = true;
     }
 }
@@ -3178,10 +3174,10 @@ bool Wayland_input_confine_pointer(struct SDL_WaylandInput *input, SDL_Window *w
     } else {
         SDL_Rect scaled_mouse_rect;
 
-        scaled_mouse_rect.x = (int)SDL_floorf((float)window->mouse_rect.x / w->pointer_scale.x);
-        scaled_mouse_rect.y = (int)SDL_floorf((float)window->mouse_rect.y / w->pointer_scale.y);
-        scaled_mouse_rect.w = (int)SDL_ceilf((float)window->mouse_rect.w / w->pointer_scale.x);
-        scaled_mouse_rect.h = (int)SDL_ceilf((float)window->mouse_rect.h / w->pointer_scale.y);
+        scaled_mouse_rect.x = (int)SDL_floor(window->mouse_rect.x / w->pointer_scale.x);
+        scaled_mouse_rect.y = (int)SDL_floor(window->mouse_rect.y / w->pointer_scale.y);
+        scaled_mouse_rect.w = (int)SDL_ceil(window->mouse_rect.w / w->pointer_scale.x);
+        scaled_mouse_rect.h = (int)SDL_ceil(window->mouse_rect.h / w->pointer_scale.y);
 
         confine_rect = wl_compositor_create_region(d->compositor);
         wl_region_add(confine_rect,
diff --git a/src/video/wayland/SDL_waylandmouse.c b/src/video/wayland/SDL_waylandmouse.c
index 39506ef51d458..576e898a9c619 100644
--- a/src/video/wayland/SDL_waylandmouse.c
+++ b/src/video/wayland/SDL_waylandmouse.c
@@ -51,7 +51,7 @@ typedef struct
 
     int dst_width;
     int dst_height;
-    float scale;
+    double scale;
 
     struct wl_list node;
 } Wayland_CachedCustomCursor;
@@ -332,7 +332,7 @@ static void cursor_frame_done(void *data, struct wl_callback *cb, uint32_t time)
     wl_surface_commit(c->surface);
 }
 
-static bool wayland_get_system_cursor(SDL_VideoData *vdata, SDL_CursorData *cdata, float *scale)
+static bool wayland_get_system_cursor(SDL_VideoData *vdata, SDL_CursorData *cdata, double *scale)
 {
     struct wl_cursor_theme *theme = NULL;
     struct wl_cursor *cursor;
@@ -357,9 +357,9 @@ static bool wayland_get_system_cursor(SDL_VideoData *vdata, SDL_CursorData *cdat
     focus = SDL_GetMouse()->focus;
     if (focus) {
         // TODO: Use the fractional scale once GNOME supports viewports on cursor surfaces.
-        *scale = SDL_ceilf(focus->internal->windowed_scale_factor);
+        *scale = SDL_ceil(focus->internal->scale_factor);
     } else {
-        *scale = 1.0f;
+        *scale = 1.0;
     }
 
     size *= (int)*scale;
@@ -435,15 +435,15 @@ static Wayland_CachedCustomCursor *Wayland_GetCachedCustomCursor(SDL_Cursor *cur
     SDL_CursorData *data = cursor->internal;
     Wayland_CachedCustomCursor *cache;
     SDL_Window *focus = SDL_GetMouseFocus();
-    float scale = 1.0f;
+    double scale = 1.0;
 
     if (focus && SDL_SurfaceHasAlternateImages(data->cursor_data.custom.sdl_cursor_surface)) {
-        scale = focus->internal->windowed_scale_factor;
+        scale = focus->internal->scale_factor;
     }
 
     // Only use fractional scale values if viewports are available.
     if (!wd->viewporter) {
-        scale = SDL_ceilf(scale);
+        scale = SDL_ceil(scale);
     }
 
     // Is this cursor already cached at the target scale?
@@ -458,7 +458,7 @@ static Wayland_CachedCustomCursor *Wayland_GetCachedCustomCursor(SDL_Cursor *cur
         return NULL;
     }
 
-    SDL_Surface *surface = SDL_GetSurfaceImage(data->cursor_data.custom.sdl_cursor_surface, scale);
+    SDL_Surface *surface = SDL_GetSurfaceImage(data->cursor_data.custom.sdl_cursor_surface, (float)scale);
     if (!surface) {
         SDL_free(cache);
         return NULL;
@@ -675,7 +675,7 @@ static bool Wayland_ShowCursor(SDL_Cursor *cursor)
     SDL_VideoData *d = vd->internal;
     struct SDL_WaylandInput *input = d->input;
     struct wl_pointer *pointer = d->pointer;
-    float scale = 1.0f;
+    double scale = 1.0;
     int dst_width = 0;
     int dst_height = 0;
 
@@ -728,7 +728,7 @@ static bool Wayland_ShowCursor(SDL_Cursor *cursor)
         }
 
         // TODO: Make the viewport path the default in all cases once GNOME finally supports viewports on cursor surfaces.
-        if (SDL_ceilf(scale) != scale && d->viewporter) {
+        if (SDL_ceil(scale) != scale && d->viewporter) {
             if (!data->viewport) {
                 data->viewport = wp_viewporter_get_viewport(d->viewporter, data->surface);
             }
diff --git a/src/video/wayland/SDL_waylandvideo.c b/src/video/wayland/SDL_waylandvideo.c
index 32cc022e7ff55..b57052ebc6034 100644
--- a/src/video/wayland/SDL_waylandvideo.c
+++ b/src/video/wayland/SDL_waylandvideo.c
@@ -895,7 +895,7 @@ static void display_handle_done(void *data,
             // ...and the compositor scales the logical viewport...
             if (video->viewporter) {
                 // ...and viewports are supported, calculate the true scale of the output.
-                internal->scale_factor = (float)native_mode.w / (float)internal->screen_width;
+                internal->scale_factor = (double)native_mode.w / (double)internal->screen_width;
             } else {
                 // ...otherwise, the 'native' pixel values are a multiple of the logical screen size.
                 internal->pixel_width = internal->screen_width * (int)internal->scale_factor;
@@ -923,7 +923,7 @@ static void display_handle_done(void *data,
     if (!video->scale_to_display_enabled) {
         desktop_mode.w = internal->screen_width;
         desktop_mode.h = internal->screen_height;
-        desktop_mode.pixel_density = internal->scale_factor;
+        desktop_mode.pixel_density = (float)internal->scale_factor;
     } else {
         desktop_mode.w = native_mode.w;
         desktop_mode.h = native_mode.h;
@@ -940,14 +940,14 @@ static void display_handle_done(void *data,
     }
 
     if (video->scale_to_display_enabled) {
-        SDL_SetDisplayContentScale(dpy, internal->scale_factor);
+        SDL_SetDisplayContentScale(dpy, (float)internal->scale_factor);
     }
 
     // Set the desktop display mode.
     SDL_SetDesktopDisplayMode(dpy, &desktop_mode);
 
     // Expose the unscaled, native resolution if the scale is 1.0 or viewports are available...
-    if (internal->scale_factor == 1.0f || video->viewporter) {
+    if (internal->scale_factor == 1.0 || video->viewporter) {
         SDL_AddFullscreenDisplayMode(dpy, &native_mode);
     } else {
         // ...otherwise expose the integer scaled variants of the desktop resolution down to 1.
diff --git a/src/video/wayland/SDL_waylandvideo.h b/src/video/wayland/SDL_waylandvideo.h
index 411670669a642..c9c75d13083be 100644
--- a/src/video/wayland/SDL_waylandvideo.h
+++ b/src/video/wayland/SDL_waylandvideo.h
@@ -105,8 +105,8 @@ struct SDL_DisplayData
     struct wl_output *output;
     struct zxdg_output_v1 *xdg_output;
     char *wl_output_name;
+    double scale_factor;
     uint32_t registry_id;
-    float scale_factor;
     int pixel_width, pixel_height;
     int x, y, screen_width, screen_height, refresh, transform;
     SDL_DisplayOrientation orientation;
diff --git a/src/video/wayland/SDL_waylandwindow.c b/src/video/wayland/SDL_waylandwindow.c
index 5a7e5b91e70e1..70560465a9477 100644
--- a/src/video/wayland/SDL_waylandwindow.c
+++ b/src/video/wayland/SDL_waylandwindow.c
@@ -48,24 +48,19 @@
 #include <libdecor.h>
 #endif
 
-/* These are *NOT* roundtrip safe! */
+// These are point->pixel->point round trip safe; the inverse is not round trip safe due to rounding.
 static int PointToPixel(SDL_Window *window, int point)
 {
-    // Rounds halfway away from zero as per the Wayland fractional scaling protocol spec.
-    return (int)SDL_lroundf((float)point * window->internal->windowed_scale_factor);
+    /* Rounds halfway away from zero as per the Wayland fractional scaling protocol spec.
+     * Wayland scale units are in units of 1/120, so the offset is required to correct for
+     * rounding errors when using certain scale values.
+     */
+    return SDL_max((int)SDL_lround((double)point * window->internal->scale_factor + 1e-6), 1);
 }
 
 static int PixelToPoint(SDL_Window *window, int pixel)
 {
-    return (int)SDL_lroundf((float)pixel / window->internal->windowed_scale_factor);
-}
-
-static bool FloatEqual(float a, float b)
-{
-    const float diff = SDL_fabsf(a - b);
-    const float largest = SDL_max(SDL_fabsf(a), SDL_fabsf(b));
-
-    return diff <= largest * SDL_FLT_EPSILON;
+    return SDL_max((int)SDL_lround((double)pixel / window->internal->scale_factor), 1);
 }
 
 /* According to the Wayland spec:
@@ -372,8 +367,8 @@ static void ConfigureWindowGeometry(SDL_Window *window)
                 data->current.logical_height = window->current_fullscreen_mode.h;
             }
 
-            data->pointer_scale.x = (float)window_width / (float)data->current.logical_width;
-            data->pointer_scale.y = (float)window_height / (float)data->current.logical_height;
+            data->pointer_scale.x = (double)window_width / (double)data->current.logical_width;
+            data->pointer_scale.y = (double)window_height / (double)data->current.logical_height;
         }
     } else {
         window_width = data->requested.logical_width;
@@ -386,7 +381,7 @@ static void ConfigureWindowGeometry(SDL_Window *window)
                 wp_viewport_set_destination(data->viewport, window_width, window_height);
             } else if (window->flags & SDL_WINDOW_HIGH_PIXEL_DENSITY) {
                 // Don't change this if the DPI awareness flag is unset, as an application may have set this manually on a custom or external surface.
-                wl_surface_set_buffer_scale(data->surface, (int32_t)data->windowed_scale_factor);
+                wl_surface_set_buffer_scale(data->surface, (int32_t)data->scale_factor);
             }
 
             // Clamp the physical window size to the system minimum required size.
@@ -394,11 +389,11 @@ static void ConfigureWindowGeometry(SDL_Window *window)
             data->current.logical_height = SDL_max(window_height, data->system_limits.min_height);
 
             if (!data->scale_to_display) {
-                data->pointer_scale.x = 1.0f;
-                data->pointer_scale.y = 1.0f;
+                data->pointer_scale.x = 1.0;
+                data->pointer_scale.y = 1.0;
             } else {
-                data->pointer_scale.x = data->windowed_scale_factor;
-                data->pointer_scale.y = data->windowed_scale_factor;
+                data->pointer_scale.x = data->scale_factor;
+                data->pointer_scale.y = data->scale_factor;
             }
         }
     }
@@ -1346,9 +1341,9 @@ static struct libdecor_frame_interface libdecor_frame_interface = {
 };
 #endif
 
-static void Wayland_HandlePreferredScaleChanged(SDL_WindowData *window_data, float factor)
+static void Wayland_HandlePreferredScaleChanged(SDL_WindowData *window_data, double factor)
 {
-    const float old_factor = window_data->windowed_scale_factor;
+    const double old_factor = window_data->scale_factor;
 
     if (!(window_data->sdlwindow->flags & SDL_WINDOW_HIGH_PIXEL_DENSITY) && !window_data->scale_to_display) {
         // Scale will always be 1, just ignore this
@@ -1357,11 +1352,11 @@ static void Wayland_HandlePreferredScaleChanged(SDL_WindowData *window_data, flo
 
     // Round the scale factor if viewports aren't available.
     if (!window_data->viewport) {
-        factor = SDL_ceilf(factor);
+        factor = SDL_ceil(factor);
     }
 
-    if (!FloatEqual(factor, old_factor)) {
-        window_data->windowed_scale_factor = factor;
+    if (factor != old_factor) {
+        window_data->scale_factor = factor;
 
         if (window_data->scale_to_display) {
             /* If the window is in the floating state with a user/application specified size, calculate the new
@@ -1384,7 +1379,7 @@ static void Wayland_HandlePreferredScaleChanged(SDL_WindowData *window_data, flo
 
 static void Wayland_MaybeUpdateScaleFactor(SDL_WindowData *window)
 {
-    float factor;
+    double factor;
     int i;
 
     /* If the fractional scale protocol is present or the core protocol supports the
@@ -1398,14 +1393,14 @@ static void Wayland_MaybeUpdateScaleFactor(SDL_WindowData *window)
 
     if (window->num_outputs != 0) {
         // Check every display's factor, use the highest
-        factor = 0.0f;
+        factor = 0.0;
         for (i = 0; i < window->num_outputs; i++) {
             SDL_DisplayData *internal = window->outputs[i];
             factor = SDL_max(factor, internal->scale_factor);
         }
     } else {
         // All outputs removed, just fall back.
-        factor = window->windowed_scale_factor;
+        factor = window->scale_factor;
     }
 
     Wayland_HandlePreferredScaleChanged(window, factor);
@@ -1481,7 +1476,7 @@ static void handle_preferred_buffer_scale(void *data, struct wl_surface *wl_surf
      * only listen to this event if the fractional scaling protocol is not present.
      */
     if (!wind->fractional_scale) {
-        Wayland_HandlePreferredScaleChanged(data, (float)factor);
+        Wayland_HandlePreferredScaleChanged(data, (double)factor);
     }
 }
 
@@ -1499,7 +1494,7 @@ static const struct wl_surface_listener surface_listener = {
 
 static void handle_preferred_fractional_scale(void *data, struct wp_fractional_scale_v1 *wp_fractional_scale_v1, uint32_t scale)
 {
-    const float factor = scale / 120.f; // 120 is a magic number defined in the spec as a common denominator
+    const double factor = (double)scale / 120.; // 120 is a magic number defined in the spec as a common denominator
     Wayland_HandlePreferredScaleChanged(data, factor);
 }
 
@@ -2397,16 +2392,15 @@ bool Wayland_CreateWindow(SDL_VideoDevice *_this, SDL_Window *window, SDL_Proper
     data->waylandData = c;
     data->sdlwindow = window;
 
-    data->windowed_scale_factor = 1.0f;
+    data->scale_factor = 1.0;
 
     if (SDL_WINDOW_IS_POPUP(window)) {
         data->scale_to_display = window->parent->internal->scale_to_display;
-        data->windowed_scale_factor = window->parent->internal->windowed_scale_factor;
+        data->scale_factor = window->parent->internal->scale_factor;
         EnsurePopupPositionIsValid(window, &window->x, &window->y);
     } else if ((window->flags & SDL_WINDOW_HIGH_PIXEL_DENSITY) || c->scale_to_display_enabled) {
         for (int i = 0; i < _this->num_displays; i++) {
-            float scale = _this->displays[i]->internal->scale_factor;
-            data->windowed_scale_factor = SDL_max(data->windowed_scale_factor, scale);
+            data->scale_factor = SDL_max(data->scale_factor, _this->displays[i]->internal->scale_factor);
         }
     }
 
diff --git a/src/video/wayland/SDL_waylandwindow.h b/src/video/wayland/SDL_waylandwindow.h
index 344ac583f9bed..125abc59cf73f 100644
--- a/src/video/wayland/SDL_waylandwindow.h
+++ b/src/video/wayland/SDL_waylandwindow.h
@@ -110,14 +110,14 @@ struct SDL_WindowData
     SDL_Window *keyboard_focus;
 
     char *app_id;
-    float windowed_scale_factor;
+    double scale_factor;
 
     struct Wayland_SHMBuffer icon;
 
     struct
     {
-        float x;
-        float y;
+        double x;
+        double y;
     } pointer_scale;
 
     // The in-flight window size request.