openfl / lime

A foundational Haxe framework for cross-platform development
https://lime.openfl.org/
MIT License
754 stars 367 forks source link

SDL_GetTicks Noticeably Imprecise #1569

Open steviegt6 opened 2 years ago

steviegt6 commented 2 years ago

SDL_GetTicks is only accurate to the millisecond, which results in deltaTime values being rather imprecise. This is immediately observable in the FPS display, which is inaccurate by a couple frames: image

In other words, if you have an application running at 60 fps, you would expect a delta time of roughly 1666 (1000 / 60 = 16.66667) at least, but you're instead just given 16.

A piece of the relevant code (SDL_GetTicks is used in several relevant places in this file): https://github.com/openfl/lime/blob/862fe55b1cf567bdfb4ada7d5bc78be36353f9e6/project/src/backend/sdl/SDLApplication.cpp#L132

EyeDaleHim commented 2 years ago

How interesting, I might fix this in a private repository of mine

joshtynjala commented 2 years ago

For reference: In Flash, which OpenFL is based on, the only option for timing is flash.utils.getTimer(), which also has millisecond accuracy.

Dimensionscape commented 2 years ago

I thought this only affected the frame rate counter, but the event loop delta is actually wrong due to this. Perhaps we should implement the timing for the main loop like Starling and use a float. I will look into this more.

joshtynjala commented 2 years ago

I will look into this more.

Sounds good. I hope that you'll wait to commit until after OpenFL 9.2 is released. This sounds like something that should be in dev builds for a while, just to ensure that there aren't any unintended side effects.

Dimensionscape commented 2 years ago

No worries there. The event loop is a critical, core component obviously and any changes should be carefully considered and well tested. There's not enough time to even attempt to fit it into the next release even if I wanted to try. 😂

Cheemsandfriends commented 4 months ago

Searching up a bit through SDL's shenanigans, I found this website detailing an alternative to SDL_GetTicks

SDL_GetPerformanceCounter with SDL_GetPerformanceFrequency

This could help with the issue facing right now? The website's example seems to be putting the stuff in seconds, but maybe we could fix that by multiplying it to 1000?

Dimensionscape commented 4 months ago

Searching up a bit through SDL's shenanigans, I found this website detailing an alternative to SDL_GetTicks

SDL_GetPerformanceCounter with SDL_GetPerformanceFrequency

This could help with the issue facing right now? The website's example seems to be putting the stuff in seconds, but maybe we could fix that by multiplying it to 1000?

There's nothing wrong with using GetTicks.(Well actually I suppose GetTicks does create a bit of a problem as well) The issue is the granularity of SDL_Delay. Fortunately, I have a solution that we may add experimentally.

Cheemsandfriends commented 3 months ago

@Dimensionscape Can you please provide the solution that you have in mind? I kinda wanna test it aswell 🙂

Dimensionscape commented 3 months ago

@Dimensionscape Can you please provide the solution that you have in mind? I kinda wanna test it aswell 🙂

My proposal for resolving this involves moving most of the main loop logic out of C++ and into haxe. From there we will make it modular so that users can swap out their own main loop logic. Finally, we will have a few generic main loop implementations that include the original so as not to create any unexpected changes in the event developers have come to expect the existing behavior. I've drafted a few new Lime System methods that will enable us to create a very accurate main loop with microsecond granularity. One of the issues here (other than being able to measure time at finer granularity higher than a single millisecond) is that the operating system itself is just not accurate at sleeping. This is essentially required to create a high performance frame loop with no churn. We can improve this by changing some settings for the application in C++, which I think SDL already does for us, but we still only get into the realm of being accurate by a few milliseconds on most operating systems, which is not good enough.

Going back to the new System APIs, I have introduced a more accurate timestamp and sleep method that combines operating system sleep with minimal spinlock. Inherently, these are all we need to create a more accurate frame loop, however, it's going to take some time before the draft makes its way into Lime and the work to extract the main loop out of the native code and into haxe is complete. The code implementation isn't very complex, but it will take a lot of thoughtful consideration as to how we handle this in a way that isn't confusing, burdensome or conflicting(unification with html5??).

If anyone has any thoughts or ideas in regards to any of this, feel free to share or talk to me about it on discord. I'd be more than happy to work with someone to wrap this up.

BoloVEVO commented 3 months ago

In SDLApplication.h converting currentUpdate, nextUpdate and lastUpdate to doubles actually solves the issue and you get an accurate framerate, it's weird because even if they are doubles, we are still working with integers because of SDL_GetTicks().

SomeGuyWhoLovesCoding commented 3 months ago

In SDLApplication.h converting currentUpdate, nextUpdate and lastUpdate to doubles actually solves the issue and you get an accurate framerate, it's weird because even if they are doubles, we are still working with integers because of SDL_GetTicks().

I already tried that and sometimes it jitters when not on fullscreen

SomeGuyWhoLovesCoding commented 3 months ago

You can change SDL_GetTicks() to some cpp timestamp if you want but the main loop is badly designed and may fuck up

BoloVEVO commented 3 months ago

In SDLApplication.h converting currentUpdate, nextUpdate and lastUpdate to doubles actually solves the issue and you get an accurate framerate, it's weird because even if they are doubles, we are still working with integers because of SDL_GetTicks().

I already tried that and sometimes it jitters when not on fullscreen

It could be because of doubles innacuracy, because of pixel scaling in fullscreen the innacuracy happens but the jitter it's not noticeable.

SomeGuyWhoLovesCoding commented 3 months ago

It actually is SDL_GetTicks

BoloVEVO commented 3 months ago

It actually is SDL_GetTicks

Try with performanceCounter if there's any difference.

SomeGuyWhoLovesCoding commented 3 months ago

It actually is SDL_GetTicks

Try with performanceCounter if there's any difference.

What is performanceCounter? What's the math?

BoloVEVO commented 3 months ago

It actually is SDL_GetTicks

Try with performanceCounter if there's any difference.

What is performanceCounter? What's the math?

uint64_t ticks = SDL_GetPerformanceCounter();
uint64_t frequency = SDL_GetPerformanceFrequency();
currentUpdate = ((double) ticks / (double) frequency) * 1000.0;

applicationEvent.type = UPDATE;
applicationEvent.deltaTime = currentUpdate - lastUpdate;
SomeGuyWhoLovesCoding commented 3 months ago

What about for other values like lastUpdate and nextUpdate?

BoloVEVO commented 3 months ago

What about for other values like lastUpdate and nextUpdate?

They remain the same, ofc as doubles but be aware that we are now working with doubles so SDL_addtimer and SDL_Delay won't be accurate to make the app wait to have the desired framerate, of course it still displays the accurate framerate but with a minimal error because of the decimals we are cutting off from the double.

SomeGuyWhoLovesCoding commented 3 months ago

What about for other values like lastUpdate and nextUpdate?

They remain the same, ofc as doubles but be aware that we are now working with doubles so SDL_addtimer and SDL_Delay won't be accurate to make the app wait to have the desired framerate, of course it still displays the accurate framerate but with a minimal error because of the decimals we are cutting off from the double.

We could use the spinlock from lime's system api and port that to cpp, and we wecan change the uint32 arguments to float (not double)

BoloVEVO commented 3 months ago

What about for other values like lastUpdate and nextUpdate?

They remain the same, ofc as doubles but be aware that we are now working with doubles so SDL_addtimer and SDL_Delay won't be accurate to make the app wait to have the desired framerate, of course it still displays the accurate framerate but with a minimal error because of the decimals we are cutting off from the double.

We could use the spinlock from lime's system api and port that to cpp, and we wecan change the uint32 arguments to float (not double)

I think so, for now I'm just using a while to wait the exact ticks as double.

SomeGuyWhoLovesCoding commented 3 months ago

We absolutely need to use float for currentupdate addtimer and delay and not douvles. This makes it so that it won't use so much cpu usage

SomeGuyWhoLovesCoding commented 3 months ago

What about for other values like lastUpdate and nextUpdate?

They remain the same, ofc as doubles but be aware that we are now working with doubles so SDL_addtimer and SDL_Delay won't be accurate to make the app wait to have the desired framerate, of course it still displays the accurate framerate but with a minimal error because of the decimals we are cutting off from the double.

We could use the spinlock from lime's system api and port that to cpp, and we wecan change the uint32 arguments to float (not double)

I think so, for now I'm just using a while to wait the exact ticks as double.

Yes exactly

SomeGuyWhoLovesCoding commented 3 months ago

So, we'll have another variable named remaining, having a value of value - (uint32) value.

After the sleep call, we can do what spinlock does and now it's all done.

BoloVEVO commented 3 months ago

Btw this doesn't solve the issue for x32 builds since x32 will convert everything to 32 bit integers and not to float. Also I don't think Lime would like to make this move of changing integers to double because it's a breaking change.

BoloVEVO commented 3 months ago

OK the problem why the innacuracy is this in SDLApplication.cpp

nextUpdate += framePeriod; 

while (nextUpdate <= currentUpdate)
    nextUpdate += framePeriod;      

framePeriod is the frame time for example for 60fps is 1000/60 if nextUpdate is an integer, it won't add 16.66 as intended it will just increase 16

SomeGuyWhoLovesCoding commented 3 months ago

Btw this doesn't solve the issue for x32 builds since x32 will convert everything to 32 bit integers and not to float. Also I don't think Lime would like to make this move of changing integers to double because it's a breaking change.

The variable will be the type float lol

SomeGuyWhoLovesCoding commented 3 months ago

OK the problem why the innacuracy is this in SDLApplication.cpp

nextUpdate += framePeriod; 

while (nextUpdate <= currentUpdate)
  nextUpdate += framePeriod;      

framePeriod is the frame time for example for 60fps is 1000/60 if nextUpdate is an integer, it won't add 16.66 as intended it will just increase 16

So now when nextUpdate is finally a float because currentUpdate is a float, that issue can probably be fixed

SomeGuyWhoLovesCoding commented 3 months ago

Your delta time fix didn't really fix the jitters

SomeGuyWhoLovesCoding commented 3 months ago

And the cpu usage has increased to 30%

SomeGuyWhoLovesCoding commented 3 months ago

The windows target is fine and has only a tiny degree of screen tearing meaning inputs are super fast

SomeGuyWhoLovesCoding commented 3 months ago

However. I'll be leaving alone the repositories that make my github account a mess and I'll be making a separate account for my own fork of lime openfl and flixel just to prevent my github from being messy. Until lime revamps the main loop, we'll stop doing this.

Dimensionscape commented 3 months ago

In SDLApplication.h converting currentUpdate, nextUpdate and lastUpdate to doubles actually solves the issue and you get an accurate framerate, it's weird because even if they are doubles, we are still working with integers because of SDL_GetTicks().

No, we can only delay and measure in 1 ms granularity. Converting to floating point doesn't solve this.

Dimensionscape commented 3 months ago

What about for other values like lastUpdate and nextUpdate?

They remain the same, ofc as doubles but be aware that we are now working with doubles so SDL_addtimer and SDL_Delay won't be accurate to make the app wait to have the desired framerate, of course it still displays the accurate framerate but with a minimal error because of the decimals we are cutting off from the double.

We don't need to use floating point values. We can simply use microseconds instead of milliseconds as integer. Just converting to float will not solve the root issue.

I've already outlined how we are going to address this.

SomeGuyWhoLovesCoding commented 3 months ago

What about for other values like lastUpdate and nextUpdate?

They remain the same, ofc as doubles but be aware that we are now working with doubles so SDL_addtimer and SDL_Delay won't be accurate to make the app wait to have the desired framerate, of course it still displays the accurate framerate but with a minimal error because of the decimals we are cutting off from the double.

We don't need to use floating point values. We can simply use microseconds instead of milliseconds as integer. Just converting to float will not solve the root issue.

I've already outlined how we are going to address this.

But, doing bolovevo's method of that worked well for me. On cpp, it was buttery smooth and stuff yk

SomeGuyWhoLovesCoding commented 3 months ago

But yeah I think microsecond accuracy is enough and your method of doing that would be better.

Cheemsandfriends commented 3 months ago

handle this in a way that isn't confusing, burdensome or conflicting(unification with html5??).

There is window.setTimeout that works exactly like Sys.sleep if Im getting what you're saying. but either way, Im excited! I wish to see a fork with your implementation to test how it works and maybe even contribute to your contribution if it's possible! 🙂

EliteMasterEric commented 3 months ago

Found this thread after looking into a solution for this issue...

SDL3 (which is currently in pre-release right now and is coming out any time now they swear) includes the function SDL_GetTicksNS(), which provides a value to the nanosecond (one billionth of a second, or one thousandth of a microsecond) as a long (and not a double which can have accuracy issues!). It, alongside with useful features like native file dialogs, HDR support, multimedia clipboard support, and more make me think that Lime should update to take advantage of these features soon.

SomeGuyWhoLovesCoding commented 3 months ago

It uses more cpu though

EliteMasterEric commented 3 months ago

It uses more cpu though

What do you mean?

Dimensionscape commented 3 months ago

Found this thread after looking into a solution for this issue...

SDL3 (which is currently in pre-release right now and is coming out any time now they swear) includes the function SDL_GetTicksNS(), which provides a value to the nanosecond (one billionth of a second, or one thousandth of a microsecond) as a long (and not a double which can have accuracy issues!). It, alongside with useful features like native file dialogs, HDR support, multimedia clipboard support, and more make me think that Lime should update to take advantage of these features soon.

Yea, sdl3 seems like a natural evolution for lime. It will be some time before we make the jump though, I think.

We can already measure time in haxe well enough, but there are other considerations that I've outlined above.

Dimensionscape commented 2 months ago

@SomeGuyWhoLovesCoding Please dont spam issues. Try to stay on topic.