search

revery-ui / revery

:zap: Native, high-performance, cross-platform desktop apps - built with Reason!
https://www.outrunlabs.com/revery/
MIT License
8.06k stars 196 forks source link

Add fps counter #962

Closed Grinshpon closed 4 years ago

Grinshpon commented 4 years ago

Add an fps counter (issue #470) and a way to display it. To enable or disable it, call the function Window.setFPSCounter:

Window.setFPSCounter(window,true/false);
OhadRau commented 4 years ago

any chance you could land a frame render time in this? should just be 1 more line of text to display but I think it's super useful for debugging

Grinshpon commented 4 years ago

It'll take a bit more effort to make it readable, since it's updated every render frame it'll look like a bunch of flickering digits.

Grinshpon commented 4 years ago

Alright, I'm pushing a commit that resolves everything above, and I added a --show-fps flag in the example.

Grinshpon commented 4 years ago

@glennsl you're right, taking the average works and looks much better. As for the second part, wouldn't that essentially just be me manually re-implementing a timer?

glennsl commented 4 years ago

It would, but more declaratively. You don't have to manage a timer instance, and it's overall much simpler. A timer (in the render loop) is just a counter and a conditional asking "is it time yet?".

Grinshpon commented 4 years ago

makes sense, I'll do that

Grinshpon commented 4 years ago

Before this is merged, I'm considering also showing the average frame render time like @OhadRau suggested earlier. Is this a good idea or no? And what should it be labelled as when displayed? I'm thinking AFT: ... for average frame time.

Edit: Actually I decided against this. the average time wouldn't be much helpful I think since it's just the reciprocal of the average fps, and any meaningful information is lost.

glennsl commented 4 years ago

I think what @OhadRau was referring to is the time it takes to render each frame (start to end), not the time between frames (start to start of the next frame). And I think that would be very useful as well. That's actually a thing we have been getting from logging, via Performance.bench, and it's an awfully noisy way of doing it. Replacing that would be great!

Grinshpon commented 4 years ago

In that case, it might be worth creating a separate issue for that, instead of tacking it on to this pr.

Grinshpon commented 4 years ago

Nice, all checks pass, I'd like to know if I've sufficiently satisfied all requests?

bryphe commented 4 years ago

Thanks for the contribution, @Grinshpon !