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

Deduplicate tick-related code in video drivers #8703

Merged
merged 9 commits into from Feb 20, 2021

Conversation

TrueBrain
Copy link
Member

Contains #8702, as I was too lazy to do this work twice.

Motivation / Problem

During the 60fps it was found that all (!) video drivers have nearly the same GameLoop code, with some minor naming differences and some minor workflow differences.

This PR sets out to unify them further, and deduplicate the tick-related code.

Future extensions are helped by this, because now they can change in a single place how the game-ticks and draw-ticks behave. For example, if the game-tick takes too long, there would be a single place to make a choice what suffers: simulation time or draw-fps.

Description

This is mostly moving around code and making sure drivers do the same into the highest amount of detail.

Drivers I have tested and on what platform:

  • SDL (WSL2, X11)
  • SDL (Ubuntu 20.04, X11)
  • SDL (Ubuntu 20.04, Wayland)
  • Allegro (WSL2)
  • Dedicated server (WSL2)
  • Win32 (Windows 10)
  • Cocoa (macOS 10.15)

To me, they all behave identical. More testing is of course appreciated :)

Limitations

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')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

src/video/win32_v.cpp Outdated Show resolved Hide resolved
…ivers

It was of all the drivers the only one doing this slightly different.
When trying to unify more code, that was rather annoying.
Also move this function to be a class member. This to allow
further deduplicating of code in a later commit.
… is called

Additionally, make sure this is a class method. Later commits
will make use of this.
These were special settings only for the win32-drivers, and
introduced in the very first version we track.

Time kinda had caught up with those variables, so it is time to
say farewell.

force_full_redraw was most likely a debug functionality "in case
our dirty-rect fails". This should no longer be needed.

display_hz was cute, as it had a max of 120. That is kinda
out-dated information, but I also doubt anyone was really using
this.
They were all identical, so better put this in a single place
hoping it is less likely to break.
Copy link
Member

@michicc michicc left a comment

Choose a reason for hiding this comment

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

Looks okay and is confirmed to work for win32.

@TrueBrain TrueBrain merged commit 058f13b into OpenTTD:master Feb 20, 2021
@TrueBrain TrueBrain deleted the video-merge branch February 20, 2021 16:08
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

3 participants