fix: 修复调用 delay_ms 时出现帧率误差较大的问题 #257
Conversation
WalkthroughThe pull request introduces new function declarations and refactors existing functions across several files. In Changes
Sequence Diagram(s)sequenceDiagram
participant Frame as Frame Render Event
participant Updater as updateFrameRate()
participant State as Frame Rate State
Frame->>Updater: call updateFrameRate(addFrameCount)
Updater->>State: update frame count & elapsed time
alt Elapsed time > 0.5 sec
Updater->>State: calculate and update frame rate
else
Updater-->>Frame: no update performed
end
Frame->>Updater: call getfps()
sequenceDiagram
participant Caller as UI/Event Loop
participant Delay as delay_ms()
participant Sleep as ege_sleep()
participant GUI as guiupdate()
participant Graph as graphupdate()
Caller->>Delay: call delay_ms(ms)
Delay->>Delay: calculate targetTime
alt Current time < targetTime
Delay->>Sleep: sleep for remaining time
end
Delay->>GUI: invoke guiupdate()
Delay->>Graph: invoke graphupdate(pg)
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/time.cpp (2)
73-73: Watch out for potential floating-point precision issues.
While storing the target delay time as a double is fine for most use cases, consider edge cases or large values ofmsthat could lead to precision loss.
91-94: Consider logging or validating negativemsinputs.
Currently, negativemsis silently ignored. If this is intentional, it's fine; otherwise, you could log or raise an error to alert callers of invalid arguments.src/ege_time.h (1)
9-10: Add some documentation toupdateFrameRate.
Consider adding a comment to explain how this function impacts the frame rate logic and what theaddFrameCountparameter does.src/ege_graph.h (1)
25-28: Add documentation for the new functions.
BothneedToUpdateandgraphupdatewould benefit from brief doc comments explaining their expected usage and return values, enhancing maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/ege_graph.h(1 hunks)src/ege_time.h(1 hunks)src/egegapi.cpp(1 hunks)src/graphics.cpp(2 hunks)src/time.cpp(3 hunks)
🧰 Additional context used
🪛 Cppcheck (2.10-2)
src/graphics.cpp
[error] 199-199: Signed integer overflow for expression '0xFF<<24'.
(integerOverflow)
🔇 Additional comments (16)
src/time.cpp (10)
63-66: Good job adding descriptive doc comments.
These lines provide clarity about the function's purpose and parameters, improving maintainability.
75-76: Doc comment and GUI update call look fine.
The inline documentation clarifies when and whyguiupdateis invoked, which is helpful for future maintainers.
78-82: Verify pointer safety forroot.
Ifpg->egectrl_rootcould ever be null, this sequence would dereference a null pointer. Ensure it's never null or add a safeguard check.
84-84: Clear inline documentation for delay section.
This comment block accurately describes the delay logic and helps new contributors quickly grasp the intention.
85-88: UsingSleep(0)to yield the CPU is acceptable.
This approach relinquishes CPU time on zero-millisecond requests while still performing event handling immediately afterward.
104-107: Integer overload fordelay_fpslooks good.
Forwarding the call to the double-based implementation ensures consistent behavior and avoids duplicated code.
109-112:longoverload maintains consistency.
Again, forwarding to the double-based version is a clean approach for code reuse.
157-160: Integer overload fordelay_jfpsfollows the same pattern.
This helps preserve uniform logic and consistent usage hints across the codebase.
162-165:longoverload fordelay_jfpsis structured consistently.
No issues here; the approach mirrors your other overload designs.
198-198: Clarify whyupdateFrameRate(false)is used.
CallingupdateFrameRate(false)omits incrementing the frame count. Verify that this is intentional and does not cause unintended behavior in FPS calculations.src/graphics.cpp (2)
185-188: LGTM! Good encapsulation of update conditions.The function nicely encapsulates the update conditions into a reusable function, improving code readability.
190-230: LGTM! Improved frame rate handling based on window visibility.The function now correctly handles frame rate updates by:
- Incrementing frame count only when the window is visible
- Still updating frame rate when window is not visible, but without incrementing frame count
This change helps fix frame rate deviation issues by ensuring accurate frame counting.
🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 199-199: Signed integer overflow for expression '0xFF<<24'.
(integerOverflow)
src/egegapi.cpp (4)
3061-3064: LGTM! Well-structured frame rate tracking variables.The static variables are well-named and provide clear separation of concerns for frame rate tracking.
3071-3098: LGTM! Robust frame rate calculation implementation.The function implements a solid frame rate calculation strategy:
- Uses 0.5s update periods for stable measurements
- Separates frame counting from timing updates
- Uses high-precision timer for accurate timing
This implementation helps fix frame rate deviation issues by providing more accurate and stable measurements.
3100-3106: LGTM! Clean frame rate state reset.The function properly resets all frame rate tracking variables to their initial states.
3108-3111: LGTM! Efficient frame rate access.The function now efficiently returns the pre-calculated frame rate instead of computing it on each call.
|
当 delay_ms 延时较长时，内部可能会进行多次交换帧缓冲操作，每次都会计一次帧数，造成帧率变大。因此调整 delay_ms 行为，只进行一次交换帧缓冲操作。 |
|
在延时里面更新 GUI 属实不妥，GUI 在应用层次应该是更上层的东西，在这里插入有点混乱，GUI 层应该自己提供更新接口， 由用户调用。等后面加入新的UI库后再将其去除。 |
Summary by CodeRabbit
New Features
Refactor