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

Feature: Choose a sensible window size on a fresh OTTD config file. #8536

Merged
merged 1 commit into from Jan 14, 2021

Conversation

@michicc
Copy link
Member

@michicc michicc commented Jan 8, 2021

Motivation / Problem

Most screens today are bigger than 640x480. Assume the user actually
wants to see OpenTTD on screen and try to make the window sized
to 75% of the screen.

Description

If the config file contains a resolution of 0x0 (which is also set as the default value), the video drivers will (where possible) set the actual resolution to 75% of the screen size.

The selected window size will be written to the config after the first start,
disabling this automatic on subsequent launches.

Each video driver must unfortunately duplicate the call to auto-resolution handling, as for example SDL functions can only be called after SDL_Init, which is only done on video driver load when we know SDL is going to be used.

If a video driver does not provide a screen resolution, the old default of 640x480 will be used.

Limitations

Missing implementation for SDL1, as I couldn't see a proper function for it.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')
@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jan 8, 2021

Why 75%? Most games these days open fullscreen borderless (windowed); we don't have a borderless mode, but why not just the biggest resolution windowed?

@michicc
Copy link
Member Author

@michicc michicc commented Jan 8, 2021

Hysterical raisins? OpenTTD has always defaulted to start windowed (where windows are a thing) and I'm keeping with that. Also, biggest windowed generally means having to use the OS maximize window function, making the whole setting thing more complicated as you need a valid resolution early on.

Copy link
Member

@TrueBrain TrueBrain left a comment

I like it :D We can debate settings all day, but in the end, I like this :D So see those remarks as a +0, not a -1 :)

src/openttd.cpp Outdated Show resolved Hide resolved
/*
* Get the resolution of the main screen.
*/
virtual Dimension GetScreenSize() const { return { 640, 480 }; }

This comment has been minimized.

@TrueBrain

TrueBrain Jan 8, 2021
Member

Shall we bump this res to 1024x768 or something while at it?

src/video/video_driver.hpp Outdated Show resolved Hide resolved
src/video/video_driver.hpp Outdated Show resolved Hide resolved
* internal drawing routines work correctly. */
Dimension res = this->GetScreenSize();
_cur_resolution.width = ClampU(res.width * 3 / 4, 640, UINT16_MAX / 2);
_cur_resolution.height = ClampU(res.height * 3 / 4, 480, UINT16_MAX / 2);

This comment has been minimized.

@TrueBrain

TrueBrain Jan 8, 2021
Member

here too, 1024x768? 1280?

@DeltaNedas
Copy link
Contributor

@DeltaNedas DeltaNedas commented Jan 14, 2021

i think 640x480 should be made into two macros at the top so its easier to change without sifting through everything

Most screens today are bigger than 640x480. Assume the user actually
wants to see OpenTTD on screen and try to make the window sized
to 75% of the screen.

The selected window size will be written to the config after the first start,
disabling this automatic on subsequent launches.
@michicc michicc force-pushed the michicc:pr/init_wnd_size branch from fc5f7dc to 02fcaa0 Jan 14, 2021
@michicc
Copy link
Member Author

@michicc michicc commented Jan 14, 2021

I've addressed the comments (pun intended). The default resolution is a constant now, but I didn't change it so far.

It is only used as a lower bound and as a fallback for those video drivers that don't have resolution detection. Right now this is Allegro and SDL1 (and of course null/dedicated), where I don't know on which kind of devices they are still used.

@michicc michicc merged commit fa60c1f into OpenTTD:master Jan 14, 2021
8 checks passed
8 checks passed
Emscripten
Details
Commit checker
Details
Check preview needs update Check preview needs update
Details
Linux (clang, clang++)
Details
Linux (gcc, g++)
Details
Mac OS (x64, x86_64)
Details
Windows (x86)
Details
Windows (x64)
Details
@michicc michicc deleted the michicc:pr/init_wnd_size branch Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.