Fpscounter #287
Fpscounter #287
Conversation
RenderDebugInfo (FPS, mouse position, camera angles) is now: - Always on in Debug builds - Off by default in Release builds, toggle with $fpscounter on/off The $fpscounter command is checked before the $fps prefix to avoid a crash from trying to parse "ounter on" as a float.
Show avg FPS, max FPS, 1% low, 0.1% low, frame time in ms. Render a color-coded frame time bar graph (green/yellow/red) with a 60fps reference line.
… overlay Address PR review feedback: replace extern declarations with header include, extract magic numbers to named constants, use std::accumulate, fix highest FPS to track instantaneous values, rename misleading 0.1% low to slowest frame, fix frame time consistency.
Move glBegin/glEnd outside the frame bar loop to reduce draw calls. Use else-if chain in CheckCommand to skip unnecessary comparisons.
Summary of ChangesHello @Mosch0512, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the in-game performance monitoring by introducing robust tools for analyzing frame rates and consistency. It provides both a basic FPS display and a comprehensive debug overlay, offering critical metrics like 1% low FPS and a visual frame time graph. These additions empower users and developers to gain a deeper understanding of performance characteristics and diagnose issues more effectively. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great addition, providing valuable performance monitoring tools. The implementation of the FPS counter and the detailed debug overlay with frame statistics is well-structured and fits within the existing architecture. The new chat commands make these features easily accessible. My review includes a few suggestions to improve maintainability and safety, such as using safer string formatting functions, refactoring command handling for better scalability, and using named constants for UI colors. Overall, this is a solid contribution.
| static float sorted[FRAME_HISTORY_SIZE]; | ||
| memcpy(sorted, s_frameTimesMs, sizeof(float) * s_frameCount); | ||
| std::sort(sorted, sorted + s_frameCount, std::greater<float>()); |
There was a problem hiding this comment.
The use of a C-style static array sorted and memcpy is functional but less idiomatic and safe than modern C++ alternatives. Consider using std::vector for the sorted data. A static std::vector would prevent reallocations on every call, and you could use std::copy or its constructor for safer data transfer. This would align better with the use of other <algorithm> features in this function and improve type safety.
static std::vector<float> sorted;
sorted.assign(s_frameTimesMs, s_frameTimesMs + s_frameCount);
std::sort(sorted.begin(), sorted.end(), std::greater<float>());| glColor4f(0.0f, 0.0f, 0.0f, 0.5f); | ||
| glBegin(GL_QUADS); | ||
| glVertex2f(gx, glBottom); | ||
| glVertex2f(gx + gw, glBottom); | ||
| glVertex2f(gx + gw, glTop); | ||
| glVertex2f(gx, glTop); | ||
| glEnd(); |
There was a problem hiding this comment.
The colors for the frame graph background, lines, and bars are hardcoded directly in the rendering calls (e.g., glColor4f(0.0f, 0.0f, 0.0f, 0.5f)). To improve readability and make it easier to tweak the UI theme later, it's better to define these colors as named constexpr constants at the top of the file, alongside the other DEBUG_* constants.
For example:
static constexpr float GRAPH_BG_COLOR[] = {0.0f, 0.0f, 0.0f, 0.5f};
and then use it like glColor4fv(GRAPH_BG_COLOR);.
| swprintf(szLine, L"FPS: %.1f Avg: %.1f Max: %.1f Vsync: %d CPU: %.1f%%", | ||
| FPS_AVG, s_avgFps, s_highestFps, IsVSyncEnabled(), CPU_AVG); |
There was a problem hiding this comment.
The use of swprintf with fixed-size buffers (e.g., szLine) is unsafe and can lead to buffer overflows. It's recommended to use the safer swprintf_s function throughout this file (in RenderDebugInfo and RenderFpsCounter) to prevent such vulnerabilities. This requires passing the buffer size as an argument.
swprintf_s(szLine, L"FPS: %.1f Avg: %.1f Max: %.1f Vsync: %d CPU: %.1f%%",
FPS_AVG, s_avgFps, s_highestFps, IsVSyncEnabled(), CPU_AVG);| bool CmuConsoleDebug::CheckCommand(const std::wstring& strCommand) | ||
| { | ||
| if (strCommand.compare(0, 4, L"$fps") == 0) | ||
| if (strCommand.compare(L"$fpscounter on") == 0) | ||
| { | ||
| SetShowFpsCounter(true); | ||
| return true; | ||
| } | ||
| else if (strCommand.compare(L"$fpscounter off") == 0) | ||
| { | ||
| SetShowFpsCounter(false); | ||
| return true; | ||
| } | ||
| else if (strCommand.compare(L"$details on") == 0) | ||
| { | ||
| SetShowDebugInfo(true); | ||
| return true; | ||
| } | ||
| else if (strCommand.compare(L"$details off") == 0) | ||
| { | ||
| SetShowDebugInfo(false); | ||
| return true; | ||
| } | ||
| else if (strCommand.compare(0, 4, L"$fps") == 0) | ||
| { | ||
| auto fps_str = strCommand.substr(5); | ||
| auto target_fps = std::stof(fps_str); | ||
| SetTargetFps(target_fps); | ||
| return true; | ||
| } | ||
|
|
||
| if (strCommand.compare(L"$vsync on") == 0) | ||
| else if (strCommand.compare(L"$vsync on") == 0) | ||
| { | ||
| EnableVSync(); | ||
| SetTargetFps(-1); // unlimited | ||
| ResetFrameStats(); | ||
| return true; | ||
| } | ||
|
|
||
| if (strCommand.compare(L"$vsync off") == 0) | ||
| else if (strCommand.compare(L"$vsync off") == 0) | ||
| { | ||
| DisableVSync(); | ||
| ResetFrameStats(); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
The current command handling uses a long if-else if chain. This pattern can become difficult to manage and extend as more commands are added. Consider refactoring this into a more scalable approach, such as using a std::map to map command strings to their handler functions.
For example:
// In your class definition
std::map<std::wstring, std::function<void(const std::wstring&)>> m_commandHandlers;
// In constructor, register commands:
m_commandHandlers[L"$fpscounter on"] = [](const auto&){ SetShowFpsCounter(true); };
m_commandHandlers[L"$fps"] = [](const std::wstring& cmd) {
auto fps_str = cmd.substr(5);
auto target_fps = std::stof(fps_str);
SetTargetFps(target_fps);
};
// ... other commands
// In CheckCommand:
for(const auto& pair : m_commandHandlers)
{
// Use rfind for prefix matching
if (strCommand.rfind(pair.first, 0) == 0)
{
pair.second(strCommand);
return true;
}
}
2 participants
This pull request significantly enhances the in-game performance monitoring capabilities by introducing both a basic FPS counter and a comprehensive debug overlay. The detailed overlay provides crucial metrics like 1% low FPS and a visual frame time graph, offering a much deeper understanding of performance consistency beyond just average frame rates. These new tools are easily accessible and controllable via chat commands, enabling developers and users to quickly diagnose and analyze performance characteristics.
Highlights