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

wayland: Set the display scale for video modes #7152

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

Kontrabant
Copy link
Contributor

  • Set the desktop display scale
  • Exclusive fullscreen modes all have a scale of 1.0, so don't add the scaled desktop mode to the list.

@slime73
Copy link
Contributor

slime73 commented Jan 25, 2023

Exclusive fullscreen modes all have a scale of 1.0, so don't add the scaled desktop mode to the list.

Does this mean it's not possible to return to the desktop's display mode via SDL_SetWindowDisplayMode? Or just that the desktop's display mode is missing from the SDL_GetDisplayMode list but is available to switch to (after a non-desktop mode has been activated) via SDL_GetDesktopDisplayMode?

If the latter, that'd be inconsistent with other platforms right?

@Kontrabant
Copy link
Contributor Author

Kontrabant commented Jan 25, 2023

The latter. Wayland doesn't actually do mode switching, so there's nothing to switch back to. Fullscreen modes are just surfaces scaled with a viewport.

The way the backend currently works, exclusive fullscreen modes all have a scale of 1.0. The native 1:1 mode for the display is mode 0, and the rest are VESA CVT video modes that are exposed to make life easier for applications that don't handle scaling internally and are scaled to the output size. I was thinking that it didn't make sense to expose strange, non-standard, scaled desktop sizes as a fullscreen mode (for instance a 4k display with 1.25 is 3072x2048, which isn't a valid CVT mode, so I'm not sure why anything would want to use it). The only exception would be if something scales the desktop but doesn't expose the viewport protocol for scaling surfaces, in which case the desktop resolution, and only the desktop resolution, has to be exposed no matter what, because there's nothing else available.

If it's a problem and in conflict with other platforms, I'll put it back in the mode list, although for now the desktop resolution in the fullscreen mode list would have to have a scale of 1.0, as the Wayland backend won't handle scaled, exclusive fullscreen modes without some extra work. SDL_GetDesktopDisplayMode() still returns the desktop resolution with the scale value, though.

@slime73
Copy link
Contributor

slime73 commented Jan 25, 2023

Personally I'd expect the SDL_GetDisplayMode list to contain all modes that are valid for SDL_SetWindowDisplayMode, including the desktop display mode even if the OS or backend is doing something with it. macOS has behaved that way forever (since modes have DPI scales associated with them on that platform, which the OS uses rather than being something that the monitor hardware itself deals with).

But maybe other people have different opinions or expectations.

@Kontrabant
Copy link
Contributor Author

This is a area where Windows/Wayland and macOS differ and whether or not exclusive fullscreen modes should have non-1.0 scale values is something that will need to be sorted out in the main discussion thread.

I'm in the camp where, when it comes to exclusive fullscreen modes, I would expect SDL_GetDisplayMode() to return a set of resolutions that the display physically supports (or pretends to support at some lower level), and applications can use as though they have direct access to the display with no desktop or scaling in between, as has historically been the case.

@slime73
Copy link
Contributor

slime73 commented Jan 25, 2023

I'm in the camp where, when it comes to exclusive fullscreen modes, I would expect SDL_GetDisplayMode() to return a set of resolutions that the display physically supports (or pretends to support at some lower level)

I generally agree - but I think it'd be very confusing for the list to not include the current desktop display mode, which the display and OS together fully support by definition. With SDL3 users now have information about DPI scales of display modes so they can make more informed choices about which ones they want to use.

SDL_GetDisplayMode and SDL_GetNumDisplayModes are simply documented to get "available display modes" - and the desktop display mode definitely falls into that camp in my opinion, since it doesn't say anything about whether they're emulated to a degree by the OS or backend, or not.

@Kontrabant
Copy link
Contributor Author

Kontrabant commented Jan 25, 2023

I agree that the terminology is ambiguous.

Perhaps what's ultimately needed here is a better distinction between physical video modes and the logical desktop/output region properties? Reserve the term 'mode' for situations that correspond to physical pixels and display resolutions, while something like 'properties' can be used when talking in terms of logical point and scaling?

@slouken
Copy link
Collaborator

slouken commented Jan 25, 2023

Conceptually I think it's fine that the desktop mode has display scale and the others don't. I'm on the fence about whether we should include it in the list of modes. We already have a function to access the desktop mode and you return to it by specifying mode NULL, so maybe for SDL 3.0 we can define the mode list as being fullscreen display modes, which always have display scale of 1.0?

@slime73, I was wondering if maybe it makes sense to skip the @2x fullscreen modes on macOS? Is there always a corresponding @1x mode?

@slime73
Copy link
Contributor

slime73 commented Jan 25, 2023

I was wondering if maybe it makes sense to skip the @2x fullscreen modes on macOS? Is there always a corresponding 1x mode?

As far as I know there isn't (here's an example of what's produced by macOS #5290 (comment) ) - and Apple likes changing things enough that I probably wouldn't want to rely on a specific behaviour that works for the current macOS version.

Since most users on macOS will have an active desktop display mode with a DPI scale above 1x I also think it'd be unintuitive to have that sort of mismatch between the active display mode and the "available display modes" - selecting 2880x1800@1x doesn't behave the same as selecting 1440x900@2x at the OS level (positioning/dimensions are different, and it would do a real modeswitch with the curtain etc. if someone chose the former as an exclusive fullscreen mode when the desktop uses the latter.)

From my perspective, having different DPI scales on display modes is just like any other property (refresh rate, bit depth, etc.) - some people will want to expose everything to users, some people will have logic in their game to prefer specific ones, some people will throw out almost everything except a handful, etc. IMO there's no reason to take those choices away from them by skipping valid modes that a platform provides.

@Kontrabant
Copy link
Contributor Author

it would do a real modeswitch with the curtain etc. if someone chose the former as an exclusive fullscreen mode when the desktop uses the latter.)

Isn't that generally the desired behavior when entering exclusive fullscreen vs fullscreen desktop on Mac though?

@slime73
Copy link
Contributor

slime73 commented Jan 25, 2023

Yeah, what I meant to say is when the user has set 1440x900@2x for their display in their Mac's system preferences, an app that uses 2880x1800@1x in fullscreen has to switch display modes at the OS level because it's not the same mode as what the desktop has. Whereas if the app uses 1440x900@2x, then the app may enter exclusive-fullscreen without switching display modes.

@Kontrabant
Copy link
Contributor Author

Kontrabant commented Jan 25, 2023

What if, in the cases where there are 2x and 1x modes that ultimately lead to the same backbuffer dimensions, the native 2x versions override the 1x versions that incur a mode switch, but are displayed as though they are 1x? So, 1440x900 would be displayed as 2880x1800 in the fullscreen mode list, but choosing it would activate the 1440x900 2x mode in the background, and the mac video backend can scale things like pointer coordinates by 2x where necessary?

@slime73
Copy link
Contributor

slime73 commented Jan 25, 2023

What's the goal of doing something complicated in SDL like that to remove information we've chosen to expose, when that information is also useful to people using SDL?

Like... this is the purpose of the dpi scale field in display modes. I don't think we should remove it. There are several bug reports in this Github issue tracker illustrating that it was sorely needed.

@slouken
Copy link
Collaborator

slouken commented Jan 25, 2023

I think the idea is to simplify, because macOS is currently the only platform that has display scale for fullscreen modes, it's going to lead to applications incorrectly handling that case. If we do end up with the guarantee that fullscreen modes are always 1-1, then we can remove the display scale from the mode and make it a desktop property.

I'm not arguing one way or the other here, I'm just clarifying why we're even talking about it.

@slime73
Copy link
Contributor

slime73 commented Jan 25, 2023

The DPI scale changes when you change display modes on other platforms too (e.g. on Windows it will be whatever value when the desktop display mode is used, including in various fullscreen situations, and 1x when some other mode is used), so it's still associated with display modes no matter what I think.

@slouken
Copy link
Collaborator

slouken commented Jan 25, 2023

The DPI scale changes when you change display modes on other platforms too (e.g. on Windows it will be whatever value when the desktop display mode is used, including in various fullscreen situations, and 1x when some other mode is used), so it's still associated with display modes no matter what I think.

I can double check, but I believe @ericwa said that Windows switches back to 96 DPI (1x) when changing from the desktop display mode. I've also seen that referenced in other documentation as well.

@slime73
Copy link
Contributor

slime73 commented Jan 25, 2023

Right that's what I mean: the active DPI scale is tied to the active display mode.

@slouken
Copy link
Collaborator

slouken commented Jan 25, 2023

In any case, @Kontrabant, for now let's leave the desktop display mode in the list, with the appropriate display scale, and also add the other fullscreen modes with 1.0 scale. That matches the current behavior on other platforms, and we can revisit whether the desktop mode should be a special snowflake later.

@Kontrabant
Copy link
Contributor Author

Got it.

@Kontrabant Kontrabant force-pushed the wayland_scale_values branch from bcc4b9b to d88234f Compare January 25, 2023 23:29
@slouken
Copy link
Collaborator

slouken commented Jan 25, 2023

It's okay to leave the SDL_WINDOW_ALLOW_HIGHDPI support in there. We can clean that out as a separate pass later once we've worked out all the kinks in our new approach.

@Kontrabant Kontrabant force-pushed the wayland_scale_values branch from d88234f to 9f724d7 Compare January 26, 2023 00:05
@Kontrabant Kontrabant marked this pull request as draft January 26, 2023 01:13
@Kontrabant Kontrabant force-pushed the wayland_scale_values branch 2 times, most recently from 333edf1 to 022cb80 Compare January 26, 2023 18:38
- Adds support for scaled fullscreen modes
- General cleanup of scale factor setting and usage
@Kontrabant Kontrabant force-pushed the wayland_scale_values branch from 022cb80 to 2adc287 Compare January 26, 2023 18:40
@Kontrabant
Copy link
Contributor Author

Updated the Wayland backend to be able to handle any arbitrary scaled fullscreen mode, like the desktop mode which is now in the mode list, so this should be good to go now.

@Kontrabant Kontrabant marked this pull request as ready for review January 26, 2023 19:10
@slouken
Copy link
Collaborator

slouken commented Jan 26, 2023

Great!

@slouken slouken merged commit b2cfcbd into libsdl-org:main Jan 26, 2023
@Kontrabant Kontrabant deleted the wayland_scale_values branch February 4, 2023 19:06
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

Successfully merging this pull request may close these issues.

3 participants