Skip to content
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

Another power saving mode. #3124

Closed
wants to merge 0 commits into from

Conversation

sergeyn
Copy link
Contributor

@sergeyn sergeyn commented Apr 13, 2020

Hi, this is alternative implementation of PR #2749 . I'm in similiar situation where I don't need to render UI all the time, just rendering updates when they happen will do. The strategy I chose is it to report for each non-static widget a time in the future relative to current frame when it'll end up rendering something different. Then platform code using this information may choose to sleep for specified period of time. I've updated win32 dx12 sample to demonstrate this behavoiur and made text cursor blinking report it's time when it's goint to blink. Other examples which I didn't touch will work same way as they were before the change.

@sergeyn sergeyn force-pushed the another_power_saving_mode branch from fe19061 to ee71810 Compare April 14, 2020 16:03
// Setup Dear ImGui context
IMGUI_CHECKVERSION();
ImGui::CreateContext();

ImGuiIO& io = ImGui::GetIO(); (void)io;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why moving those CreateContext lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

It's because ShowWindow function will call into wmproc, which in turn needs context for most of the messages (WM_SIZE in particular). Thus I moved context creation to before ShowWindow.

Regards,

@ocornut
Copy link
Owner

ocornut commented Apr 15, 2020

It feels that it'd be simpler and more portable to have all those check done by core imgui (e.g. ImGui::NewFrame()) rather than leaving it to each backend though?

@sergeyn
Copy link
Contributor Author

sergeyn commented Apr 15, 2020

Would you please be more specific about which checks you refer to ? Thanks

@ocornut
Copy link
Owner

ocornut commented Apr 15, 2020

DisplaySize change, MousePos/Buttons/Wheel change, Gamepad change, Keyboard changes, Character Inputs.

It feels that we could come up with a design that would put less strain on the specific back-end by actively doing a compare in e.g. ImGui::NewFrame() or add a specific function so we have more control over what's running. Agreeing it would consistute more "polling" instead of wake-up-on-event, but comparing < 1 KB of memory every frame would be negligible.

I haven't dug very deep into this topic (so may be very wrong) but this is my intuition.

@sergeyn
Copy link
Contributor Author

sergeyn commented Apr 15, 2020

Ideally yes, I agree - it's better be ImGui answering the question "has something changed big enough to require a redraw". In this case ImGui can be more aggressive on skipping redraw with some extra logic, for instance, if the mouse is staying within same element ( a button for instance), then you don't have to redraw on every mouse move like it does now.
As for the interface - it'll get slightly more cluttered with one or two extra function to cover this logic, which as I see now has a few distinctive steps, and one of them has to be done on the platform side before doing rendering:

  1. gather all updates (input/non-input) so far <- platform side
  2. send it to ImGui and get an answer if redraw is required <- imgui side
  3. optinally sleep/wait for more messages if nothing to render <-- back to platform side
  4. full blown draw step <- imgui side.

It can be whole new IO structure as you suggest since indeed memcmp'ing 1kb on input is not a big deal. But it would be very nice to be able to accumulate updates within this structure independently with main drawing because there's a use case of running message loop in a separate thread (as I do atm.)
An question to answer this PR is whether "NextRefresh" is the way to go. This is a feedback of the rendering code and is independend of the questions you ask about how to get updates into ImGui. And I'm also not sure if this member should be part of IO structure.

@joeslay
Copy link
Contributor

joeslay commented Apr 15, 2020

You can skip step 2 sergeyn. Just put all that logic into the event loop, seems much simpler to me. No events, no rendering, just as responsive, much lower CPU usage. I just wrote this for the SDL example and it works fine for me :

SDL_Event event;
wait_for_event:
if (!SDL_PollEvent(&event))
{
     Sleep(1);
     goto wait_for_event;
}
else
{
   do 
   {
       ImGui_ImplSDL2_ProcessEvent(&event);

       //do your normal event processing here

   } while (SDL_PollEvent(&event));
}

@gfickel
Copy link

gfickel commented Apr 20, 2020

Hi @joeslay ! This solution has the drawback that nothing gets drawn on the screen if there is no user input. This is bad for everything that has something dynamic such as a movie player or even the Plot Widgets from imgui_demo.

@joeslay
Copy link
Contributor

joeslay commented Apr 21, 2020

Hi @joeslay ! This solution has the drawback that nothing gets drawn on the screen if there is no user input. This is bad for everything that has something dynamic such as a movie player or even the Plot Widgets from imgui_demo.

Yeah, you'd have to manage dynamic widgets.
A function that generically generates an event on a timer in another thread would work.
In SDL you use SDL_PushEvent.

@sergeyn
Copy link
Contributor Author

sergeyn commented Mar 3, 2022 via email

@sergeyn sergeyn closed this Mar 18, 2022
@sergeyn sergeyn force-pushed the another_power_saving_mode branch from b64e123 to 6fae296 Compare March 18, 2022 08:57
@sergeyn
Copy link
Contributor Author

sergeyn commented Mar 18, 2022

I've accidentally closed this one, created another one instead: #5116

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants