openframeworks / openFrameworks

openFrameworks is a community-developed cross platform toolkit for creative coding in C++.
http://openframeworks.cc
Other
9.84k stars 2.56k forks source link

ofFpsCounter with more precision using straight std::chrono #7966

Open dimitre opened 1 month ago

dimitre commented 1 month ago

ofFpsCounter with more precision using straight std::chrono when possible. filterAlpha is being calculated with double. the removal of some includes shows other files were depending of the includes included here, so now they are more explicit.

this PR:

dimitre commented 1 month ago

Finally! ready to go @ofTheo

ofTheo commented 1 month ago

oh cool @dimitre - going to really hammer it this week! be great to get some extra eyes on this too as its pretty core to a lot of things :)

@artificiel @2bbb @NickHardeman

dimitre commented 1 month ago

Nice. It is a direct replacement line by line, and keeping the same logic from original fps counter. Using std::chrono allows the same code for all platforms, with the maximum precision possible allowed by the time resolution in a specific platform. As it was possible to remove ofUtils.h include I had to add back includes in lots of other OF parts, which is good. The most direct the dependencies are, less things for preprocessing and compiling.

NickHardeman commented 1 month ago

@dimitre, I just did a simple test with setting the fps of the app and recalculating via ofFpsCounter and it looked good to me.

While looking at the PR, I noticed these namespace lines at the top of ofTimeFps.h https://github.com/openframeworks/openFrameworks/blob/12f4a84f2e1d8838b6e472aba85e374ebfaabf56/libs/openFrameworks/utils/ofTimerFps.h#L10-L11

I think the appropriate std::chrono's should be prepended to the vars in the .h file and the above lines moved to ofTimeFps.cpp. What do you think? Otherwise the using namespace std::chrono; and using namespace std::chrono_literals; is global.

std::chrono::time_point<std::chrono::steady_clock> wakeTime;
std::chrono::time_point<std::chrono::steady_clock> lastWakeTime;
dimitre commented 1 month ago

Thank you Nick, good catch. I'll remove this from .h files

artificiel commented 1 month ago

this PR circumvents ofTime, which has platform #ifdefs etc (I think that pre-dates std::chrono?).

could the work here expand into simplifying the std::chrono in ofTime itself? so the timing functions internals are all based on the same core, in the sense that ofGetElapsedTimef() itself gets "upgraded"?

dimitre commented 1 month ago

@artificiel good idea, ofTime can be updated so all internal operations are done using std::chrono. I still think this one can be done with straight chrono as it will reduce .h includes