-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Added power saving mode #2749
base: master
Are you sure you want to change the base?
Added power saving mode #2749
Conversation
An issue I forgot to mention is that the frame rate requirement is a best-effort thing, and it tends to slightly undershoot because it does not take the actual frame calculation time into account (so typically if you set 30fps requirement in practice you might get around 27 or 28). One solution is to be more clever and dynamically adjust the returned timeout, either by subtracting the average frame processing time, or by implementing a control algorithm to track a target frame rate. |
Hello Corentin, Thanks for the detailed PR and idea. I don’t think using an allocating pool is required. Widgets could simply request a certain timestamp they would want to be woken up, and all of that logic can be handled with a min(), a finite amount of fields and without allocations. Generally speaking any user inputs should trigger 3 frames of update, the speed of which can be decided by the application. I generally feel like the library should provide info about the required timeout and let the application handle framerate (vs trying to decide of framerate for the app) but I haven’t had time to think about the feature more. For the blinking cursor specifically I would like to implement something even more custom and faster, because blinking cursor are both common and one of the worse case scenario. Also maybe read #2268 for other ideas. |
Hi Omar, thanks for the feedback and the link to #2268. Your first point is a good idea, it would indeed make things simpler and in fact solve an issue I'm currently facing; having two distinct calls to set/reset the requirement is not great, because it's difficult to handle resetting the value when the widget is not rendered anymore! So I could replace the method SetFrameRateRequirement with a method named for example WakeUpBefore(double timeout). I agree that the application should decide the frame rate. What I have in mind for my application (a tracker-inspired digital audio workstation; I'll post screenshots at some point when it's a bit more polished 😄) is a way to be idle by default but let certain widgets temporarily increase the frame rate: when playing a song I need some auto-scrolling of the piano roll, at maybe 5fps, and when a real-time signal visualizer (waveform or spectrum) is visible, go to to 30fps (on battery) or 60fps+ on mains. Obviously this is application logic, but with the above call, the spectrum analyzer widget could say something like:
and maybe the piano roll would say:
and obviously the actual timeout picked would end up being the shortest one. I will try that first (also implementing your suggestion to always render 3 frames) and see how it goes. While I do that, I will think about your cursor fast path idea. The binding can tell ImGui whether or not it's waking up due to an event or because it's a timeout. And ImGui knows whether or not the application requested a timeout (or it's just the cursor). With those two pieces of information it knows whether or not it should enter the fast path or the normal path. SDL_Event event;
bool got_event = 1 == SDL_WaitEventTimeout(&event, 1000.0 * ImGui::GetEventWaitingTimeout());
if (got_event)
{
do
{
// Event stuff.
}
while (SDL_PollEvent(&event) == 1);
}
else
// New call to let ImGui know there was timeout.
// ImGui will know whether or not the app required a timeout
// (and thus know if it's the fast path).
is_imgui_fast_path = ImGui::NotifyEventTimeout();
if (is_imgui_fast_path)
{
// Some clever stuff happens here.
}
else
{
// Business as usual.
// Start the Dear ImGui frame
ImGui_ImplOpenGL3_NewFrame();
ImGui_ImplSDL2_NewFrame(window);
ImGui::NewFrame();
...
} |
So, I've re-designed it following the line you suggested:
I'm quite happy with the way it behaves, now. For the cursor, aiming at 6fps, you actually end up getting something under 18 fps, because you actually get six 3-frame salvos, due to the aforementioned logic: tac-tac-tac, sleep, tac-tac-tac, sleep etc. I have implemented and tested:
I have also done some actual power measurement. I've used a pretty cool tool called s-tui (can be installed using pip). This was done with nothing else running. I started the demo app, let it run for a bit in normal mode (i.e. at 60fps), then enabled the power saving mode, let it run for a bit, then clicked on a Text Input with a blinking cursor, and finally enabled the animated plots demo: It's not a super methodical experiment but it gives an indication. Unfortunately it's only CPU power, so I can't see the overall actual impact on the entire system, including GPU (which obviously is what matters for battery life). Next steps
I will do step 3 after the other 2, because the fast path logic will constrain how the event logic works and integrates. Call for testingIt's now in a decent state where other people can start having a look, see if they find the API convenient, see if it works for their binding/platform, etc. |
Some more work... A bit of cleaning and refactoring, in particular I've extracted the waiting logic into the common SDL implementation file, to keep the platform+renderer binding simple and not deviating too much from existing. I will do the same thing for the other examples. I've also fixed the cursor logic to wait for the appropriate time to flip instead of trying to animate it as 6fps. I repeated my not-so-scientific test from above and this time I can't see any difference at all. But, again, that's just CPU power. I might try measuring at the socket on my desktop computer to see what difference GPU power makes. Then we can see if the more sophisticated logic to only blit the cursor is worth the effort/complexity. Otherwise I had a look at the Emscripten example, and it won't be as simple as the other ones (or even possible?) due to the different in event model. |
Finished refactoring all examples (i.e. Win32, SDL, Glfw and Allegro). And now we only have 3+ frames of update after user inputs, not on things like cursor blinks (which means the cursor blink is now effectively 1.7fps). Known issue(s):
|
So, I'm happy with how the patch looks at this stage. I've done some basic testing on:
Note that I did not test gamepad/joystick input; just mouse and keyboard. I've addressed all the issues I was aware of. So I think it's good for review / testing / merging. Regarding the rendering optimization you suggested, I'm not convinced it's super critical at this stage, and in any case can be done in a separate PR, and perhaps in a more generic way (rather than just for the cursor). |
This is awesome. For example, if the user has no input activity at all, the FPS must significantly decrease. Of course, this should never happen when vSync is turned on. |
@corentin-plouet, looks cool! |
Thanks for the feedback! @bsviglo that's a good question indeed. This PR is on the master branch, not the viewport branch, but I need to start messing with that branch soon anyway, so I will take that into consideration. On top of my head, there is nothing preventing from doing it on a per-window basis. But as always, the devil is in the details. |
7d9a8cb
to
57caec9
Compare
Rebased on 1.73 |
Any news on this thread? |
56ed656
to
bf23075
Compare
Rebased on master. |
Thanks Corentin for updating this, i'm not sure when I'll be able to look at this thoroughly but I feel it's looking quite good. As for @bsviglo suggestion, there are two things we could be later be investigating: The obvious general "make this multi-viewport" aware. The typical use case is as bsviglo mentioned: a running game rendered somewhere may want to request its host viewport to render at maximum rate, while other windows may be refreshed differently. The tricky thing being that imgui back-ends will end up seeing the inputs going to the game and this may erroneously wake up the imgui parts.. so this need to be solved somehow. It's going to be difficult to make this PR in two steps (master branch then viewports). If you attempt to experiment with multi-viewports version of it, may I suggest you open a separate PR for your experiments, keep this one here as reference. Based on its contents, when we are happy with the result, we can attempt to split the non-viewport code and merge first into master then merge the rest in docking branch. Or maybe by the time I look at this PR seriously, I'll be able to provide a solution for multi-viewports. ( |
@corentin-plouet I don't understand the ImGui::WakeUpBefore API, shouldn't it be ImGui::WakeUpAt? what we want is not that the widget is refreshed Before a time out expire but at dedicated time... for example a video widget 25Fps should ask to be refreshed "At" 1/25s past the last refresh, it won't care if it is refreshed "Before" the timeout expire... or ImGui::RemainingTimeUntilRefresh() |
Hi @JLMorange, first, please note that the code has been through a few iterations, and my comments above reflect the entire development history (the code history has been "squashed" by git force-push though). In fact, the API you describe is now named SetMaxWaitBeforeNextFrame, which is a longer name but more accurate. Note that it's not guaranteed scheduling: if you call it with a value of 1/25, you may end up waking up before that, simply because the user interacted (moved the mouse, etc.) So it's a "max wait" indeed. @ocornut, I'll have a look at applying this patch to the multi-viewport / docking branch. I think the simplest and most robust solution is to have any event, on any viewport, refresh all of them. In terms of power saving, the idea is to get the CPU and GPU to be idle/sleeping as often as possible, but when they need to wake up I'm not sure we save much by having viewport-specific behavior. But I'm open to any sensible behavior; first I need to get myself familiar with that branch... |
i really like this! i've used something similar / less sophisticated in the past. i think our code has the same issue: when using the gamepad nav system, how do you make sure you're staying responsive enough? glfw does not generate joystick events, so it won't wake up glfwWaitEvents*(). this is probably pending something like this: glfw/glfw#1590 does this mean with gamepads we're doomed to do power intense polling? |
We can still poll events at some rateand do nothing else if there aren’t gamepads changes. |
hmm. right, i suppose a background thread calling |
this works, thanks for the idea. for reference, here's my polling thread: https://github.com/hanatos/vkdt/blob/master/src/gui/main.c#L71 and further down the same file is the glfw main loop: https://github.com/hanatos/vkdt/blob/master/src/gui/main.c#L257 |
8b83e0a
to
d735066
Compare
b3b85d8
to
0755767
Compare
c817acb
to
8d39063
Compare
Blinking cursor is not showing with ImGuiColorTextEdit, any ideas how to fix that?
#include "TextEditor.h"
ImGui::Begin("Some window");
static TextEditor editor;
editor.Render();
ImGui::End(); and it doesnt use |
Hi, Any plans to merge this to master? I am also evaluating ImGui for a regular desktop app and the CPU consumption is the only issue I am facing. |
…#7556, #5116 , #4076, #2749, #2268) currently: ImGui::SetNextWindowRefreshPolicy(ImGuiWindowRefreshFlags_TryToAvoidRefresh); - This is NOT meant to replace frame-wide/app-wide idle mode. - This is another tool: the idea that a given window could avoid refresh and reuse last frame contents. - I think it needs to be backed by a careful and smart design overall (refresh policy, load balancing, making it easy and obvious to user). - It's not there yet, this is currently a toy for experimenting. My other issues with this: - It appears to be very simple, but skipping most of Begin() logic will inevitably lead to tricky/confusing bugs. Let's see how it goes. - I don't like very much that this opens a door to varying inconsistencies - I don't like very much that it can lead us to situation where the lazy refresh gets disabled in bulk due to some reason (e.g. resizing a dock space) and we get sucked in the temptation to update for idle rather than update for dynamism.
What
When enabled, this mode reduces the frame rate (potentially down to zero) by waiting for inputs (i.e. blocking/sleeping).
Both ImGui and user code can require a minimum frame rate for individual components (identified by an ImGui ID) to cater for things like blinking cursor or animations. This is dynamic; i.e. a requirement can be set when starting an animation, and set to zero if/when it's finished.
Note that I've used a fairly broad name for this feature (power saving) because in the future we might want to do other things beyond just altering the frame rate.
Why
My use case is a regular desktop application (audio) that I want to use on a laptop without too much noise, heat and power usage. This application does not need to run at the display rate/60fps, even when showing things like real-time waveform and spectrum analyzers.
How
I went for the simplest and ideally the most reliable, and efficient, implementation I could think of.
The feature is enabled setting ImGuiConfigFlags_EnablePowerSavingMode in IO.ConfigFlags.
The binding code can call ImGui::GetEventWaitingTimeout() to know how long it should wait for events; possibly 0, in which case it can poll (or just pass a 0 timeout if supported by the implementation), or infinity (when the frame rate is zero) in which case it can call a non-timeout event waiting method.
As said above, user code can add frame rate requirements. I've used an ImPool structure to store those requirements, which allows fast lookup to identify which one is currently the highest one (if a cursor requires 6fps and an animation requires 30fps, naturally the effective minimum frame rate will be the highest, 30fps).
ImGui does not share this structure; it uses its own boolean flags (currently a single one, Blinking).
Integration
I have added support for blinking cursors in ImGui.
I have added the feature in the demo app. A good test is to enable the power saving mode and observe the FPS counter when looking at the animated widgets example.
I have added support for the SDL and Glfw (OpenGL 3) bindings, and the Allegro binding.
Testing / Known issues
I have done some manual testing on Linux, with the demo (SDL, Glfw and Allegro bindings) and my own app. It seems to work well.
One known issue is that the frame rate requirement for blinking cursor is not properly reset when the input box is not visible. That should be easy to fix; I thing I can use a flag.(FIXED)Next steps
I'd like to have your input.
I plan to test on Windows and implement support in more bindings, and use a power profiler tool to get an idea of the potential power saving.