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.