Page MenuHomeWildfire Games

Target FPS in menus and running games
ClosedPublic

Authored by elexis on Jan 30 2017, 4:07 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 4, 4:12 AM
Unknown Object (File)
Wed, Oct 2, 11:21 AM
Unknown Object (File)
Fri, Sep 20, 8:23 AM
Unknown Object (File)
Sun, Sep 15, 7:26 PM
Unknown Object (File)
Sun, Sep 15, 8:18 AM
Unknown Object (File)
Fri, Sep 13, 3:40 PM
Unknown Object (File)
Wed, Sep 11, 7:02 PM
Unknown Object (File)
Sun, Sep 8, 4:27 AM

Details

Summary

Use case:
Since the renderer tries to display as many frames as possible, it will always induce 100% CPU workload on that core without benefit.
Not a direct concern of 0A.D., but it can also overheat hardware if throttling was disabled (as experienced by MojoRising420).

Implementation:
rP16927 / #2882 addressed this issue for main menus. But it is insufficient:

  1. It doesn't apply to running games.
  2. It doesn't allow the user to chose a target FPS.
  3. It renders about 20% too few FPS when picking a higher framerate
  4. It doesn't actually limit the FPS, because it only considers the time consumed by the most recently rendered frame, while the framerate fluctuates.

The inaccuracy in 3. derives from the frequency filter framerates (SmoothedFrequency, StableFrequency, AverageFrequency).
Instead, measuring the milliseconds since the last call gives a correct result.

An actual FPS limiter might be useful as well, as there can be screen tearing with Vsync disabled and input lag with Vsync enabled.
Never seen any of either though, likely due to the window manager doing Vsync already.

Test Plan

To test the inaccuracy described in 3., do not apply the patch, but change the 50 to 100 in main.cpp.
Replace realTimeSinceLastFrame by 1.0 / g_frequencyFilter->StableFrequency() to see that this won't help.
Add a new function AverageFrequency to frequency_filter.cpp / .h to see that this doesn't improve either.
Apply the patch and enjoy a proper rate.
Notice that removing the wait > 0 check is fatal, as negative values converted to u32 will become a huge number, effectively freezing the app.
Observe that with the patch applied, the target FPS is reached with much greater precision, even if exceeding often.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1395
Build 2196: Vulcan BuildJenkins
Build 2195: arc lint + arc unit

Event Timeline

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/271/ for more details.

Some observations:

  • We should be unable to set negative values for FPS (and probably also unable to set very low values).
  • The FPS limiting should probably be disabled if VSync is enabled, otherwise it might interfere, and the options should also be greyed out in this case.
  • This could be able to use #2593 / #4039 when that is in.

We should be unable to set negative values for FPS (and probably also unable to set very low values).

We are able to type non-numerical strings, negative numbers and low numbers, since #4448 is not implemented.
But the code will just default to if a non-numerical value or number below 30 is given.
This must be the case as the user can also pass broken config values by editing the config file.

Furthermore typing 0 means disabling the option. If we force a value > 30 FPS, then we need to add another boolean option, which is also why a slider wouldn't be ideal, at least with the current option page.

The FPS limiting should probably be disabled if VSync is enabled, otherwise it might interfere, and the options should also be greyed out in this case.

How would it interfere? Why is that an issue?

In case VSync is used, as far as I'm aware VSync itself determines the frame rate (and, to add onto that, in case VSync is enabled there is no reason to limit the framerate).

"Adaptive FPS" might be a better wording than "Target FPS"

It actually limit FPS, because we just waiting until time will be divided by FDT since last frame began. So the only except case: when fdt > limited_fdt.

Also if it needs to use smooth fps, you always could use a queue or something like this.

binaries/data/mods/public/gui/options/options.json
216

Isn't it too hard for newbie players to understand this option?

source/main.cpp
195

Why it so high, I could set menu in [24, 25] fps. I suggest to use 20-24 as lower bound.

198

We could use chrono, if it needs a higher precision.

This revision now requires changes to proceed.Apr 2 2017, 9:08 PM

It actually limit FPS, because we just waiting until time will be divided by FDT since last frame began. So the only except case: when fdt > limited_fdt.

That iteration of the patch definitely didn't limit and had some bumps if the FPS changed ingame.
With the microsecond precision the difference isn't noticeable, so we can just call it a limiter, even if it isn't in all cases.

I've been waiting a long time to have a justification to post pornographic material (= reasoning why the ingame one is set to 60fps):
https://vimeo.com/197728547

  • This could be able to use #2593 / #4039 when that is in.

Done. Much saner, even if it can't display the range and currently selected value IMO

binaries/data/mods/public/gui/options/options.json
216

Yes, contains unneeded info

source/main.cpp
195

Set the lower bound to 20 for the engine

198

Came across that one when searching for something microsecond precise.
It seemed exotic, but since it does the job (much more stable FPS than with milliseconds) and since it's supported, lets go for it!

elexis edited edge metadata.
elexis marked 3 inline comments as done.

Use chrono for microsecond precision and the new slider GUI object, rephrase strings.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/950/ for more details.

Last version uploaded didn't work because of two failures:

  • g_LastFrameTime must be set to the actual current time, not the time before the delay.
  • That 10^6 should have been 10^3.

Fix another failure found by Vladislav:

  • Don't use floats when we want double.

Also:

  • Remove Min/Max from the tooltip, as that will become appended generically.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/961/ for more details.

About the microsecond precision: If we have 200 fps, then there will be 5 millisecond delay between consecutive frames. The greatest error will be 0,5, so 10%, so I'd say 10% more precision sounds good

elexis added inline comments.
source/main.cpp
196

we could also use int for fpsLimit and wait.
But using double for fpsLimit should avoid the int -> double conversion when dividing 1000.0 by this number.

Use .0 for comparison to avoid another type conversion.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/963/ for more details.

25 FPS minimum in both main.cpp and options.js
Disable if VSync is enabled, as proposed by Vladislav and echotangoecho

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/977/ for more details.

As reported by Vladislav:
Use same phrasing for slightly different options.json strings
20 instead of 25 min
Move the LimitFPS() call to the bottom of that function instead of doing it in the middle of the Frame()

Also:
Didn't I already claim to use doxygen?

Use static for g_LastFrameTime and remove the g_, because it's not really going to be used elsewhere, as pointed out by Vladislav.

I've tested it, it works for me, it has a small error enough: +/- 1 FPS.

This revision is now accepted and ready to land.May 4 2017, 1:35 AM
This revision was automatically updated to reflect the committed changes.

Thanks for all the elaborate reviews and cpp explanations for dummies!

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/982/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/983/ for more details.