Closed kylawl closed 8 years ago
I fail to understand the reasoning here. You are throwing lots of CPU power at the window in the first place, why would you do that?. And most likely there's no point processing inputs early if you aren't rendering. I don't understand what UpdateGui_Paint() is compared to UpdateImGuiWindows() and DrawImGui(). Who is calling ImGui::Render()? Where all the windows created? Why do you have 3 functions?
Some processing, the ones you pointed - are done at the end of the frame which is essentially done in ImGui::Render().
If you insist on throwing CPU power at the window :) I can extract part of Render() into an optional EndFrame() functional (that Render() would call if you don't call it). Don't mind doing that as it'll be useful for other constructs. Then you can call EndFrame() multiple times.
You can also just call Render() mulitple times and ignoring the render data if you don't actually want to render.
So the reason we pump ImGui for each input message is because it is being executed as part of our input processing phase. This allows ImGui (or any other input even handlers) to handle an input event before it is processed by handlers with lower priority. The benefit of this is that we always capture all user input even if the user is super fast, the framerate is low or if your input events are being sent over a network.
For example:
In your directx11_example, because all input messages are accumulated into ImGuiIO state before the next actual gui loop. It is possible for 3 messages (LMB Down, Up, Down) to come in on the same message handle loop. This would cause the ImGui to think that you simply clicked down but in reality you double clicked.
I should have provided a better code example rather than what I did. This is is more or less how the system behaves, I've inlined the imgui stuff and unrolled the inner loop iterating the handlers.
Game_BeginFrame();
for( Input event : input_events )
{
UpdateSomeOtherInputHandler0(event);
if( event.IsHandled() ) continue;
// ImGuiInputHandler(event)
{
ImGuiIO& io = ImGui::GetIO();
ParseEventIntoImGuiIO(event, io);
ImGui::NewFrame();
// Make some gui
if( ImGui::BeginMainMenuBar() )
{
...
ImGui::EndMainMenuBar();
}
if( ImGui::Begin() )
{
...
}
ImGui::End();
if( io.WantCaptureMouse | io.WantCaptureKeyboard )
{
event.Handle();
}
}
if( event.IsHandled() ) continue;
UpdateSomeOtherInputHandler1(event);
if( event.IsHandled() ) continue;
UpdateSomeOtherInputHandler2(event);
if( event.IsHandled() ) continue;
UpdateSomeOtherInputHandler3(event);
...
}
// Do other game stuff here
ImGui::Render();
Game_EndFrame()
What the Paint message does is solve the issue of what happens when you have no input events in a frame. We always push a Paint message on last which makes no changes to the input state, but allows the gui to still pump at least once per frame. Given that part of the end frame happens in Render, it makes sense that if the Paint runs first that everything works but if it runs last it doesn't.
Splitting the ImGui::EndFrame() out of ImGui::Render() would definitely solve this state issue in that case
In the imgui implementation I did at the last studio I was at, there was a slightly different design for how windows and input were setup. But in the end it came down to:
There is a flaw in the current input mechanism of ImGui in that it doesn't process events as you pointed out. In practice it's not much of a problem and character inputs which are the more prone to fast inputs aren't affected as they can be accumulated (but we should aim to allow for a more event driven approach to inputs). I think you better ought call ImGui once or aim to fix ImGui is you really need that support, but right now you may literally be throwing milliseconds of CPU out of the windows so I don't think it is worth it.
For now as I said you can just call Render() and ignore or disable the callback and that'll do the same until I had an EndFrame().
Allow the gui to run fully an arbitrary number of times between renders
This doesn't make as much sense here, Render() is a really light-weight function. The actual cost of building the render list is done as you call widgets. So I suppose we could add a "render off" mode but again it'll be more fruitful to turn the system into a timestamped-event driven system while figuring out how to retain the code simplicity.
The only reason for ignoring render is if you still want to update as a faster rate, presumably because your framerate is slow, so using a solution that consume even more CPU isn't a good idea and we should aim for the solution that consume less (timestamped events).
I have added an optional EndFrame() function because it makes the code intent neater anyway. Note that the gain compared to not calling Render() will be absolutely minimal and hardly measurable.
Cool thanks,
It makes sense there is little gain given that the draw lists are generated when the widget run.
That ties into the second point
Which is where something like your "Render Off" flag which widgets could check and not generate draw commands.
At any rate this is a good change,
Thanks again
Only build draw lists when you were actually ready to draw ie. on the last input event or a paint event
That's also possible, but do you see how that would always be a sub-par solution to the problem you are trying to solve?
The only problem we're tying to solve is ensuing that all input messages that occurred during the last frame are guaranteed to be processed in the next frame.
While amortizing the input messages over many frames would reduce the cpu cost, ImGui isn't the only thing at the party. All game play input is also processed here and we don't want to introduce input latency to the whole game.
As far as increasing cpu usage as the frame rate goes down, you are right it does seem the opposite of what you would want to do. However when you consider that we do exactly that with fixed time stepping in order to keep physics simulations stable it's not so crazy. If things totally tank during the input phase, we can cap the number of messages handled per frame. But we want to first give the game the opportunity to handle what the user has asked it to do as early as possible.
While amortizing the input messages over many frames would reduce the cpu cost,
I haven't suggested that, maybe my messages were confusing. You pointed (rightly) the problem that ImGui process discrete input states instead of timestamped inputs and may miss inputs such as mouse-click if they happen within a single frame. So this problem should be fixed in ImGui and I agree with that.
There's not many trivial solutions for physics if you want determinism, whereas it's a much easier and cheaper problem to solve for imgui than for a physics engine thankfully :)
Anything else is probably a workaround - running multiple passes on the UI is an immediately functional but costly workaround. We should aim to allow your program to only run your UI creation once.
Maybe there could be an input phase for imgui wherein input events are sent from the host to imgui.
The host could dictate which events happen should be sent that frame such as mouse click at x,y or key down.
Then the host can check for events like normal during its imgui setup e.g. if (button(...)) { send_button_event_to_host (); }.
Yes this is what I'm saying, it should be implemented and supported by all widgets. :)
Do you not have to evaluate the imgui for each event in that case anyway? I'm not sure how that's avoidable without retaining a significant amount of internal state.
In the end, the RenderOff flag is all we're looking for. It doesn't require any major rework to ImGui for input event processing, and gives user code that is concerned about input accuracy a reasonable solution in the meantime.
Is there any progress about this issue (or maybe some workarounds)? Is there some way to disable rendering info generation during input/update? Something like this would be pretty nice for perfomance improvements (fixed delta time game loop example):
timeSinceLastUpdate += delta;
ImGui::DisableRendering();
while (timeSinceLastUpdate > TimePerFrame) {
timeSinceLastUpdate -= TimePerFrame;
processEvents();
if(timeSinceLastUpdate > TimePerFrame) {
ImGui::EnableRendering(); // enable rendering generation for last update call
}
update(TimePerFrame); // ImGui functions are being called here
}
draw(window); // ImGui::Render is called here
I still fail to understand why we should implement a non-trivial sub-par solution to a problem when the optimal solution is to not call your imgui code. Add 1 bool somewhere and check it because calling Begin/End pairs.
If you have 6 input events come in and you want to consume all queued events in the next frame. IMGUI and all your other input handling systems need to process all the events in order they were received. You don't want to limit your input consumption to 1 event per frame, and IMGUI is not the only consumer of input. If you have a game viewport in an IMGUI window for example, you're viewport camera can process the mouse move event if IMGUI doesn't report a mouse capture.
Just try queuing 1 second worth of keystroke events and then process them. There are only 2 ways to go that promise you get the correct state. You either run your ui 1 event per frame or you process the whole queue in the frame. You pay can for frame performance with input latency or pay for input responsiveness with frame performance.
As far as frame performance goes, a lot of that can be mitigated by not generating render/draw lists at all until your last input event for the frame which is exactly what @ElisaD is suggesting. The rest of the cycles, moving windows, typing into text boxes, dragging sliders. You want that stuff to be up to date so it's going to happen multiple times. You have no choice if you want the responsiveness.
In fact if we even refer to your demo code, you actually update IMGUI for every input event received as well as render the frame. The shitter is you obviously can't do that in a game, the game needs to run at some time-step. So between time-steps a bunch of events are queued, we basically want to run your demo code, but instead of using WM_ messages, we have our own platform indep versions. And Instead of generating vertex buffers and drawing for every input, we want to decide which input events we're going to draw with.
This concept would actually work well as a flag on ImGui::NewFrame(ImGuiFrameFlag_NoRender) or something to that effect.
If you have 6 input events come in and you want to consume all queued events in the next frame.
That's a very theoretical example. You don't have 6 input events in a single real frame unless your frame-rate is terrible. What sort of event would you have 6 of in a frame? 6 clicks? Character inputs are already queued. At that sort of frame-rate you can't really realistically expect user to maneuver both game and imgui so the timing of transition between one and the other doesn't matter much. Everything you said is very correct but I don't think it is actually very needed. Actually aiming toward an action or involving different things takes time.
Rendering cost is already minimal compared to processing cost (you can make it more minimal by disabling anti-anti aliasing), you'll only save a fraction of the total cost.
I agree we should allow processing of multiple event per frame in ImGui and I would like to work toward that.
I'm not against adding a render off flag but it may be intrusive, and I have other priorities. My guess is that the sanier approach to implement it would be to perform testing in the RenderXXX functions in imgui.cpp. Clip rect processing needs to be kept enough so as not to affect interactions. You can try working on it, it is probably an easy patch.
I'm not against adding a render off flag but it may be intrusive, and I have other priorities.
Even when I initially suggested this, I don't expect you to do all the work we need. Certainly this is open source, and we appreciate all the work that you've put in so far.
I just don't want to be forking the project permanently if we can agree there is a solution to be mainlined.
We posted at the same time :)
In fact if we even refer to your demo code, you actually update IMGUI for every input event received as well as render the frame.
The demo code doesn't do that. They run are fixed time-step (when vsync is enabled) or unthrolled (when vsync is disabled) and feed all inputs before NewFrame.
The granularity at which the WantCaptureMouse
flag is updated isn't actually that important. Unless your frame-rate is under like 15 FPS or lower, just always pass current input state (note that imgui doesn't take events), and pass or not to your application according to WantCaptureMouse
flag. What actual problem are you having with using this scheme? If your frame-rate is actually very low your scheme would be a suitable workaround but I don't think you'll save much disabling rendering.
I just don't want to be forking the project permanently if we can agree there is a solution to be mainlined.
Sure, well. We can't change the signature of NewFrame for such a rarely used feature but you could add a flag in the IO structure and see where would be the best place to check it. I'm happy with having this feature in if it doesn't alter performances or readability too much.
Rendering cost is already minimal compared to processing cost (you can make it more minimal by disabling anti-anti aliasing), you'll only save a fraction of the total cost.
By processing cost, do you mean within IMGUIs widget calls, or the overall cost of executing user code and hopping around memory?
The demo code doesn't do that.
You're right, I missed the continue in the peek message block.
What actual problem are you having with using this scheme?
I'm not sure what @EliasD's performance issue's are.
In practical terms, you're actually right we don't really have a problem. I think I've seen at most 3 input events needed by IMGUI processed in a frame. Usually a mouse move, a click and a key stroke in a frame. It's never a been bottleneck for us, it's always within frame time and only really happens in the editor so we're fine. In fact the original issue was to do with menu behavior and such which behave just fine now with the EndFrame() so thanks for that :)
The rest of this discussion has been off topic really. As it stands, you're saying that rendering isn't the slow part so we don't need to actually disable it which is fine by me. I can stop trying to disable it on you. We have quite a bit of IMGUI running in our editor now now. If we start seeing spikes due to multiple events and it's actually in IMGUI we should start a new discussion around what those are and how we can make that stuff better.
Thanks!
@ocornut, okay, I can agree with what you've said about processing events once per frame being enough in most cases. Probably if something is taking so long that lots of events are being missed, than probably I need to just optimize this so the following situation doesn't appear. Processing input once per frame may be pretty bad for the game, but if it's just for a dev tools, then it's okay. I guess that not a lot of people have issue with this or else there would be far more requests for ability to do what I've proposed. And, as I understand, implementing this is not trivial and may make code a lot more complicated than needed. So it's alright then, I'll just use ImGui as you've advised (and I'll advice the same thing to others!). Thank you for you responses and feedback. :)
P.S. ImGui rocks, keep doing great job!
@kylawl
By processing cost, do you mean within IMGUIs widget calls, or the overall cost of executing user code and hopping around memory?
The code within widgets calls (layout, calculating text size) + the cost of executing user code and touching lots of memory.
edit Within widgets call there is a cost of rendering but that only happens for visible widgets so it is quickly capped to how much you can have visible on-screen.
That cost of rendering vs the rest would depend on how many items you have visible vs clipped ones, so it's hard to give a rough metrics. The worst case for rendering if when everything is visible (nothing is out of view, no scrollbar, etc.) but if everything is visible then you probably aren't dealing with so many widgets? It would be interesting to hook such boolean flag in all the RenderXXX functions in imgui.cpp to get a number for your use case, would probably be trivial to add such flag and cover 95% of the rendering.
We have quite a bit of IMGUI running in our editor now now. If we start seeing spikes due to multiple events and it's actually in IMGUI we should start a new discussion around what those are and how we can make that stuff better.
It would be very interesting to look at perfs (in another topic) if you have issues. Performing clipping of large list with a ImGuiListClipper pattern is generally a good way to deal with large amount of data.
@EliasD For the records, I would like to change inputs processing so it would actually process events and among other things play a little better with very low framerates. So that is in my to do list, it just hasn't been a top priority and isn't a small change. The small workaround trick that the example back-end are using is that if a click came in during the frame we always pass the mouse button as "down" even if a release came in during that frame. It is imperfect but alleviate the issue a lot.
So both of you are totally right, I'm just saying that IHMO it isn't nearly as bad as one would think.
I am closing this as it looks like we are on the same page here. I added a TODO note to eventually add a helper "DisableRender" flag, and the need to process events is already recorded.
I have a similar problem, and I cant be the only one, since most physics engines/frameworks out there demands a fixed time step for updates. And if you interpolate the matrix when drawing, it will appears smooth even tho the physics is only updated once in a while.
I tried to put up a clean example, so its easy to see where the ImGui parts should fit. It would be great if someone could give me some recommendations on how implement ImGui here.
while() // Running as fast as possible.
{
game.process(); // Process inputs.
if() // Run only once every 33 ms.
{
// If I put ImGui calls anywhere in here, it will flicker like hell, since its only called once in a while.
game.update();
game.render(); // Gather vertex data from newly updated game objects.
// ImGui::Render() here? In my mind, "render" means to get data ready for OpenGL.
}
window.clear();
game.draw(); // Interpolate vertex data, so it appears smooth visually, even tho game hasnt been updated for long time.
// gui.draw(ImGui::GetDrawData()) here? Is it possible for ImGui to keep DrawData every frame, until its been updated?
window.swap();
}
Is this something that could be done as a user without editing ImGui, for example a modified implementation? I am still a newbie among you people, so I would be glad if I got a hint at least :) Also, have you though about the decision on "DisableRender"-flag lately?
NewFrame() basically clears the data that Render() finalized. You can perfectly Render() multiple times.
If you have two tasks running at different update frequency and you want them both to use imgui unfortunately the only way I can think of right now is to use 2 imgui contexts.
However as perhaps you don't have lots of code outside the game update block, you may restrict your usage of imgui to that block, so you leave NewFrame() in that block, and call Render() every frame.
Quick response, thanks, alot!
Yes, keeping ImGui calls inside that block is perfectly fine. But I still cant get it to work, I've been bruteforcing different approaches, hehe. Atleast now I got an assert instead of segmentation faults: "imgui.cpp:3947: void ImGui::EndFrame(): Assertion `g.CurrentWindowStack.Size == 1' failed." Which says "Mismatched Begin()/End() calls".
Could it be something with not calling NewFrame() and still calling Render()? Or something with the ImGui::ShowTestWindow()?
while() // Running as fast as possible.
{
if() // Run only once every 33 ms.
{
ImGui_ImplSdlGL3_NewFrame();
if(someWindow)
{
ImGui::Begin("Debug window", &someWindow);
ImGui::Text("Hello");
ImGui::End();
}
game.update();
}
game.render();
ImGui::Render();
window.clear();
game.draw();
ImGui_ImplSdlGL3_RenderDrawData(ImGui::GetDrawData());
window.swap();
}
The assert is still showing if I have no ImGui windowing calls at all.
In our engine, we process all input messages which were sent during the last frame including a paint message which pumps ImGui every frame regardless of the number of additional input messages.
The goal here is to respond to input as early as possible and to capture all user input and not discard anything.
A frame with a mouse click would look something like this
A frame with a mouse move and a mouse click would look something like this
A frame without a mouse click would be
In this setup, I'm able to click on widgets in an open window, scroll the scroll bar both with the wheel or by dragging it and I can also resize the window by dragging the resize grip.
I cannot however click in the empty space of the window to bring it into focus, or click out of the window to move it out of focus. I also cannot click and drag the window.
In a MenuBar I'm able to click on and open a Menu, I can click MenuItems and they'll do whatever they're supposed to do. But I cannot click off the menu (say into another window or just empty space) to close the menu or cause it to lose focus.
What's most odd to me (and what I'm hoping makes the sense to you) is that if I put the Paint update first instead of last, everything seems to behave correctly?
I'm not sure if this is a bug, something I'm doing wrong or both?
Thanks for keeping this project going!