Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] wayland: Fix transform and scale handling when setting display mode #3798

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 0 comments
Closed

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.0
Reported for operating system, platform: Linux, All

Comments on the original bug report:

On 2020-08-17 01:11:05 +0000, Sebastian Krzyszkowiak wrote:

Created attachment 4440
0001-wayland-Fix-transform-and-scale-handling-when-settin.patch

This patch makes the reported desktop/fullscreen mode aware of screen rotation and hidpi scale, fixing games that don't react well to window resizes on such screens. This is particularly useful on mobile phones such as Librem 5, where plenty of games require landscape screen orientation in order to be playable.

Since there is no way for clients to request mode changes on Wayland, the mode handling code is simplified to just care about current mode, which also makes some games behave better under Wayland.

On 2020-10-09 01:18:38 +0000, Sebastian Krzyszkowiak wrote:

Comment on attachment 4440
0001-wayland-Fix-transform-and-scale-handling-when-settin.patch

From d5013d15119ddfce569c117a7f6d6d2d65c766ff Mon Sep 17 00:00:00 2001
From: Sebastian Krzyszkowiak sebastian.krzyszkowiak@puri.sm
Date: Sat, 15 Aug 2020 04:56:04 +0200
Subject: [PATCH] wayland: Fix transform and scale handling when setting
display mode


src/video/wayland/SDL_waylandvideo.c | 32 +++++++++++++++++++---------
src/video/wayland/SDL_waylandvideo.h | 1 +
2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/src/video/wayland/SDL_waylandvideo.c b/src/video/wayland/SDL_waylandvideo.c
index 469bb7364..439af44c3 100644
--- a/src/video/wayland/SDL_waylandvideo.c
+++ b/src/video/wayland/SDL_waylandvideo.c
@@ -241,6 +241,7 @@ display_handle_geometry(void *data,
SDL_VideoDisplay *display = data;

 display->name = SDL_strdup(model);
  • ((SDL_WaylandOutputData*)display->driverdata)->transform = transform;
    }

static void
@@ -253,18 +254,12 @@ display_handle_mode(void *data,
{
SDL_DisplayMode mode;
SDL_VideoDisplay *display = data;

  • SDL_zero(mode);
  • mode.format = SDL_PIXELFORMAT_RGB888;
  • mode.w = width;
  • mode.h = height;
  • mode.refresh_rate = refresh / 1000; // mHz to Hz
  • mode.driverdata = ((SDL_WaylandOutputData*)display->driverdata)->output;
  • SDL_AddDisplayMode(display, &mode);
  • SDL_WaylandOutputData* driverdata = display->driverdata;

    if (flags & WL_OUTPUT_MODE_CURRENT) {

  •    display->current_mode = mode;
    
  •    display->desktop_mode = mode;
    
  •    driverdata->width = width;
    
  •    driverdata->height = height;
    
  •    driverdata->refresh = refresh;
    
    }
    }

@@ -274,6 +269,23 @@ display_handle_done(void data,
{
/
!!! FIXME: this will fail on any further property changes! */
SDL_VideoDisplay *display = data;

  • SDL_WaylandOutputData* driverdata = display->driverdata;
  • SDL_DisplayMode mode;
  • SDL_zero(mode);
  • mode.format = SDL_PIXELFORMAT_RGB888;
  • mode.w = driverdata->width / driverdata->scale_factor;
  • mode.h = driverdata->height / driverdata->scale_factor;
  • if (driverdata->transform & WL_OUTPUT_TRANSFORM_90) {
  •   mode.w = driverdata->height / driverdata->scale_factor;
    
  •   mode.h = driverdata->width / driverdata->scale_factor;
    
  • }
  • mode.refresh_rate = driverdata->refresh / 1000; // mHz to Hz
  • mode.driverdata = driverdata->output;
  • SDL_AddDisplayMode(display, &mode);
  • display->current_mode = mode;
  • display->desktop_mode = mode;
  • SDL_AddVideoDisplay(display, SDL_FALSE);
    wl_output_set_user_data(output, display->driverdata);
    SDL_free(display->name);
    diff --git a/src/video/wayland/SDL_waylandvideo.h b/src/video/wayland/SDL_waylandvideo.h
    index 2c481d85e..d080be595 100644
    --- a/src/video/wayland/SDL_waylandvideo.h
    +++ b/src/video/wayland/SDL_waylandvideo.h
    @@ -86,6 +86,7 @@ typedef struct {
    typedef struct {
    struct wl_output *output;
    float scale_factor;
  • int width, height, refresh, transform;
    } SDL_WaylandOutputData;

#endif /* SDL_waylandvideo_h_ */

2.28.0

On 2020-10-09 01:20:32 +0000, Sebastian Krzyszkowiak wrote:

Created attachment 4477
0001-wayland-Fix-transform-and-scale-handling-when-settin.patch

Rebased to apply cleanly on current tip.

Sorry for the mess above, I forgot how to use Bugzilla :D

On 2021-01-31 04:29:44 +0000, Sebastian Krzyszkowiak wrote:

Ping? Fullscreen in SDL is unusable on rotated screens under Wayland without this patch; plus I've had another patch prepared for a while now which depends on this one to fix some crashes.

On 2021-01-31 04:35:02 +0000, Sebastian Krzyszkowiak wrote:

Created attachment 4743
0002-wayland-Don-t-crash-when-the-properties-of-already-e.patch

Added a second patch that fixes SDL apps crashing when properties of some output change. We're shipping those two patches in PureOS for Librem 5 for a while now, since they're needed for SDL2 games to work reasonably well.

On 2021-02-01 03:08:07 +0000, Sam Lantinga wrote:

The first patch is in, thanks!
https://hg.libsdl.org/SDL/rev/8aa3242eec6e

Please open a separate bug for the second patch.
BTW, I see that you add a "done" variable, and set it, but never read it in that patch.

On 2021-02-01 06:11:42 +0000, Sebastian Krzyszkowiak wrote:

Indeed, looks like I posted a wrong version with a botched rebase. Sent the second patch now as https://bugzilla.libsdl.org/show_bug.cgi?id=5525

Thanks!

On 2021-02-06 23:45:50 +0000, Christian Rauch wrote:

Created attachment 4768
fix memory leak in display callbacks

The patch "wayland: Don't crash when the properties of already existing wl_output change" opens a memory leak by not free-ing the 'display' any more.

The newly added AddressSanitizer reports this as:

Direct leak of 104 byte(s) in 1 object(s) allocated from:

0 0x7fb49ba999d1 in malloc (/usr/lib/x86_64-linux-gnu/liblsan.so.0+0xf9d1)

1 0x55ba26a8f870 in SDL_malloc_REAL [...]/SDL/src/stdlib/SDL_malloc.c:5387

2 0x55ba26c570cc in Wayland_add_display [...]/SDL/src/video/wayland/SDL_waylandvideo.c:323

3 0x55ba26c5749c in display_handle_global [...]/SDL/src/video/wayland/SDL_waylandvideo.c:398

4 0x7fb49c384ff4 (/usr/lib/x86_64-linux-gnu/libffi.so.7+0x6ff4)

The attached patch fixes this issue.

On 2021-02-06 23:53:00 +0000, Christian Rauch wrote:

Sorry, I just saw that the patch I referenced was actually merged as part of issue 5525.

See https://bugzilla.libsdl.org/show_bug.cgi?id=5525#c3 for the memory leak fix patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant