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

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 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

Choose a reason for hiding this comment

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

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 }; }
Copy link
Member

Choose a reason for hiding this comment

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

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);
Copy link
Member

Choose a reason for hiding this comment

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

here too, 1024x768? 1280?

@deltanedas
Copy link
Contributor

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
Copy link
Member Author

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
@michicc michicc deleted the pr/init_wnd_size branch January 14, 2021 20:53
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.

None yet

4 participants