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

Fix game scaling #910

Merged
merged 14 commits into from
Nov 24, 2024
Merged

Fix game scaling #910

merged 14 commits into from
Nov 24, 2024

Conversation

royfalk
Copy link
Contributor

@royfalk royfalk commented Nov 22, 2024

Close #913.

Please answer the following:

Code Changes:

Issues:

  • When specifying the monitor resolution but in window mode, edges of he game appear cropped.

@royfalk royfalk self-assigned this Nov 22, 2024
@evertvorster
Copy link
Contributor

Hi there!

I compiled and installed this branch, then ran vsettings and picked a resolution that is less than my computer's native resolution. I also selected fullscreen 32bit mode.

When running vs, it still pops up on the wrong screen, and does not fit onto that screen. Then, when I drag the game screen onto my main screen, it displays the game screen at the native resolution, unscaled, and has black bars on the top and right hand side up to the native resolution of the screen.

For what its worth, I am running Wayland. Emulators like DosBox-x, which also used SDL, has no problem scaling to the proper resolution, and also pops up on the right monitor.

@evertvorster
Copy link
Contributor

I tried a few different screen sizes with vsettings, and it does not appear to have an effect on the display resolution.

@evertvorster
Copy link
Contributor

I edited config.json manually, and with this I can adjust the screen resolution.
It would appear that the vsettings does not apply changes to the config.json file.

I don't see how it could, because config.json is installed in a system directory, and user space is not able to modify it without escalating priveleges.

What is the plan for the future or this file? With the normal vegastrike.config, there is a copy in the user's home directory, in the subdirectory .vegastrike. This file can be modified in userspace by the user, or applications that the user runs.

Something similar could work for config.json.

Also, some instructions on each configuration option in config.json as it is added.

@royfalk
Copy link
Contributor Author

royfalk commented Nov 22, 2024

I edited config.json manually, and with this I can adjust the screen resolution. It would appear that the vsettings does not apply changes to the config.json file.

That is correct. I disabled the settings app functionality a few commits ago.

What is the plan for the future or this file? With the normal vegastrike.config, there is a copy in the user's home directory, in the subdirectory .vegastrike. This file can be modified in userspace by the user, or applications that the user runs.

The plan is to modify the settings app (hopefully next week) to also write to config.json. It should also solve #882.

Also, some instructions on each configuration option in config.json as it is added.

I thought it was mostly self explanatory.

The important thing for now and this PR is whether it solves this specific issue.

@evertvorster
Copy link
Contributor

Hi there!
Will there be a seperate config.json in the user's directory? Does the game engine check for that and interpret it first?

Currently on my system there is only /usr/share/vegastrike/config.json, and that is not modifyable by a non-root user.

What might be self-explanatory to you could be completely non-obvious to someone else. .json is already very new to me.
I don't know which variables to use, or which ranges each variabble accepts. Now, if the plan is to have the settings app write to the config.json, and the user is not expected to edit it, that is fine.

If, on the other hand the user will be expected to edit the config.json, then there needs to be some comments in the file specifying which variables can be changed, and what the legal ranges for those variables are.

On to the point of this PR.
Scaling does not appear to work at all, and any resolution is mapped unscaled. Put in another way, if I set the resolution to something smaller than my monitor, it displays smaller with black bars. If I set the resolution to larger than the screen area, the game screen is cropped.

My setup is a 32" 4k monitor that is attached to my laptop's external HDMI port. It's scaled with Wayland by 150%.
When it is connected, it acts as my primary display. It is to the right of the laptop.

@royfalk
Copy link
Contributor Author

royfalk commented Nov 24, 2024

This does not fix #883 but rather #913. Please review it accordingly.

@evertvorster
Copy link
Contributor

The /usr/share/vegastrike/config.json is overwritten every time a new copy of Assets are installed.
This is the same as /usr/share/vegastrike/vegastrike.config.

When the game engine starts up, it checks for vegastrike.config in a few locations in a specific hierarchy.
So, if the user has a different config than the default /usr/share/vegastrike/vegastrike.config, the configuration file needs to be higher up in the hierarchy. The vsettings app also saves the custom config into the users home directory, which is higher up in the hierarchy than the default configuration file.
This config file then survives re-installs of the Assets.

Is there a similar mechanism in place for config.json?
ie: Can I make a copy of config.json in my ~/.vegastrike directory and the game engine will use that one rather than /usr/share/vegastrike/config.json?

@royfalk
Copy link
Contributor Author

royfalk commented Nov 24, 2024

Is there a similar mechanism in place for config.json?

Not at the moment. Please document this in an issue or in #912. I'll add that to the python settings app.

I'm really playing catch-up here. I've spent the last few years touching the unit code in the engine. Before today, I haven't ran vegasettings for several years. Your insights and assistance is very helpful here.

@evertvorster
Copy link
Contributor

OK, setting the resolution in two places has some interesting effects.

When changing the resolution in config.json, the game screen does indeed change size in accordance with the settings.
There does not seem to be any scaling applied. ie: when selecting a resolution of 1920x1080, only a small part of the 3200X1800 native resolution of the screen is painted on, roughly the size of what I would expect 1920x1080 to be.

Usually selecting "Fullscreen" also means, fill the entire screen, and so I was expecting some scaling to happen so as to stretch the game screen to my monitor's native resolution.

Anyways, when I set the resolution in config.json to be 3200X1800, the game screen nicely fills my monitor.

Then I use the vsettings app to change the screen size in vegastrike.config.
When setting a really low resolution, the game screen still fills the entire screen, but the text is of a much smaller pitch.
When using a higher resolution, the text is a much larger point size.
In all cases, the game screen nicely fills my 3200x1800 monitor.

So, the game engine picks up the game screen resolution from config.json, and the text point size from vegastrike.config in my home directory.

@royfalk
Copy link
Contributor Author

royfalk commented Nov 24, 2024

So, the game engine picks up the game screen resolution from config.json, and the text point size from vegastrike.config in my home directory.

I wasn't aware of this. Usually I play in window mode and there the display breaks if the resolutions don't match.
I'll open another issue for font scaling. If this solves #913 , please approve.

Copy link
Contributor

@evertvorster evertvorster left a comment

Choose a reason for hiding this comment

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

It is indeed possible to change game resolution in config.json.

@royfalk royfalk merged commit f89b525 into master Nov 24, 2024
43 checks passed
@royfalk royfalk deleted the task_sdl_scaling branch November 24, 2024 15:06
g_game.x_resolution = 800;
g_game.y_resolution = 600;
g_game.x_resolution = 1920;
g_game.y_resolution = 1080;
Copy link
Member

Choose a reason for hiding this comment

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

These should probably be in variables/constants and match L2168. (1920, 1080) is outside the window defined by L2168.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The plan (fairly advanced actually) is to move all settings in the engine to an automatically generated and populated singleton. That would deprecate a huge chunk of code, from settings, configuration, all those XMLParser lines, etc.

The JSON parser would convert directly to the filetype and you'd work against a variable. https://github.com/royfalk/JsonStruct has the script to generate this.

Copy link
Member

Choose a reason for hiding this comment

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

@royfalk that's fine - just calling out the potential bug due to the the window size not matching here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's interesting. I guess I need to figure out what objconv does. It's not part of the main game,as far as I can tell. I'm also not convinced we are using glutInit in the actual game. I think both SDL and glut were supported and I picked SDL.

This lack of focus by the original devs... well.

Copy link
Member

Choose a reason for hiding this comment

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

At least when working with Qt, the coordinate system worked in two parts:

  1. Canvas that everything was drawn against
  2. View Port for what on the canvas to show the user
  3. Device Canvas that laid out the hardware

It gave some interesting effects. So technically, those coordinates exist but they may be outside the view port; but that's probably not what we want.

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.

Duplicate game resolution
3 participants