[rcore] Document that GetFPS() must be called every frame - #6120
Closed
burinc wants to merge 1 commit into
Closed
Conversation
GetFPS() is a sampler rather than a query. Each call takes at most one sample, gated on FPS_STEP, writes GetFrameTime()/FPS_CAPTURE_FRAMES_COUNT into a 30-slot ring and returns 1/sum-of-ring. So the ring needs 30 calls before the answer means anything, which per-frame calling reaches in half a second and a timer or a periodic summary never reaches at all. Called every frame, as DrawFPS() does and as every example does, it is correct. Called less often it silently returns a value that is too high by roughly the factor the ring is short: measured on device at a steady 17.02 ms frame time, one call per second from a cold ring gave 1773, 887, 591, 444, 354, 295, 253 and 221, which is 1773/n, while calling it every frame in the same process gave a steady 59. Nothing in the header said so, and the failure has no tell: no error, and a plausible number rather than an obviously wrong one. Comment only, no behaviour change. The rlparser outputs carry the header comment as the function description, so they are updated to match rather than regenerated, which keeps the diff to one line per file.
Owner
|
Thanks but I prefer to keep the comment simple, I think it's not a big deal. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GetFPS()is a sampler rather than a query, and the header does not say so.Each call takes at most one sample, gated on
FPS_STEP, writesGetFrameTime()/FPS_CAPTURE_FRAMES_COUNTinto a 30-slot ring, and returns1/sum-of-ring:So the ring needs 30 calls before
averageis the mean frame time. Callingevery frame reaches that in half a second. Calling from a timer, or from a
periodic summary, never reaches it: after n calls only n slots hold anything, so
it returns
1/(n * frame_time/30).Measured
An app with a steady 17.02 ms frame time, one binary, the call cadence being the
only variable:
The first row is
1773/nto within a percent at every point, which is the ringfilling one slot per call.
Called every frame, as
DrawFPS()does and as the examples do, it is correct.The reason this is worth a comment is that misuse has no tell: no error, and a
plausible-looking number rather than an obviously wrong one. It took a
frame-time series that stayed flat while the reported FPS decayed to notice.
The change
Comment only, no behaviour change:
The
rlparseroutputs carry the header comment as the functiondescription,so they are updated to match rather than regenerated. That keeps the diff to one
line per file; happy to regenerate them properly instead if you prefer.
Verified against
masterat9b2efc45, which is where this branches from.GetFPS()is byte-identical there and at 6.0. raylib still builds with thechange (comment only).
Entirely reasonable to close this if per-frame calling is considered obvious
enough not to document.