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

Updated SDL high DPI support #7711

Merged
merged 2 commits into from
May 17, 2023
Merged

Conversation

slouken
Copy link
Collaborator

@slouken slouken commented May 16, 2023

We have gotten feedback that abstracting the coordinate system based on the display scale is unexpected and it is difficult to adapt existing applications to the proposed API.

The new approach is to provide the coordinate systems that people expect, but provide additional information that will help applications properly handle high DPI situations.

The concepts needed for high DPI support are documented in README-highdpi.md. An example of automatically adapting the content to display scale changes can be found in SDL_test_common.c, where auto_scale_content is checked.

Fixes #7709

@slouken slouken requested review from Kontrabant, flibitijibibo and a team and removed request for Kontrabant May 16, 2023 23:41
@slouken slouken force-pushed the high-dpi-take-2 branch 2 times, most recently from f854a01 to e3ffceb Compare May 17, 2023 00:44
@slouken
Copy link
Collaborator Author

slouken commented May 17, 2023

@Kontrabant, in the new model, does it make sense for the Wayland scale to be a content scale or a pixel density scale? Right now content scale is advisory and meant for UI, and pixel densities > 1 are not exposed by default.

@slouken slouken force-pushed the high-dpi-take-2 branch from 6e851c2 to 14beef6 Compare May 17, 2023 00:58
@Kontrabant
Copy link
Contributor

@Kontrabant, in the new model, does it make sense for the Wayland scale to be a content scale or a pixel density scale? Right now content scale is advisory and meant for UI, and pixel densities > 1 are not exposed by default.

Pixel density. Wayland handles scaling similar to how Mac does, so the compositor will scale the backbuffer automatically if necessary. This is how it was handled in SDL 2 for non-DPI aware apps as well.

@slouken slouken force-pushed the high-dpi-take-2 branch 3 times, most recently from 1bc1405 to dc70582 Compare May 17, 2023 05:07
@slouken
Copy link
Collaborator Author

slouken commented May 17, 2023

@Kontrabant, can you review this? I'm pretty sure the places where I switched pixel_w -> w in the Wayland code isn't correct.

The mode size is in window coordinate space, and the pixel_density transforms it into pixel space, but it seems like there's a better way to handle this than multiplying them out every time we want the pixel size of the mode.

slouken added 2 commits May 16, 2023 22:18
We have gotten feedback that abstracting the coordinate system based on the display scale is unexpected and it is difficult to adapt existing applications to the proposed API.

The new approach is to provide the coordinate systems that people expect, but provide additional information that will help applications properly handle high DPI situations.

The concepts needed for high DPI support are documented in README-highdpi.md. An example of automatically adapting the content to display scale changes can be found in SDL_test_common.c, where auto_scale_content is checked.

Also, the SDL_WINDOW_ALLOW_HIGHDPI window flag has been replaced by the SDL_HINT_VIDEO_ENABLE_HIGH_PIXEL_DENSITY hint.

Fixes libsdl-org#7709
@slouken slouken force-pushed the high-dpi-take-2 branch from dc70582 to 2879f2b Compare May 17, 2023 05:18
float display_scale; /**< scale converting screen coordinates to pixels (e.g. a 2560x1440 screen size mode with 1.5 scale would have 3840x2160 pixels) */
int w; /**< width */
int h; /**< height */
float pixel_density; /**< scale converting size to pixels (e.g. a 1920x1080 mode with 2.0 scale would have 3840x2160 pixels) */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to split this into separate variables for width and height? The RISC OS and 3DS video drivers are both likely to benefit from this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would add a fair amount of complexity, both internally and for the application, understanding how to handle non-uniform scale. Can you provide an example of how this could happen? Both of those platforms currently use normal width and height and pixel density of 1.0.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • On RISC OS, modes have separate X and Y eigen factor values of 0-3 (for DPIs of 180, 90, 45 and 22.5). Square pixel modes are the most common, but some older numbered modes like mode 15 use EX1 EY2.
  • On the 3DS, all models except the Old 2DS support a higher resolution 800x240 mode for the top screen that can be used instead of the regular 400x240. It can be enabled using the gfxSetWide() function in libctru.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined not to support non-square pixels at this point. If an application really needs to use these modes, they can use platform specific code to set a hint that's used by the RISCOS driver. We can revisit this in the future if non-square pixels become more widely used, but aside from the complexity of the API, the application needs to be specially designed to handle them.

For the 3DS, that sounds like it's just two different display modes and if the driver sees the 800x240 one requested, it can call the gfxSetWide() function to enable it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, what should be reported for pixel_density in those modes?

@Kontrabant
Copy link
Contributor

Kontrabant commented May 17, 2023

@Kontrabant, can you review this? I'm pretty sure the places where I switched pixel_w -> w in the Wayland code isn't correct.

The mode size is in window coordinate space, and the pixel_density transforms it into pixel space, but it seems like there's a better way to handle this than multiplying them out every time we want the pixel size of the mode.

Actually, those instances of pixel_ww look OK. The code for adding display info will require some work for the new pixel density hint, but that can happen after this is committed.

As for multiplying by the scale factor to get the real pixel size, those instances can go too with the work for the above, as exclusive fullscreen modes will always have a pixel density of 1.

@slouken
Copy link
Collaborator Author

slouken commented May 17, 2023

The pixel density hint feeds into the HIGHDPI flag, so that can use review but should otherwise be fine. I'll go ahead and merge, thanks!

@slouken slouken merged commit 26e780b into libsdl-org:main May 17, 2023
@slouken slouken deleted the high-dpi-take-2 branch May 17, 2023 19:58
@sezero
Copy link
Contributor

sezero commented May 17, 2023

sdl2-compat needs updating after this.

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.

High DPI support in SDL 3.0 - Take 2
4 participants