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

[OSX] Render screen in full native resolution on HiDPI displays. #8519

Merged
merged 3 commits into from Feb 14, 2021

Conversation

michicc
Copy link
Member

@michicc michicc commented Jan 7, 2021

Motivation / Problem

On HiDPI displays, OTTD renders in virtual resolution, giving a permanent x2 scaling effect. This makes it impossible to e.g. get crisp text on HiDPI displays. OSX is the only platform that does this.

Description

Always use native resolution for rendering. x2 scaling can be achieved by using font/UI scaling and in-game zoom.

Limitations

Rendering in full native resolution can affect performance, there are more pixels after all. This PR adds an OSX only (non-GUI) setting to restore the old behaviour for cases where OTTD struggles to render in native resolution.

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

@michicc michicc added the OS: MacOS This issue is related to a Mac OS problem label Jan 7, 2021
@andythenorth
Copy link
Contributor

andythenorth commented Jan 7, 2021

First observation is zoom 😸
8519-zoom!

@LordAro
Copy link
Member

LordAro commented Jan 8, 2021

have the climate icons always been that small?

@michicc
Copy link
Member Author

michicc commented Jan 8, 2021

@LordAro There's a bug with main menu GUI scaling, when you reduce the scaling factor form the settings menu. I've noticed it, but haven't done any digging yet.

@nielsmh
Copy link
Contributor

nielsmh commented Feb 6, 2021

Is there a chance this could affect/be part of the cause for #7644 and #8300 ?

@michicc
Copy link
Member Author

michicc commented Feb 6, 2021

Unlikely. Test reports on IRC agreed on: slower, as expected. The slowdown did vary individually, of course.

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.

Codewise this looks fine by me; I understood people reported worse performance with this PR. Should we disable it by default for now? Or how do we want to deal with it .. as to me, it sounds like the right thing to do, to do support it. But we also have to overcome the performance reports somehow :)

@michicc
Copy link
Member Author

michicc commented Feb 13, 2021

OpenTTD has to generate four times the amount of pixels, so yes, a performance impact is entirely expected. The question (which has to be answered by users) is simply if the performance impact is worth it.

The added setting (settings are discoverable, driver params are not) is exactly because of that. I'm entirely open to defaulting to the current scaling behaviour.

@orudge
Copy link
Contributor

orudge commented Feb 13, 2021

I've tested it, and while there is a performance impact, I imagine it would be mostly noticeable if people are fast-forwarding. Depends on the complexity of the game of course. Quality looks good, especially with the native font rendering merged in too.

I'd be happy for it to go in, but would think it might be best to disable it by default.

@michicc
Copy link
Member Author

michicc commented Feb 13, 2021

Disabled by default is probably equal to nonexistent for most people. Precedent would be that we display in native resolution on Win32 as well.

The setting really is only for the cases where the performance impact is unacceptable IMHO.

@orudge
Copy link
Contributor

orudge commented Feb 14, 2021

Disabled by default is probably equal to nonexistent for most people. Precedent would be that we display in native resolution on Win32 as well.

I hadn’t spotted your other PR with support for autoscaling the UI elements at the time. With that included, then rendering in HiDPI by default seems good to me.

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.

You are right @michicc ; making it false by default is pointless and we might as well thrown away the PR.

Mostly, this comes out of angst for more people complaining macOS is slow; but we have to deal with that sooner or later anyway, and hiding for that is not a solution either :D So I was wrong, and it was a stupid suggestion of mine :P Let's do this :D

@michicc
Copy link
Member Author

michicc commented Feb 14, 2021

Will update the auto-zoom PR after merging this.

@michicc michicc merged commit e5c3253 into OpenTTD:master Feb 14, 2021
@michicc michicc deleted the pr/osx_hidpi branch February 14, 2021 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS: MacOS This issue is related to a Mac OS problem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants