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

Fixes for FPS display, for #117 #118

Merged
merged 8 commits into from
May 14, 2021
Merged

Fixes for FPS display, for #117 #118

merged 8 commits into from
May 14, 2021

Conversation

res2k
Copy link
Contributor

@res2k res2k commented Apr 28, 2021

Contains the following changes:

  • A fix in CL_Frame() causing from physics & refresh intervals in SYNC_ASYNC case
  • Display "refresh" FPS on screen instead of "client FPS" (the latter may run faster) - "refresh" FPS is probably what most people expect
  • Port a number of maxfps-related patches from Q2PRO. Notably, it contains a change to inform the user when the requested maxfps value can not be achieved exactly.

All in all, these changes should address the complaints from #117.

sthalik and others added 8 commits April 28, 2021 22:28
These only happen on connection/map change or maxfps
change.
There's a case with cl_async=0 and cl_maxfps=0 such
that physics run thousand times a second. This breaks
down movement due to possibly `cl_maxpackets', `rate',
or network bandwidth. This happens anytime the client
isn't hosting the game.

There's the same case with cl_async=1 but it's
unreachable due to `QVF_VIDEOSYNC' not getting set
anywhere.

Having removed the bug and dead code, we can see that
the `CL_Frame()' main loop can process several cases
through the same logic if it's set up right in
`CL_UpdateFrameTimes()'.

- Reduce branching in the main loop.
- Unify some cases that depended on stubbed code
  elsewhere and remove the stubs. There are more
  stubs that may complicate code paths elsewhere
  in the files that had the `VID_*' stuff stubbed.
- Remove the duplicate enum elements.
- Add some #define constants
- `cl_maxfps 120' is really 125 Hz by virtue of
  rounding done in `fps_to_msec'. Adjust the value.

Note that on Windows, SYNC_SLEEP_60 is inaccurate,
resulting in 50 Hz rather than 60. It shouldn't be too
tragic. While we could use `timeBeginPeriod()', let's
leave it at that.
Due to millisecond-precision timekeeping, {cl,r}_maxfps only has few
distinct actual values. Different values will round off to one of these.

I don't think it's practical to expect users to know that implicit
rounding behavior. It's also not practical to rework the timekeeping
code from the ground up.

Potentially emit the message on direct maxfps change and on startup.

v2:

- Prevent logspam on focus change.

v3:

- Prevent logspam on `developer=2'.
- Remove effectively dead debug code.
- In half-initialized state we may have "msec" equal to zero inside
  `warn_on_fps_rounding()' that needs guarding against. Happens when set
  during `autoexec.cfg' invocation. The same will happen when using
  cl_maxfps=0 for max Hz.

Thanks to @skullernet for extensive review.
In addition, cl_maxfps=0 w/ cl_async=1 now stands for
maximum allowed Hz.
Also fix bogus warning if cvar value is negative.
main_extra grows too fast, it contains previously accumulated frames as well.
This is probably what most people expect from an FPS counter.
@apanteleev apanteleev merged commit 212ba4e into NVIDIA:master May 14, 2021
@res2k res2k deleted the maxfps branch May 31, 2021 21:20
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.

4 participants