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

Add support for scaled resolutions #6516

Merged
merged 25 commits into from Feb 5, 2023
Merged

Add support for scaled resolutions #6516

merged 25 commits into from Feb 5, 2023

Conversation

ihhub
Copy link
Owner

@ihhub ihhub commented Jan 24, 2023

Based on the demand from users with high resolution displays. This feature is available only on SDL 2.

Also fixed text alignment for resolution selection dialog.

relates to #5373
relates to #4946

Based on the demand from users with high resolution displays. This feature is available only on SDL 2.

relates to #5373
relates to #4946
@ihhub ihhub added improvement New feature, request or improvement ui UI/GUI related stuff labels Jan 24, 2023
@ihhub ihhub added this to the 1.0.1 milestone Jan 24, 2023
@ihhub ihhub self-assigned this Jan 24, 2023
@ihhub ihhub marked this pull request as draft January 24, 2023 08:08
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

src/engine/screen.h Outdated Show resolved Hide resolved
src/engine/screen.h Outdated Show resolved Hide resolved
src/fheroes2/dialog/dialog_resolution.cpp Outdated Show resolved Hide resolved
src/fheroes2/dialog/dialog_resolution.cpp Outdated Show resolved Hide resolved
src/fheroes2/dialog/dialog_resolution.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

src/fheroes2/dialog/dialog_resolution.cpp Show resolved Hide resolved
src/fheroes2/dialog/dialog_resolution.cpp Show resolved Hide resolved
src/fheroes2/dialog/dialog_resolution.cpp Show resolved Hide resolved
src/engine/screen.h Show resolved Hide resolved
src/engine/screen.h Show resolved Hide resolved
@ihhub ihhub marked this pull request as ready for review January 24, 2023 09:08
Copy link
Collaborator

@oleg-derevenetz oleg-derevenetz left a comment

Choose a reason for hiding this comment

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

Hi @ihhub I just left a few suggestions, please take a look when you have time. In addition, this scale is actually not really used anywhere yet, as far as I understand?

src/engine/screen.cpp Outdated Show resolved Hide resolved
src/engine/screen.cpp Outdated Show resolved Hide resolved
src/engine/screen.cpp Outdated Show resolved Hide resolved
src/engine/screen.h Show resolved Hide resolved
src/fheroes2/dialog/dialog_resolution.cpp Outdated Show resolved Hide resolved
src/fheroes2/dialog/dialog_resolution.cpp Outdated Show resolved Hide resolved
src/fheroes2/dialog/dialog_resolution.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@oleg-derevenetz oleg-derevenetz left a comment

Choose a reason for hiding this comment

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

Hi @ihhub I just left a few questions and suggestions, could you please take a look when you have time?

src/engine/screen.cpp Outdated Show resolved Hide resolved
src/engine/screen.cpp Outdated Show resolved Hide resolved
src/fheroes2/dialog/dialog_resolution.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@Districh-ru Districh-ru left a comment

Choose a reason for hiding this comment

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

Hi @ihhub, I found one bug, could you please address it when you have time:
If I set the 2x resolution in windowed mode (i.e. 960x540(x2)) and then switch to fullscreen mode by clicking the Mode icon (not F4), the engine switches to 960x540 without x2.
If I press F4 in windowed mode for the first time the engine correctly switches to 960x540(x2). But if I press F4 two more times the engine first switches to 960x540(x2) windowed and then to 960x540 fullscreen without x2.
Changing resolution without switching fullscreen/windowed mode works correctly.
I use Windows OS.

@Districh-ru
Copy link
Collaborator

Hi @Districh-ru , could you please test the latest changes in this pull request and see if this issue is gone?

Hi, @ihhub, I've tested the last commit. The issue is gone, now the Fullscreen mode is correctly set for scaled resolutions.

@ihhub
Copy link
Owner Author

ihhub commented Feb 4, 2023

Hi @oleg-derevenetz , I addressed your concerns about uniqueness of elements as well I made extra optimization of the code. Please take a look again when you have time.

@ihhub
Copy link
Owner Author

ihhub commented Feb 4, 2023

Hi @Branikolog , could you please test this pull request as the highest priority as we need this change for the upcoming release?

Copy link
Collaborator

@oleg-derevenetz oleg-derevenetz left a comment

Choose a reason for hiding this comment

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

Hi @ihhub I just left a few questions and suggestions, could you please take a look?

src/engine/screen.cpp Outdated Show resolved Hide resolved
src/engine/screen.cpp Show resolved Hide resolved
src/engine/screen.cpp Outdated Show resolved Hide resolved
src/engine/screen.cpp Outdated Show resolved Hide resolved
src/engine/screen.cpp Outdated Show resolved Hide resolved
src/engine/screen.cpp Outdated Show resolved Hide resolved
src/fheroes2/system/settings.cpp Outdated Show resolved Hide resolved
@Branikolog
Copy link
Collaborator

Hi, @ihhub
Everything works stable.
A few things I don't get...
This PR adds an ability to double only one resolution for my device:
image
My monitor is 1080p, but for some reason none of custom resolutions are available for being x2.
Also, while choosing x2 resolution in window mode or even full screen I have blurred image:
image
I suppose, this feature was made to create a precise x2 picture increase, but currently it works the same to software cursor mode with linear filtering as if I manually resize game window....
Sorry, if I understood somethig wrong...

@oleg-derevenetz
Copy link
Collaborator

My monitor is 1080p, but for some reason none of custom resolutions are available for being x2.

640x480x2 resolution is 1280x960, but with in-game picture linearly scaled x2, so GUI elements doesn't look small, like with just 1280x960. To have, say, 800x600x2, you should have 1600x1200 in the list of supported resolutions, and to have 928x520x2 you should have 1856x1040 in the list of supported resolutions. Do you have them in this list?

Also, while choosing x2 resolution in window mode or even full screen I have blurred image

Yes, it was to be expected because of upscaling.

I suppose, this feature was made to create a precise x2 picture increase, but currently it works the same to software cursor mode with linear filtering as if I manually resize game window....

There is still one of SDL upscaling algorithms is applied. It's in fact really as-if you resized your window x2 linearly in both dimensions.

@Districh-ru
Copy link
Collaborator

Districh-ru commented Feb 4, 2023

Hi, @ihhub Everything works stable. A few things I don't get... This PR adds an ability to double only one resolution for my device: My monitor is 1080p, but for some reason none of custom resolutions are available for being x2. Also, while choosing x2 resolution in window mode or even full screen I have blurred image: I suppose, this feature was made to create a precise x2 picture increase, but currently it works the same to software cursor mode with linear filtering as if I manually resize game window.... Sorry, if I understood somethig wrong...

Hi, @Branikolog.
Interesting, I have also 1080p display, but fheroes2 allowed me to use two 2x resolutions:
изображение
As I know, this depends on resolutions supported by graphics card. Some drivers allow to set custom resolutions. After I added new resolutions fheroes2 allowed me to use them:
изображение

(There would be good in future to have non integer scales, for example set in-game resolution to 1366x768 and upscale it to fullscreen 1920x1080.)

@Branikolog
Copy link
Collaborator

Hi, @oleg-derevenetz !

Do you have them in this list?

That's a valid point. I've added this custom resolution and one more x2 option appeared for me. 👍

There is still one of SDL upscaling algorithms is applied. It's in fact really as-if you resized your window x2 linearly in both dimensions.

Well, I've expected, if I set 640x480 x2 (window more) I would get a clear image with each game pixel include 2 pixels of my monitor. But it works the same (I mean being blurred while custom stretched) as if I stretched the game window by hand for some unknown, non-integer size.
That looks a bit weird, applying this x2 resolution in fullscreen mode in hardware mode looks blurry, while choosing 640x480 x1 which looks completely the same in fullscreen mode, except of scaled cursor, and it works crispy.

Anyway, Thanks for clarification, @oleg-derevenetz , very much. :)

Hi, @Districh-ru !

Interesting, I have also 1080p display, but fheroes2 allowed me to use two 2x resolutions:

My monitor is rather old and supports quite a poor set of resolutions. I had to add manually some low widescreen resolutions, as the lowest default available 16:9 for me was 1280x768.

As I know, this depends on resolutions supported by graphics card. Some drivers allow to set custom resolutions. After I added new resolutions fheroes2 allowed me to use them:

Worked for me as well. :)

(There would be good in future to have non integer scales, for example set in-game resolution to 1366x768 and upscale it to fullscreen 1920x1080.)

Isn't it upscaled if we run the application fullscreen? I mean each of the available resolutions are stretching the image on running application fullscreen.
For me, this particular PR changes almost nothing, since I mostly play fullscreen so any resolution I choose shows the image upscaled to fullscreen. Even more, while playing with hardware cursor mode I enjoy crisp and clear image, no matter what resolution I've chosen. However playing with software emulated cursor mode turned on, makes image blurred. If I want to play in a window mode I simply stretch game window by hand to any size I want, which brings the same quality as software emulated cursor mode (I mean scaled and blurred image). So those fixed x2 options could be useful only for the cases where user runs application fullscreen, but without stretching on default (in the middle of the screen with black borders). But I'm not sure anyone uses such video driver setting for any application in OS.
I've expected these x2 resolutions scaling the game for an integer x2 value in window mode at least, keeping crisp image while playing in stretched window mode, but unfortunately it doesn't work for me.

More anticipated feature for me is to allow to set a custom resolutions, as I have to manually set them for my OS, before it appears in resolution selection window.
Many players (same as me) are missing widescreen resolutions, lower than default availabe (which is ~1280x768... I've noticed the majority of streamers choose this resolution, as it's the lowest available widescreen one, otherwise they set lower resolutions, but 4:3 ratio). As you can see on my screenshot I've had to set a custom 928x520 resolution within my OS to be able to play 16:9 with satisfying size of battle screen.
I wish an option to be able to paste a custom resolution at least for software emulated cursor mode (since in hardware mode OS itself switches to the resolution we select, which couldn't be possible, if it's not in the OS list, I suppose), but I'm not sure it's possible though.

@sonarcloud
Copy link

sonarcloud bot commented Feb 5, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Districh-ru
Copy link
Collaborator

(There would be good in future to have non integer scales, for example set in-game resolution to 1366x768 and upscale it to fullscreen 1920x1080.)

Isn't it upscaled if we run the application fullscreen? I mean each of the available resolutions are stretching the image on running application fullscreen.

Not exact. Any resolution changes the Desktop resolution ans the upscale is done by monitor or graphic card driver. This causes a problem in Windows OS: Opened windows change their size to match the set resolution, changing their inner arrangement. And after exiting the game some of opened windows do not restore its original size and inner arrangement. Also some windows are force dropped to my second monitor, which is TV and is mostly turned off, so I have to get them back from the switched off TV "to the touch". :) This is the Windows OS issue and it causes with many applications. But if fheroes2 would set the in-game resolutuion to chosen and in fullscreen do not change the Desktop resolution it would be very nice for me. :)
Setting x2 resolution the upscale is done by fheroes2 engine (actually by SDL2) and not by monitor or graphics driver.

@Branikolog
Copy link
Collaborator

Not exact. Any resolution changes the Desktop resolution ans the upscale is done by monitor or graphic card driver. This causes a problem in Windows OS: Opened windows change their size to match the set resolution, changing their inner arrangement. And after exiting the game some of opened windows do not restore its original size and inner arrangement. Also some windows are force dropped to my second monitor, which is TV and is mostly turned off, so I have to get them back from the switched off TV "to the touch". :) This is the Windows OS issue and it causes with many applications. But if fheroes2 would set the in-game resolutuion to chosen and in fullscreen do not change the Desktop resolution it would be very nice for me. :)

I experience the same problems, but I prefer image quality over my OS windows arrangement. :)

Isn't this problem was fixed while running on software emulated cursor mode? (which lowers quality as well, but makes it possible to run engine on any resolution plus scaling the cursor)

Setting x2 resolution the upscale is done by fheroes2 engine (actually by SDL2) and not by monitor or graphics driver.

I see.

@ihhub ihhub merged commit 0c94c69 into master Feb 5, 2023
@ihhub ihhub deleted the scaled_resolution branch February 5, 2023 11:00
@Districh-ru
Copy link
Collaborator

Districh-ru commented Feb 5, 2023

I experience the same problems, but I prefer image quality over my OS windows arrangement. :)

Then in would be great if the next improvements could give the ability to set the scaler, i.e. like in Homm3HD :)
изображение

Not all all the first time, but currently, I think, we can choose between smoothed (Bilinear scaling) and "Nearest neighbor".

@oleg-derevenetz
Copy link
Collaborator

@Districh-ru you are already able to choose the scaling method (among supported by SDL) via fheroes2.cfg.

@Districh-ru
Copy link
Collaborator

Thanks, @oleg-derevenetz, I forgot about this. I prefer Bilinear scaling, so I did not search the сап file for it.
Setting screen scaling type = nearest disables blurring. @Branikolog is that what you expected from the "x2" resolutions?

@Branikolog
Copy link
Collaborator

@Districh-ru
I'll check it later once more to be sure... I've checked that mode yesterday, but with nearest scaling the image on x2 resolutions looked like scaled not for integer value.

@Branikolog
Copy link
Collaborator

Setting screen scaling type = nearest disables blurring. @Branikolog is that what you expected from the "x2" resolutions?

I believe, nearest scaling should be turned on as a default while selecting x2 resolutions in window mode.
Is it time to add software/hardware modes and scaling toggles in game, so player could easily adjust the image? Right now I have to edit .cfg file each time I want to switch to that x2 resolutions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high priority Very critical change needed immediately improvement New feature, request or improvement ui UI/GUI related stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants