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

Implement GUI scaling for HighDPI displays and accessibility (SDL2 only) #1594

Merged
merged 31 commits into from Aug 13, 2023

Conversation

falbrechtskirchinger
Copy link
Contributor

@falbrechtskirchinger falbrechtskirchinger commented Jul 5, 2023

This PR implements a global scaling of the window content in the SDL2 driver (a stub has been added to the the WinAPI driver).

The window zoom can be adjusted in the options screen by selecting the desired zoom level from a combo box or by holding down Control and scrolling.

The zoom behavior in the menu screens is a little janky due to how the controls are sized. I've not investigated this any further. After looking at #1579 again, I suspect this is related to RescaleWindowProp/ScaleWindowPropUp. Maybe there's something to be done?

I don't own a HighDPI display myself and have only tested it on setups where windowSize == renderSize. Some optional improvements are under consideration for this PR and are listed in the to-do list below.

Fixes #533
Closes #1579

To-do:

@falbrechtskirchinger
Copy link
Contributor Author

I've incorporated @Spikeone's feedback and added an option to decouple scaling the game world from scaling the GUI. On HighDPI setups, some scaling is applied regardless of the setting.

I want to note that I very specifically chose the term "window zoom" (inspired by VSCode) over "GUI scale" because the game world was scaled as well (I wasn't eager to figure out how not to when I started and was far less familiar with the code base than I am now). Unlike in 3D games where scaling is limited to the GUI/HUD and scaling the game world makes no sense due to the perspective projection.

Arguably, this setting could be removed, "Window Zoom" be renamed to "GUI Scale," and the game world scaled depending on DPI, as is the case with the setting turned off.

@Xellzul
Copy link
Contributor

Xellzul commented Jul 11, 2023

@falbrechtskirchinger
Hey, had quick look at this PR (e64943e) on Windows 11 at 6880x2880 (21:9) resolution:

Everything works as expected except:
Taking in-game screenshots using F12, this was the result with scaling (note the size 1720 x 720 of the screenshot was exact 1/4 of used resolution of 6880x2880 using 400% scaling):
2023-07-11_19-56-19

In-game graph was thin
Screenshot 2023-07-11 200349

Few screenshots:
https://imgur.com/a/VX9xn1u

@falbrechtskirchinger
Copy link
Contributor Author

@Xellzul Thank you! I'm away for a few days but should be able to fix those issues over the weekend.

@falbrechtskirchinger
Copy link
Contributor Author

Both issues should be fixed. As expected, they required minimal changes, though the line drawing could be optimized by calculating the line width only once at a higher level – sacrificing a degree of abstraction – or by implementing BeginDrawLine()/EndDrawLine() functions to calculate and set the width once per multiple draw calls.

iwStatistics could also benefit from a significant rewrite, as many draw positions are specified manually. (In a separate PR and at a later date, of course.)

@falbrechtskirchinger
Copy link
Contributor Author

  • Should the setting be renamed to (G)UI scale and Zoom Game World be removed and always disabled?

I'm making an executive decision and going ahead with the change. "Window Zoom" becomes "GUI Scale" and the DPI scale always scales the game world view. I'm also moving from using unsigned to a new type that encapsulates all the precomputed values and the functions to transform points, obviating some of the additions to `IVideoDriver, as well as further additions or redundant calculations.

@falbrechtskirchinger falbrechtskirchinger marked this pull request as ready for review July 23, 2023 13:03
@falbrechtskirchinger falbrechtskirchinger changed the title Implement window zoom for HighDPI and/or accessibility (SDL2 only) Implement GUI scale for HighDPI and/or accessibility (SDL2 only) Jul 23, 2023
Copy link
Contributor Author

@falbrechtskirchinger falbrechtskirchinger left a comment

Choose a reason for hiding this comment

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

Discovered a few minor things, but otherwise am quite happy with the state of this PR.

extras/videoDrivers/SDL2/VideoSDL2.cpp Outdated Show resolved Hide resolved
libs/driver/include/driver/GuiScale.h Outdated Show resolved Hide resolved
libs/driver/src/VideoDriver.cpp Outdated Show resolved Hide resolved
libs/s25main/desktops/dskOptions.cpp Outdated Show resolved Hide resolved
libs/s25main/desktops/dskOptions.cpp Outdated Show resolved Hide resolved
libs/s25main/drivers/VideoDriverWrapper.cpp Outdated Show resolved Hide resolved
libs/s25main/ogl/OpenGLRenderer.cpp Outdated Show resolved Hide resolved
libs/s25main/world/GameWorldView.cpp Show resolved Hide resolved
libs/s25main/world/GameWorldView.h Outdated Show resolved Hide resolved
@falbrechtskirchinger falbrechtskirchinger changed the title Implement GUI scale for HighDPI and/or accessibility (SDL2 only) Implement GUI scaling for HighDPI displays and accessibility (SDL2 only) Aug 6, 2023
@falbrechtskirchinger
Copy link
Contributor Author

I've started using C++17 features. CI is expected to fail accordingly.

getDpiScale() returns the average ratio between the render size and the
window size.
The game world is only scaled to compensate for DPI scale.
Even though the SDL2 video driver is using SDL_GetWindowSize() and
SDL_GL_GetDrawableSize() to be DPI-aware, when creating a window,
HighDPI support was never actually requested.
TakeSreenshot() uses the wrong size, resulting in cropped screenshots
being taken.
The separate function has outlived its usefulness and has been folded
into setGuiScalePercent().
Handle each change individually and without delegation.
@falbrechtskirchinger
Copy link
Contributor Author

@Flamefire The rebase is done. Once you approve the changes a force-push should happen automatically within about a minute.
I've added a few more doc comments and applied one of your suggestions to another line.

Flamefire
Flamefire previously approved these changes Aug 13, 2023
Copy link
Member

@Flamefire Flamefire left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@Flamefire Flamefire left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your effort!

@Flamefire Flamefire merged commit e37a322 into Return-To-The-Roots:master Aug 13, 2023
12 of 13 checks passed
@falbrechtskirchinger falbrechtskirchinger deleted the window-zoom branch August 14, 2023 08:14
@Blackpearl1
Copy link

I just used the newest nightly(e37a322)

I change the GUI zoom to 200% and.... nothing happen.
Restartet the game and... nothing happen.

I Use 3840x2160 (16:9)

Whats wrong?

@falbrechtskirchinger
Copy link
Contributor Author

falbrechtskirchinger commented Aug 28, 2023

I just used the newest nightly(e37a322)

I change the GUI zoom to 200% and.... nothing happen. Restartet the game and... nothing happen.

I Use 3840x2160 (16:9)

Whats wrong?

@Blackpearl1 Changing the GUI scale in the options screen should have an immediate effect.
I need more information:

  • What's your OS?
  • Are you using the SDL2 video backend/driver? (WinAPI is not supported.)
  • Did you get any warning messages when leaving the options screen?
  • Could you post screenshots of the options screen with two different GUI scales selected?

The most likely scenario is that you're using the WinAPI backend. If you didn't get a warning message when leaving the options screen, I made some mistake in determining whether the selected GUI scale is supported.

@Blackpearl1
Copy link

Blackpearl1 commented Aug 29, 2023

@falbrechtskirchinger

Are you using the SDL2 video backend/driver? (WinAPI is not supported.)

That was my fault.
I used the win...whatever

I changed it to SDL2 and now it works like a charm. Thank you :D nice job.

The Message that it wont work with the win...whatever comes after changing to SDL2...

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.

[wish] some kind of hidpi support
5 participants