-
Notifications
You must be signed in to change notification settings - Fork 312
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
On-demand rendering (power saving mode) #436
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool, and surprisingly simple. I was expecting something much more invasive.
I'd be happy to include this functionality. And this looks like a good and simple approach, and significantly reduces the CPU usage for me.
I see that there are no automatic "update requests" during DOM, property, or layout changes. But, I guess, the idea is that if we are making any such modifications, that means we are already looping, so we don't need to make any explicit requests. That moves some of the responsibility over to the client. Do you have some thoughts on this?
I think ideally, the Lua plugin should make these requests automatically during cases like those mentioned above. But we can deal with that later on. I also think we should add this feature to all of the backends, unless they are very complicated, but we can do that gradually.
It doesn't really work that well with the Invader sample, since it stops the game part of the application. Maybe for the purpose of the samples, add something like a bool power_saving_mode
argument to Backend::ProcessEvents
? I think by default, this could be enabled, since most of our samples are mostly idle. And then we can go through each sample and test where it needs to be disabled. This would also serve as an example for both approaches in the backends, so users can pick their desired one.
The
RequestNextUpdate
andNextUpdateRequested
functions should probably be in the header to make sure they get inlined as often as possible.
I couldn't measure any performance impact on this PR at all, so I think it is fine as it is.
Because its only a time delta
float
will probably do as well
Double seems reasonable to me.
See also my other comments, most of them are minor style and semantics.
Yes that was the idea. As far as I could tell RmlUi is not threadsafe, so if you want to make UI changes you need to do so in the main thread, which involves waking up from the event loop anyway. So you might as well rerender it, which is probably cheaper (and a lot less troublesome) than detecting and handling every since change everywhere.
I noticed this as well. Because the stars are a custom element and not an animation they don't fall under the delay calculation. This can be relatively easy fixed by inserting appropriate
I'd rather fix the custom elements in the samples because I think they should behave correctly regardless of whether or not powersaving is on. The entire point of the feature is for RmlUi to tell the app how often it wants to be rendered.
We can still add the flag, but I don't think it should break some of the samples if you enable it. |
The library is also used quite a lot in games, and there you don't really want to be troubled by this feature. I mean, it would just be an extra call to RequestNextUpdate, but still, that is an extra step to learn about for new users. I think it would be nice to also show how you make your loop always run without delay. For the invader sample, you would always just submit zero, because the stars and invaders are moved every frame. So to me, it makes more sense to just disable the feature. Same with the benchmark. I haven't really tested all the other ones. |
I added the power_save flag to all the backends and updated the samples to use it when it didn't influence the functionality.
|
This is looking good! Tested the win32 backends, and all the samples. Everything seems to be working well on my end. We can merge this in my view, let me know when you are happy with it. It would be very helpful if you could add some entries into the documentation as well, explaining how to use this feature. |
Will do. |
Thanks a lot, this is a very welcome addition! :) |
In todays edition of "Lets build a desktop app with RmlUi" we get on demand rendering.
Unlike a game, which tends to always have motion on screen, a desktop/gui app typically is fairly static. The idea here is to reduce the rate of updates and render commands when there are no changes to the onscreen content. This idea came up before in #417 and #331 and was briefly mentioned in 430.
At its core are two functions for setting and getting a value that is updated by elements in the update phase. It is initialized to infinity before each cycle and can only be reduced but never increased. If an element needs updates (for example because an animation is playing) it can calculate the delay until the next visible change and store this value.
After the update was run the platform can now stop rendering for the calculated maximum duration and use a blocking call to wait for input events. The only way for the onscreen content to change at this point is by input events or the app doing changes, both of which will trigger an event and cause a rerender. This drastically reduces the cpu and gpu usage of an idle application.
This is a fairly unintrusive change, unless the app opts into using the calculated value, nothing changes. It can just render at full FPS and everything will behave correctly. You can also use the calculated value but apply a minimum/maximum to limit the frame rate within bounds.
Consider this an initial draft, while it works really well for the demo applications and I tried to find every case where regular refreshes are necessary (e.g. the blinking text cursor) I am not familiar enough with RmlUi to judge if I found all of them. I also didn't look into how it interacts with scripting at all.
I also only implemented support for it in the glfw_vk backend and I am unsure if it should stay there (or be a compile time decision for the samples).
There are some things I'd like to change though:
RequestNextUpdate
andNextUpdateRequested
functions should probably be in the header to make sure they get inlined as often as possible.float
will probably do as well, though I am unsure if it make a difference performance wise and since all other timestamps use double I went with double here as well.IsInViewport
function or something could be used to skip animations on non visible elements. I did useIsVisible
, but that only checks if the element has the visibility set to visible, not if its actually visible to the user. This might be difficult to do especially with alpha though. (EDIT: I now check IsVisible recursively which is not perfect but at least better)Let me know what you think about it.