Implements mouse input scaling, auto screen scaling option

[zigmar]

• Scale mouse input coordinates to screen size, fixing input in scaled mode (Fixes Normalise the zoom on various screens. #222)
• Implement convenience option to auto-scale screen according to reference resolution (currently set to 1280x720) eliminating a need to manually calculate scale values. Enabled by default in non-fullscreen mode to improve first time experience.

[FilmBoy84]

Looks good to me

Probably my only thoughts on it are do we want the reference resolution at 1280x720p (16:9 Half-HD)?

Default assets assume 4:3 Screen scaled from 640x480 - the next 4:3 standards are SVGA @ 800x600, XGA @ 1024x768 and XGA+ @ 1152x864

The first whole number (2:1) scale-up at a 4:3-aspect is 1280x960 - but this is none-standard (although it will fit on a 1920x1080 HD display it wouldn't work on a 720p display)

I can see why using 16:9 @ 1280x720 is a good go-to - It's a high-dpi widescreen monitor standard and is in an aspect ratio that will make most people happy without having to change settings

Although is there a better resolution we're missing that's less destructive to the original assets in the shift from 4:3 to 16:9?

I'm inclined to leave as is - this comment is more to push the boat out there with regards to thinking about resolutions and not stretching assets

As an example, with the current method and default, using 4:3 resolutions stretches and condenses things oddly (these examples are at 1280x960)

EDIT: One other thing - the scaling options need adding to the launcher before merge so end users can make quick changes without having to dive into the .conf

[zigmar]

@FilmBoy84 Thanks a lot for a thorough for the review! Kudos for bringing up this resolution issue, after some thoughts and few more tests, I realized I made a mistake. When working on it I first tried 4:3 resolution as reference which I assumed would be correct based on original game, but got a stretched graphics and naively assumed the engine somehow expects 16:9. Turned out I got distortion because I myself set windows resolution values corresponding to 16:9. So I think the scaling should be proportional to windows resolution preserving it's aspect ratio, otherwise there will be distortion. What I'm thinking to do is to calculate the scaling based only on one dimension (either horizontal or vertical) and use it for both dimensions to preserve aspect ratio. WDYT?

Re: launcher, I thought about adding it there but wasn't sure: thought users probably would rarely need to turn it off, as on modern screens (in windowed mode) without scaling game probably will be too small to be playable. But it is very easy to add of you think it is useful. Another idea: maybe I could combine several options in a launcher and make a drop down menu which will allow to select "Scaling" between "None", "Auto" and couple of predefined settings (e.g. 200%, 300%, 400%).

[zigmar]

Ok, I fixed the aspect ratio issue (now it always respects window size) and added UI to choose between several scaling options.

Also have a question: might it be a good idea to enable/allow auto scaling in full screen as well? Currently setting low resolution in fullscreen implicitly does the scaling, but if for whatever reason a user decides to run fullscreen in higher resolution, scaling might come handy. WDYT?

[FilmBoy84]

Many thanks for the update, I will get a build together now and test

Looks like you have a formatting error somewhere though - Lint is kicking up one for Clang/Tidy

I'd certainly enable scaling for fullscreen in a similar manner to what you have for windowed (with the option to turn off) if only because we get a lot of discord feedback asking how to enable fullscreen and then questions about "things being too tiny" 😆

The values of None/Auto/100/200/300 for scaling options seem like a good start - not tested yet of course, but can power-users specify values beyond this? Or is it rigid to nearest hundred?

[JonnyH]

Had a quick glance and looked OK, but will do a proper review later.

One thing - is there any chance you can split this into 2 things? One PR to fix input event scaling, and another to enable autoscaling? We might need to discuss how to implement the second (what defaults to use, how to expose it in the ui etc) but that shouldn't block the first.

And we use clang-format - we ship a .clang-format config file in the git repository, and depending on your workflow it's normally a good idea to find a plugin for your ide/look at the "format-sources" targets in cmake/just run it manually before committing. Annoyingly, there's no guarantee of stability of output for clang-format between versions - the lint machine currently uses clang-format-10, so if you have the opportunity to install branched versions on your system matching that will make things easier.

Though those differences tend to be small on a few specific patterns that don't seem to be common - the lint output should output a diff format of what it expects so you can use that (either manually or just copy into patch) to fix small things up.

[zigmar]

Thanks @FilmBoy84 and @JonnyH.

1. Fixed the lint error. Not sure what happened as the error wasn't even in the code that changed and I did run format-sources (though I discarded format fixes unrelated to my changes).

2. can power-users specify values beyond this
   They can via config file but not UI. I could have added ability to manually edit old ScaleX/ScaleY values but they are quite unintuitive (e.g. scaling UI by factor of 3 requires setting scale value to 33) and I'm not sure every single option should be in launcher cluttering the interface. So I've implemented a convenient shortcut with few useful values, which feels like a good compromise between usability and choice. And we can easily additional predefined values if needed.

3. I'd certainly enable scaling for fullscreen (...) if only because we get a lot of discord feedback
   Sounds logical, if other stakeholders don't mind, I can enable. However I'm a bit hesitant, as fullscreen does not work at all on my machine and I can't properly test it.

4. is there any chance you can split this into 2 things
   Splitting PR would be somewhat cumbersome and I prefer to avoid it if possible. I can disable auto scaling by default if it helps and/or we can continue to discuss implementation on this PR.

[FilmBoy84]

Thanks for the continued updates

A couple of observations on the latest build

The launcher options are fine - simple to use for an end user - thanks for implementing

I notice that, when using the config to set 150% scale, you need to input 66 as the value - this seems counter intuitive - whilst mathematically it makes sense it may confuse some users.

For that reason; it may be worth implementing a check for those users who input a percentage and make the default expected value for ScaleX and ScaleY percentage based instead of using the real scaler value. Otherwise I can forsee people going "I wanted 175% so i put that in the conf" and much end-user confusion 😆

With regards to fullscreen, if you cannot test, no worries, feel free to make the changes to enable scaling for fullscreen and I can test for you. I assume you are on multiple monitors? (where we know there are some issues with fullscreen)

Also, if the user puts in a scaler value that is not compatible with their monitor or resolution, then OpenApoc crashes without warning currently - we probably need an "incorrect scale value" type prompt instead of a CTD with no log

And example of how to recreate this crash - Set the resolution to 1280x960, windowed, set the scaler to 300% in the launcher (or <50 in the conf) and run the game, it will load the window (filled white) then crash

Thanks once more for your work on this

[zigmar]

Good job catching that crash. Turned out there was already an old code which should have prevented scaling too much, but the check itself had a bug which caused the crash. Fixed that.

I thought about changing ScaleX and ScaleY to something more intuitive (there even a comment in the code suggesting to do so), but the only reason I didn't is that change will break stored configuration for existing users. Another option is to remove old config params entirely and create new ones with new names. Though I will suggest doing it as different PR, as this one is getting somewhat big.

Thanks a lot for volunteering to test full screen mode, I've enabled auto-scaling for full screen in latest commit, let me know if it works as expected. I indeed run a setup with multiple monitors + UI scaling + Linux + AMD graphics card. So there a lot of things which might get wrong and cause fullscreen to glitch out. Maybe I'll take a look as one of the next tasks.

[FilmBoy84]

Agreed, changing the way we deal with scaling is best left for a future PR

Just testing fullscreen now and all appears to check out in the build

Got a few things to do to try and break it, but if those fail to break it, I'm good to merge and deal with other features relating to scaling in future PRs :)

[FilmBoy84]

Good news! I cannot break it in fullscreen, so merging now

Thanks for all your work on this
I've added you to the project credits here
https://www.ufopaedia.org/index.php/Credits_(OpenApoc)

[zigmar]

Oh, shoot. I'm really sorry, but I only now realized I made a blunder, in the last commit I accidentally committed debug change that I shouldn't: https://github.com/OpenApoc/OpenApoc/pull/1040/files#diff-27613e54f6b1b2fe0db47131fd46b3f8ed10dd75e6d5902d75a590c79bcd39d2L18

Should I submit a new PR with the fix or you want to revert this file yourself?

And while on the topic, what is correct way to enable debug logging?

[FilmBoy84]

@zigmar if you can submit a new PR for a fix I can quickly merge
I've buggered up reverting changes to Master before
So probably best to play safe 😁

As to the correct way to enable debug logging - may be a question for @JonnyH

[zigmar]

Created a PR with the revert: #1041
Sorry again about the hassle

[JonnyH]

All config settings can be set on the command line when running openapoc itself - try running with "--help" to see the big list.

There are integer log levels for outputting to the log file and when to put a dialog box up - e.g. the help for the log file level:

--Logger.FileLevel arg (=3) Loglevel to output to file (0 = nothing, 1 =
error, 2 = warning, 3 = info, 4 = debug)

So running "./OpenApoc --Logger.FileLevel=4" will output all messages (from the "debug" loglevel and up) to the log file.

The stderr isn't really set-able yet, as I saw that as only useful for fatal errors like failing to write to the log file itself. We could add that as a config option, but it may be of limited usefulness. (e.g. on windows even a few hundred lines/frame ends up significantly slowing the entire thing down due to synchronous logging and super slow console)