Skip to content

Add Screen Scaling Type to graphics settings dialog and Interface Type to the game settings dialog #9971

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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

Districh-ru
Copy link
Collaborator

@Districh-ru Districh-ru commented Jul 6, 2025

Thanks to @PusshPop for the Linear/Nearest scaling icon!

This PR adds new settings:

  • Screen Scaling Type to the Graphics Settings dialog;
  • Interface Type to Game Settings dialog.

The parameter names, description texts and place in the dialog can be discussed.

The new Screen Scaling Type option must be tested on all supported platforms.

@Districh-ru Districh-ru added this to the 1.1.10 milestone Jul 6, 2025
@Districh-ru Districh-ru self-assigned this Jul 6, 2025
@Districh-ru Districh-ru added improvement New feature, request or improvement ui UI/GUI related stuff labels Jul 6, 2025
@zenseii
Copy link
Collaborator

zenseii commented Jul 6, 2025

@Districh-ru, nice job adding all of these. I was thinking if we should have the scaling filters and mouse rendering under the graphics settings? Consider that we're probably soon-ish going to add the numeric army estimate option too, then we could add that to the main options page? I'm talking about this one:
image
#9678
We would still need 1 more option to have 3 (gotta admit I don't like that we always have to add several options just to fit the grid), so for the last option maybe we could just add the battle mode button (quick, manual, spell casting etc.) to the main options page. What do you think?

EDIT: I guess it would also make sense to have the cursor rendering option under a sub menu for cursor settings, in which we would include the color/b&w setting.

@Districh-ru
Copy link
Collaborator Author

@ihhub and @zenseii, we can add a UI option for "numeric army estimate" in this PR after #9678 is merged.

@zenseii
First of all we need to test scaling type and cursor renderer options on different systems (PS Vita, Android, ...).

We can move the made options to the other dialog. It is not difficult. :)
On the next days I'll move the options to the Graphics menu or to some other suitable place.

Making an extra dialog for two options sounds not good to me: a user has to make more clicks to get to the option ... but I'll give it a chance. :)

And IMHO we can make not all lines with 3 options, what do you think about the next approach:
изображение

PS
@zenseii could you please check the new added strings when you have time? We can move the items to the other place but the text will be the same.

Copy link
Collaborator

@zenseii zenseii left a comment

Choose a reason for hiding this comment

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

Looked over the strings and didn't find much. I'll test on Vita and report back soon.

@zenseii
Copy link
Collaborator

zenseii commented Jul 6, 2025

@Districh-ru, I agree about your sentiments about 2 options for cursor so we can drop that idea. My main concern is that the cursor rendering setting is so technical that having it on the main page of settings will confuse users, which means it might be better on the graphics page. Ideally we would not need an option for it but apply this whenever an integer or larger than 640x480 resolution is selected (?). But this is a much bigger discussion and not necessary here.

I think it looks good to have the grid you showed:
xxx
xxx
x x

@oleg-derevenetz
Copy link
Collaborator

Hi @Districh-ru please note that on devices with touchpad/touchscreen software-rendered mouse cursor is mandatory, because automatic conversion of mouse events to touch events and vice versa is disabled because we need to reliably distinguish between them. Please see the initTouchpad() in the engine/localevent.cpp. On these platforms, hardware mouse cursor is harmful and should never be enabled regardless of the config setting.

@Districh-ru
Copy link
Collaborator Author

Hi @Districh-ru please note that on devices with touchpad/touchscreen software-rendered mouse cursor is mandatory, because automatic conversion of mouse events to touch events and vice versa is disabled because we need to reliably distinguish between them. Please see the initTouchpad() in the engine/localevent.cpp. On these platforms, hardware mouse cursor is harmful and should never be enabled regardless of the config setting.

Thanks for the warning, @oleg-derevenetz!

But as I can see the cursor "software emulation" stays only for how to render it on the screen:

  • "hardware" - by using SDL cursor setting (SDL_SetCursor()) and rendering;
  • "software" - by rendering cursor sprite to the display image before calling _renderFrame() in render() method.

The events processing in localevent.cpp does not rely on cursor software emulation state, as I can see... but I might miss something important. I'll do some test in the evening on Android and PS Vita emulator.

@oleg-derevenetz
Copy link
Collaborator

The events processing in localevent.cpp does not rely on cursor software emulation state, as I can see...

Please note the _mouseCursorPos and _globalMouseMotionEventHook in the LocalEvent. The latter effectively calls the Cursor::updateCursorPosition(), which moves the software mouse cursor to the correct position. Where do you think a "hardware mouse cursor" can come from in a device without a mouse? :) This could be (in theory) emulated by SDL to some extent... but the automatic translation of touch events to the mouse events is disabled (not to mention joysticks, for which this translation logic is not provided by SDL at all).

@Districh-ru
Copy link
Collaborator Author

Please note the _mouseCursorPos and _globalMouseMotionEventHook in the LocalEvent. The latter effectively calls the Cursor::updateCursorPosition(), which moves the software mouse cursor to the correct position. Where do you think a "hardware mouse cursor" can come from in a device without a mouse? :) This could be (in theory) emulated by SDL to some extent... but the automatic translation of touch events to the mouse events is disabled (not to mention joysticks, for which this translation logic is not provided by SDL at all).

You are absolutely right.

I guess for now we need to disable this option for systems with touchscreen by making it gray or simply not showing it.

Later we can think about if we can use SDL_WarpMouseInWindow() to set the hardware cursor position by touch or by gamepad controller.

@oleg-derevenetz
Copy link
Collaborator

oleg-derevenetz commented Jul 7, 2025

Later we can think about if we can use SDL_WarpMouseInWindow() to set the hardware cursor position by touch or by gamepad controller.

I strongly doubt that SDL_WarpMouseInWindow() will somehow help if there is actually no mouse in the system (and its driver is not running). Even in Windows, the system mouse cursor disappears completely when the mouse is disconnected from the USB port (at least in Windows 10, I just tested this). Which cursor will this function move in this case?

@Districh-ru
Copy link
Collaborator Author

Districh-ru commented Jul 7, 2025

I strongly doubt that SDL_WarpMouseInWindow() will somehow help if there is actually no mouse in the system (and its driver is not running). Even in Windows, the system mouse cursor disappears completely when the mouse is disconnected from the USB port (at least in Windows 10, I just tested this). Which cursor will this function move in this case?

Agree. If there is no cursor then there is nothing to move and the only way is to use software cursor.

I'll add the corresponding comments about why we must use hardware cursor for touchscreen devices.

I did a test for Android and it acts the same as Windows - the system (hardware) cursor is visible only if mouse if plugged.

@oleg-derevenetz
Copy link
Collaborator

Hi @Districh-ru what do you think about making the software cursor the only option (or perhaps a default with config file-only configuration)? Pros of the software cursor:

  • It is scaled along with the game area itself (so it has an adequate size in case of xN resolutions);
  • It does work even if there is no "real" mouse.

Cons of the software cursor:

  • It can move sloppily when event processing loop is busy (e.g. during the AI turn);
  • ???

@Districh-ru
Copy link
Collaborator Author

Districh-ru commented Jul 8, 2025

Hi @oleg-derevenetz, I prefer to keep as it is now - "software" default with config file-only configuration.
I'll remove this option from this PR (probably tomorrow).
I use hardware cursor when I play the game at 1366x768 resolution maximized to 1920x1080 and I have not so "small" game image and 1:1 unscaled cursor. It looks good for me. :)
And, yes, it moves smooth during AI turn.

But I'm also OK with leaving only software cursor. We can discuss it separately with the team and active players and remove the hardware cursor ability from the engine in a new PR if we decide to do this.

@Districh-ru Districh-ru modified the milestones: 1.1.10, 1.1.11 Jul 15, 2025
@Districh-ru Districh-ru changed the title Add Screen Scaling, Cursor Renderer and Interface Type settings to the game settings dialog Add Screen Scaling Type to graphics settings dialog and Interface Type to the game gettings dialog Jul 19, 2025
@Districh-ru Districh-ru marked this pull request as ready for review July 19, 2025 13:19
@Districh-ru Districh-ru requested a review from zenseii July 19, 2025 13:20
@zenseii zenseii changed the title Add Screen Scaling Type to graphics settings dialog and Interface Type to the game gettings dialog Add Screen Scaling Type to graphics settings dialog and Interface Type to the game settings dialog Jul 19, 2025
@Districh-ru
Copy link
Collaborator Author

Hello @ihhub, @oleg-derevenetz and @zenseii!
From tomorrow until August 6th I will be on vacation and away from the internet.
If you find any issues in this PR or want to apply changes, you are free to commit to this PR.

@zenseii
Copy link
Collaborator

zenseii commented Jul 27, 2025

Enjoy your vacation, @Districh-ru !

@ihhub
Copy link
Owner

ihhub commented Jul 27, 2025

Hello @ihhub, @oleg-derevenetz and @zenseii! From tomorrow until August 6th I will be on vacation and away from the internet. If you find any issues in this PR or want to apply changes, you are free to commit to this PR.

Have a nice vacation!

@Districh-ru
Copy link
Collaborator Author

Thanks guys!

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.

Technically this PR looks okay to me, I'm also fine with proposed GUI solutions (although I suspect they may cause a debate).

@oleg-derevenetz oleg-derevenetz requested a review from ihhub August 5, 2025 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement New feature, request or improvement ui UI/GUI related stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants