coelckers / prboom-plus

This is a cleaned up copy of the PrBoom+ SVN repository as a courtesy for those interested in forking that port
293 stars 117 forks source link

Stuttering with Uncapped Framerate enabled (SOLVED in DSDA-Doom commit 17f1b84) #400

Closed Kappa971 closed 2 years ago

Kappa971 commented 3 years ago

As described in the title, with uncapped framerate enabled, stuttering occurs. With a 60hz monitor, uncapped framerate enabled and vsync active, you will notice some pretty pronounced stuttering. With higher refresh rates, the problem is reduced but doesn't go away (at 144hz it is slight but still present). Owning a 144hz freesync monitor, I was able to test if, with uncapped framerate disabled (35 fps), stuttering was still present and it wasn't, so I guess there is something wrong with uncapped framerate code (probably remained unsolved for years). Woof! has no problems and It is very smooth, maybe it is possible to bring the part of the code concerning the uncapped framerate from Woof! to Prboom-plus? I want to specify that I have always had this type of problem with Prboom-plus, with all the versions I have tried (2.5.1.5, 2.5.1.5um, 2.5.1.6um, etc.), with different PCs and monitors, and it happens both on Windows (main PC) and Linux (on an old laptop).

EDIT Ticket #379 could be about the same problem, I have opened a new issue to try to better explain what happens.

Kappa971 commented 3 years ago

This is what I'm talking about:

https://user-images.githubusercontent.com/62349018/138553577-6c08dd54-a2a5-4e5b-8017-3a1ed73d982d.mp4

In summary I can say that:

  1. On a 60hz monitor with vsync active (but the same thing happens with a freesync monitor with an fps limiter active at 60), there is very noticeable stuttering, both with render software and OpenGL.
  2. The higher the fps produced by the game, the more the phenomenon is reduced.
  3. By disabling uncapped framerate, at 35 fps on a freesync monitor, stuttering is not present. I am not an expert, but I think there may therefore be some problem with the interpolation code of PrBoom-plus/DSDA-Doom.
  4. Crispy/Woof interpolation works perfectly, would it be possible to bring their code into PrBoom-plus? I guess it might take a lot of work, but I think it would be worth it (and it would solve a historical problem of PrBoom-plus).
  5. It happens on both Windows and Linux, with different PCs and monitors, so everyone should be able to reproduce it.
ghost commented 2 years ago

Yeah, this matches my problem exactly. The code for frame interpolation is likely just borked. I second the crispy/woof code integration idea :^)

kraflab commented 2 years ago

I'd guess limiting the framerate to 60 is the problem. PRBoom+ measures the time it takes to render the screen and accounts for this in its interpolation. So, if it took 3 ms to render the last frame, it will interpolate to x + 3ms for the next frame, even though the time is at x. That way when it finishes displaying, the timing is correct. I'm not sure if that accounts for external programs blocking the render from hitting. Crispy doom doesn't do this prediction, so it has more delay (at least, I didn't find it). In other words, prboom+ may be too smart for its own good ๐Ÿ˜‰

ghost commented 2 years ago

Interesting. Maybe Proff knew something we don't, haha. Until there's a solution, I'll stick to Woof lmao

kraflab commented 2 years ago

I tried disabling the displaytime thing but this didn't change anything. With that disabled the code is basically identical to crispy. There's no magic here, it just gets the elapsed time and sets the fraction based on how far into the tic it is. I will dig a bit deeper though.

ghost commented 2 years ago

Sounds good. Keep us posted ๐Ÿ‘Œ

Kappa971 commented 2 years ago

I think I was clear but to be sure I repeat it. I talked about 60fps because at 60fps it is easier to notice the problem and because I believe that 60hz monitors are very popular. On a freesync monitor, even locking at 70fps or 80fps the problem is present so not something that only happens at 60fps. At over 120fps the stuttering is always there but it is much less pronounced.

EDIT On 144hz freesync monitor, I tested with keyframe set to 0 (default 60), vsync disabled, Uncapped framerate enabled and render limit for tic set to 2. With OpenGL rendering the fps are locked at 67 (shouldn't they be locked at 70fps?) and nothing changes, while with the software render the fps are locked at 69 (??), the game seems to be smoother but the stuttering occurs at irregular intervals. On a 60hz monitor I think the test is useless because more than 60fps would be produced and, without vsync, there would be tearing so in any case it would be difficult to understand if the game is smoother or not.

Kappa971 commented 2 years ago

I would remove all the interpolation code from PrBoom-plus, everything related to it, and I would start from scratch, maybe replacing it with Woof's one (you could create a new branch, I would be happy to test it). Although the PrBoom-plus/DSDA-Doom interpolation code is better written (at least from what I understand), in practice it works worse. From what I have seen also render limit for tic (DSDA-Doom) works in a strange way, it behaves differently between render software and OpenGL and doesn't solve the stuttering (with the software render, however, it reduces it, but it occurs at irregular intervals) as already described above. Anyway, these are just my opinions, then you programmers know better than me what is the best way to proceed ;)

kraflab commented 2 years ago

I don't think it's the interpolation code, I think there's some problem with time management and rendering outside of the interpolation code. I've been testing by tracking the fractional tic value used for interpolation and it doesn't change by a consistent amount (which is what would be needed for smooth interpolation). In other words, the outer game loop is diving into the interpolation code at a bad time. I don't know if this is enough to explain your stuttering problem though. Typically people play uncapped without vsync and get 100s of fps (software) or thousands (opengl) at which point I assume this is invisible.

Kappa971 commented 2 years ago

I don't think it's the interpolation code, I think there's some problem with time management and rendering outside of the interpolation code. I've been testing by tracking the fractional tic value used for interpolation and it doesn't change by a consistent amount (which is what would be needed for smooth interpolation). In other words, the outer game loop is diving into the interpolation code at a bad time. I don't know if this is enough to explain your stuttering problem though.

Any changes you make, I will be happy to test to see if the problem is solved ;)

Typically people play uncapped without vsync and get 100s of fps (software) or thousands (opengl) at which point I assume this is invisible.

Probably yes, this could explain why no one has ever reported it over the years. But playing in this way would have tearing (which is no less annoying than stuttering) as well as producing useless fps that do nothing but consume more current/heat up the PC.

kraflab commented 2 years ago

I'm not saying we shouldn't fix it, just an idea for why it hasn't been reported much.

There's some seriously weird stuff going on here ๐Ÿค” I stripped out all the fancy stuff (which means it's more or less identical to crispy) but there's some very strange behaviour that I can't explain. I'm displaying the standard deviation in the frame length while I play, and if I move the mouse, it goes up ๐Ÿ˜ฎ

Furthermore, it goes up when moving the mouse even when I'm dead and the mouse has no effect. This is the kind of thing that makes me think the problem isn't in the interpolation code (in this case, nothing is even being interpolated, it's just weirdness happening in the framerate...).

kraflab commented 2 years ago
FRAC STDDEV: 0.004337, AVERAGE: 0.583159
FRAC STDDEV: 0.004155, AVERAGE: 0.583305
FRAC STDDEV: 0.004198, AVERAGE: 0.583337
FRAC STDDEV: 0.004545, AVERAGE: 0.582809
FRAC STDDEV: 0.004661, AVERAGE: 0.583911
FRAC STDDEV: 0.004488, AVERAGE: 0.583420
FRAC STDDEV: 0.004612, AVERAGE: 0.583308
FRAC STDDEV: 0.004853, AVERAGE: 0.583533
FRAC STDDEV: 0.005235, AVERAGE: 0.582874
FRAC STDDEV: 0.005444, AVERAGE: 0.583974
FRAC STDDEV: 0.005302, AVERAGE: 0.583371
FRAC STDDEV: 0.005360, AVERAGE: 0.583308
ev_mousemotion
FRAC STDDEV: 0.007727, AVERAGE: 0.584631
FRAC STDDEV: 0.009704, AVERAGE: 0.583308
ev_mousemotion
FRAC STDDEV: 0.011408, AVERAGE: 0.584746
FRAC STDDEV: 0.013172, AVERAGE: 0.582755
ev_mousemotion
FRAC STDDEV: 0.014632, AVERAGE: 0.585507
ev_mousemotion
FRAC STDDEV: 0.015009, AVERAGE: 0.583844
FRAC STDDEV: 0.015334, AVERAGE: 0.583311
ev_mousemotion
FRAC STDDEV: 0.016750, AVERAGE: 0.584940
FRAC STDDEV: 0.018312, AVERAGE: 0.582785
ev_mousemotion
FRAC STDDEV: 0.019028, AVERAGE: 0.585110
ev_mousemotion
FRAC STDDEV: 0.018166, AVERAGE: 0.582909
FRAC STDDEV: 0.017793, AVERAGE: 0.583308
ev_mousemotion
FRAC STDDEV: 0.017555, AVERAGE: 0.583092
FRAC STDDEV: 0.017316, AVERAGE: 0.583308
ev_mousemotion
FRAC STDDEV: 0.015952, AVERAGE: 0.581824
FRAC STDDEV: 0.015802, AVERAGE: 0.582146
ev_mousemotion
FRAC STDDEV: 0.016089, AVERAGE: 0.584853
ev_mousemotion
FRAC STDDEV: 0.014437, AVERAGE: 0.582304
FRAC STDDEV: 0.013167, AVERAGE: 0.583313
ev_mousemotion
FRAC STDDEV: 0.013165, AVERAGE: 0.583311
FRAC STDDEV: 0.014234, AVERAGE: 0.581813
ev_mousemotion
FRAC STDDEV: 0.013919, AVERAGE: 0.584433
FRAC STDDEV: 0.012828, AVERAGE: 0.581912
FRAC STDDEV: 0.011624, AVERAGE: 0.583305
FRAC STDDEV: 0.011529, AVERAGE: 0.583131
FRAC STDDEV: 0.011435, AVERAGE: 0.583308
ev_mousemotion
FRAC STDDEV: 0.010532, AVERAGE: 0.582451
FRAC STDDEV: 0.010678, AVERAGE: 0.582025
FRAC STDDEV: 0.010080, AVERAGE: 0.583830
FRAC STDDEV: 0.008263, AVERAGE: 0.582144
FRAC STDDEV: 0.006414, AVERAGE: 0.583316
FRAC STDDEV: 0.005864, AVERAGE: 0.582965

Disabling the code that does anything with a mouse event has no effect, the existence of the mouse event itself causes a problem...๐Ÿคจ

JadingTsunami commented 2 years ago

Disabling the code that does anything with a mouse event has no effect, the existence of the mouse event itself causes a problem...๐Ÿคจ

You might try disabling mouse events entirely and see if that has any effect. I suppose it's possible that a ton of mouse events are filling up the event queue.

kraflab commented 2 years ago

Disabling the code that does anything with a mouse event has no effect, the existence of the mouse event itself causes a problem...๐Ÿคจ

You might try disabling mouse events entirely and see if that has any effect. I suppose it's possible that a ton of mouse events are filling up the event queue.

Looks like it happens with keyboard events too if there's a bunch, seems like this is at least caused by the event queue. Although, if that were the problem, it would affect crispy too, since it handles the events the same way.

JadingTsunami commented 2 years ago

Looks like it happens with keyboard events too if there's a bunch, seems like this is at least caused by the event queue. Although, if that were the problem, it would affect crispy too, since it handles the events the same way.

If the stuttering happens during a demo, you might try running a short demo with all SDL keyboard/mouse events disabled. If this toggles the stuttering on and off it would be a pretty clear sign that the SDL event queue is the problem.

I don't see this stuttering issue so I'll have to defer to others on this one.

rfomin commented 2 years ago

I think PrBoom+ doesn't have an event queue. PostEvent calls responders immediately.

JadingTsunami commented 2 years ago

I think PrBoom+ doesn't have an event queue. PostEvent calls responders immediately.

I'm talking about the SDL event queue, not any internal queueing.

kraflab commented 2 years ago

When I said event queue, I meant the SDL one. The event polling is what seems to be the problem. I've been planning on actually polling events more frequently and storing an internal queue as well in dsda-doom, which might do something about this, but it won't help for vsync since the program doesn't have control to poll events while waiting, right?

kraflab commented 2 years ago

@Kappa971 @srludicolo could you try this build out and see if it's any better or worse? You can set any fps limit to try in the menu and it has the simplified interpolation timing. I also improved the accuracy of the fps limiter so you might try setting that to 60 without vsync as another test. Maybe it does something for you. It includes the debug info, so if you run from a terminal it will also print out the standard deviation stuff I mentioned above. Unfortunately I don't really see the stuttering issue so it's hard for me to test myself.

https://drive.google.com/file/d/1wZH4rvwIBm3BI1nVxbweIidHE2x1st9W/view?usp=sharing

Kappa971 commented 2 years ago

@Kappa971 @srludicolo could you try this build out and see if it's any better or worse? You can set any fps limit to try in the menu and it has the simplified interpolation timing. I also improved the accuracy of the fps limiter so you might try setting that to 60 without vsync as another test. Maybe it does something for you. It includes the debug info, so if you run from a terminal it will also print out the standard deviation stuff I mentioned above.

Of course, I'm happy to help. Unfortunately I cannot help you in any other way because it is beyond my competence : (

Unfortunately I don't really see the stuttering issue so it's hard for me to test myself.

This is really strange, it is something that I still cannot understand. I have tested with several PCs with Windows 10, 11 and one with Linux (Debian) and in all of them there is the same problem... What is it that causes some to have stuttering while others not? Do you have Windows 7? Maybe with Windows 7 the problem is not present? mmh

JadingTsunami commented 2 years ago

This is really strange, it is something that I still cannot understand. I have tested with several PCs with Windows 10, 11 and one with Linux (Debian) and in all of them there is the same problem...

Are you using the same peripherals? The same mouse and/or keyboard for instance.

Kappa971 commented 2 years ago

Are you using the same peripherals? The same mouse and/or keyboard for instance.

No

@Kappa971 @srludicolo could you try this build out and see if it's any better or worse? You can set any fps limit to try in the menu and it has the simplified interpolation timing. I also improved the accuracy of the fps limiter so you might try setting that to 60 without vsync as another test. Maybe it does something for you. It includes the debug info, so if you run from a terminal it will also print out the standard deviation stuff I mentioned above. Unfortunately I don't really see the stuttering issue so it's hard for me to test myself.

https://drive.google.com/file/d/1wZH4rvwIBm3BI1nVxbweIidHE2x1st9W/view?usp=sharing

This solved the problem for me, stuttering seems to have disappeared. The function to limit the fps I find it better this way than before, it is simpler.

fabiangreffrath commented 2 years ago

In Crispy, I could achieve a significant increase in "smoothness" by moving the calculation of the tic fraction for interpolation immediately after rendering of the previous frame. Just saying. ๐Ÿ˜‰

https://github.com/fabiangreffrath/crispy-doom/commit/e2f5a2d3775e29794a666aa36489fff5dc57ce5b

kraflab commented 2 years ago

In Crispy, I could achieve a significant increase in "smoothness" by moving the calculation of the tic fraction for interpolation immediately after rendering of the previous frame. Just saying. ๐Ÿ˜‰

fabiangreffrath/crispy-doom@e2f5a2d

I saw that as one of the main differences, but that means that crispy has more delay, right?

Kappa971 commented 2 years ago

The problem is solved both with render software and OpenGL. I also tested it with the other PC (it has a 60hz monitor) with vsync enabled and that's fine.

EDIT: I noticed that the movement of the chainsaw isn't interpolated, is this a deliberate behavior?

kraflab commented 2 years ago

EDIT: I noticed that the movement of the chainsaw isn't interpolated, is this a deliberate behavior?

I believe the chainsaw is not updated as often as other weapons - that's just how it is in vanilla doom.

Kappa971 commented 2 years ago

I believe the chainsaw is not updated as often as other weapons - that's just how it is in vanilla doom.

Ok, I didn't know.

This fix will be applied in both PrBoom-plus and DSDA-Doom, right?

kraflab commented 2 years ago

The changes I did are here: https://github.com/kraflab/dsda-doom/pull/86/files

It can't simply be merged into prboom+ because dsda-doom manages time completely differently. Someone would either need to adapt it or to apply a different solution in prboom+.

kraflab commented 2 years ago

@Kappa971 if you wouldn't mind doing one final test for me, this is the build with the debugging removed and merged into master so it's up to date. With these kinds of things I'm always paranoid about what might break it so I want to make sure the debugging wasn't some how affecting the time in a good way ๐Ÿ˜„

https://drive.google.com/file/d/1UgDxwUdVj2z_xdkxTVGQnuKKkxnAaTJM/view?usp=sharing

And @srludicolo it would still be good to confirm if it works for you too when you are around ๐Ÿ™‚

Kappa971 commented 2 years ago

The changes I did are here: https://github.com/kraflab/dsda-doom/pull/86/files

It can't simply be merged into prboom+ because dsda-doom manages time completely differently. Someone would either need to adapt it or to apply a different solution in prboom+.

So do I have to keep the issue open until someone applies it to PrBoom-plus?

@Kappa971 if you wouldn't mind doing one final test for me, this is the build with the debugging removed and merged into master so it's up to date. With these kinds of things I'm always paranoid about what might break it so I want to make sure the debugging wasn't some how affecting the time in a good way ๐Ÿ˜„

https://drive.google.com/file/d/1UgDxwUdVj2z_xdkxTVGQnuKKkxnAaTJM/view?usp=sharing

Of course :)

EDIT: I have tested this build and it works fine :)

kraflab commented 2 years ago

EDIT: I have tested this build and it works fine :)

Thanks ๐Ÿ™Œ

ghost commented 2 years ago

And @srludicolo it would still be good to confirm if it works for you too when you are around ๐Ÿ™‚

It works! The issue is solved!

ghost commented 2 years ago

I recommend that this be merged with the UMAPINFO fork ASAP. If you decide to do that, I think a compiled development build on Doomworld or such would be great for the community members struggling with this problem :)

Kappa971 commented 2 years ago

I recommend that this be merged with the UMAPINFO fork ASAP

I already asked above and @kraflab replied. In practice, the fix must be "readapted" for PrBoom-plus. I'll keep the issue open in case anyone in the future is interested. I would switch to DSDA-Doom.

If you decide to do that, I think a compiled development build on Doomworld or such would be great for the community members struggling with this problem :)

I could upload a compiled development build of DSDA-Doom for Windows 10/11 64 bit to Doomworld but I don't know if anyone would be interested.

kraflab commented 2 years ago

I could upload a compiled development build of DSDA-Doom for Windows 10/11 64 bit to Doomworld but I don't know if anyone would be interested.

DSDA-Doom is supremely unstable right now, please don't do that ๐Ÿ˜‰ v0.22 will come out probably by the end of the month.

ghost commented 2 years ago

I already asked above and @kraflab replied. In practice, the fix must be "readapted" for PrBoom-plus. I'll keep the issue open in case anyone in the future is interested. I would switch to DSDA-Doom.

Shoot, sorry. I gotta read closer, geez. That's like the second time that's happened today.

ghost commented 2 years ago

Considering that DSDA-Doom and Prboom+ manage time differently, would some kind of conversion even be possible? I assume they both are built on a 35 TPS foundation, so maybe there's something I don't know ๐Ÿค”

Kappa971 commented 2 years ago

@srludicolo, I think you could close this issue: https://github.com/coelckers/prboom-plus/issues/379

ghost commented 2 years ago

Just closed it

fabiangreffrath commented 2 years ago

The changes I did are here: https://github.com/kraflab/dsda-doom/pull/86/files

It can't simply be merged into prboom+ because dsda-doom manages time completely differently. Someone would either need to adapt it or to apply a different solution in prboom+.

@kraflab Sorry to bother you about this, but could you please elaborate in a few sentences what are the relevant changes in this commit, so I could at leat try to adapt it to Pr+?

kraflab commented 2 years ago

Maybe just replace

https://github.com/coelckers/prboom-plus/blob/master/prboom2/src/SDL/i_system.c#L174-L183

With

frac = (fixed_t)((now - tic_vars.start) * FRACUNIT / tic_vars.step);

Clean up this test option: https://github.com/kraflab/dsda-doom/pull/86/files#diff-f94c2ac27426229d328c48beb16fc42f549e22f330bb834fbe58054233af6a0cL411-L412

You can delete all references to subframe, prevsubframe, displaytime, and start_displaytime.

Since prboom+ doesn't have a framerate limiter, the code related to that isn't applicable. Maybe just abandoning the displaytime estimation code is enough to stabilize it.