nukeykt / NBlood

Reverse-engineered ports of Build games using EDuke32 engine technology and development principles (NBlood/Rednukem/PCExhumed)
616 stars 78 forks source link

[PCExhumed/Rednukem/VoidSW] Micro Stuttering with refresh rate > 60hz (PCExhumed/Rednukem SOLVED) #522

Closed Kappa971 closed 3 years ago

Kappa971 commented 3 years ago

As described in ticket #468 (closed because it became too confusing), with monitors with a refresh rate higher than 60hz (tested with 144hz freesync monitor but I guess it also happens without freesync), PCExhumed and Rednukem have micro stuttering. VoidSW also suffers from framerate issues (doesn't look smooth despite fixed 144fps), but this is still in beta. In the old ticket, it is said that some changes have probably been made to Eduke32, which have probably been ported to Nblood but not to PCExhumed and Rednukem. It happens in both Windows and Linux. Raze runs smoothly at 144fps/144hz.

Kappa971 commented 3 years ago

I tested EDuke32 r7067 from 10 October 2018 (just before Rednukem was released), I also tested other revisions (r6962, r6580, r5999), all of these run smoother than Rednukem but less than the newer EDuke32 builds. Perhaps by finding out which revision of EDuke32 made the framerate improvements, it could be useful in solving the problems that plague PCExhumed and Rednukem

Kappa971 commented 3 years ago

I have tested other newer builds and found that the 2019-2020 builds behave identical to Rednukem, this up to r9166 (package name eduke32_win64_20200716-9166-32096f6c6). Whatever was done in EDuke32 r9202 (package name eduke32_win64_20200726-9202-edac43bc4), it fixed the framerate and micro stuttering problems. Hope this helps.

Tested builds: eduke32_win64_20200824-9249-175ae4cd5 [Smooth] eduke32_win64_20200802-9206-878f7d6eb [Smooth] eduke32_win64_20200727-9205-74f7b0da9 [Smooth] eduke32_win64_20200726-9203-94179a4dc [Smooth] eduke32_win64_20200726-9202-edac43bc4 [Smooth] eduke32_win64_20200716-9166-32096f6c6 [Micro stuttering] eduke32_win64_20200702-9159-dcc28cbf1 [Micro stuttering] eduke32_win64_20200528-9053-e784aa3e9 [Micro stuttering] eduke32_win64_20200406-8807-cad905dd5 [Micro stuttering] eduke32_win64_20200304-8681 [Micro stuttering] eduke32_win64_20191129-8344 [Micro stuttering] eduke32_win64_20181010-7067 [Not perfectly smooth] eduke32_win64_20180801-6962 [Not perfectly smooth] eduke32_win64_20180115-6580 [Not perfectly smooth] eduke32_win64_20170104-5999 [Not perfectly smooth]

CommonLoon102 commented 3 years ago

Wow, you put a lot of effort into this. I checked the NBlood repo for Blood, here are the commits I think have changed the microstuttering case: 442aefc6 a189828b

You can build and test after and before these commits and see if these are really the ones.

CommonLoon102 commented 3 years ago

There is no build for 442aefc on lerppu. But you can find the a189828 one.

This is for a189828: https://lerppu.net/wannabethesis/nblood/20200527-11613/nblood_win64_20200527-11613.zip

And this is the one right before it: https://lerppu.net/wannabethesis/nblood/20200527-11612/nblood_win64_20200527-11612.zip

You can check if there is microstuttering change between the two.

You need to compile 442aefc and the one before it yourself though.

Kappa971 commented 3 years ago

There is no known performance difference between nblood_win64_20200527-11612 and nblood_win64_20200527-11613 (a189828). Not even between these and the latest Nblood builds. I was unable to compile the build with commit 442aefc and the previous one (023ba96) on Visual Studio 2022 as there are many compilation errors (the latest build is compiled so it's not a Visual Studio issue).

It's really very strange, Nblood worked better than EDuke32 until July 26, 2020? It looks like a puzzle :) I'm not a programmer but, if I were you, I'd check what they did in EDuke32 r9202, even if it doesn't make much sense

CommonLoon102 commented 3 years ago

The changelog for EDuke32 r9202 says: Engine: fix sprite sorting issue that caused broken voxel rendering in polymost It is by nukey who also had this change in NBlood. Maybe this is causing the stuttering? It is a small change made in NBlood, and then it was backported to eDuke32. Here is the commit for NBlood: 888bda5c

There is no build on lerppu which only contains this change, so you would need to build yourself (what you are unable to do). This sprite sorting stuff is together with a lot of other changes from upstream in release r11612 (what you have already tried). So maybe try one before it, the r11576 one. It is here: https://lerppu.net/wannabethesis/nblood/20200526-11576/nblood_win64_20200526-11576.zip

But since this sprite sorting change is in the engine, it should affect all games, not just some of them.

CommonLoon102 commented 3 years ago

I tried building 023ba96 in VS 2019. The NMapedit project fails, but we don't need that. You only need to right click the nblood project and then click Rebild All. It went fine for me. If it still doesn't build for you, I recommend using VS2019 because 2022 is still just beta.

CommonLoon102 commented 3 years ago

Ah, sorry, you want to build rednukem, not nblood, and as I can see that fails too. Luckily, the fix is very simple though, see here: https://github.com/nukeykt/NBlood/commit/197c38fe2a6a05cfd35c1c9bb039dcbee6e9c095

CommonLoon102 commented 3 years ago

So I'm not even sure what we are trying to find. I guess we wanted to find the last NBlood version which microstutters, see what change fixed the stuttering and apply the change to rednukem. So you need to do a binary search for NBlood, and building and testing the versions and find the commit which fixes the stuttering. But we need to know first if NBlood stuttered at all for you for any version.

Kappa971 commented 3 years ago

I have tried nblood_win64_20200526-11576 and it works like nblood_win64_20200527-11612 and nblood_win64_20200527-11613 and like the latest build.

But since this sprite sorting change is in the engine, it should affect all games, not just some of them.

Then this commit is to be excluded. Are there any changes in EDuke32 r9202 only concerning EDuke32?

So I'm not even sure what we are trying to find. I guess we wanted to find the last NBlood version which microstutters, see what change fixed the stuttering and apply the change to rednukem. So you need to do a binary search for NBlood, and building and testing the versions and find the commit which fixes the stuttering. But we need to know first if NBlood stuttered at all for you for any version.

Actually I tested EDuke32 to understand what the micro stuttering in EDuke32 solved. This has been solved, from the tests done, in EDuke32 r9202 and I was hoping this could help bring the fix to Rednukem, PCExhumed and VoidSW. Nblood apparently worked fine before EDuke32 was fixed.

CommonLoon102 commented 3 years ago

I made a huge mistake. Somehow I checked the wrong changelog. The changelog for r9202 is here: https://dukeworld.com/eduke32/synthesis/20200726-9202-edac43bc4/ChangeLog.txt

And in it, this is very suspicious:

Duke3d: add an additional layer of camera vec3_t position smoothing across frames

This is intended to mask a slight jitter that occurs when strafing and turning at the same time as a result of turning being decoupled from movement.

I'll have a look in this.

CommonLoon102 commented 3 years ago

It (a359877b) is completely Duke Nukem 3D related, and it was reverted in 600ce46a.

Kappa971 commented 3 years ago

It (a359877) is completely Duke Nukem 3D related, and it was reverted in 600ce46.

So it's not. Excluding, the remaining commits are:

Duke3d: testing an alternate version of calc_smoothratio_demo() that returns values based on fractional tics

Duke3d: only allow doubled up game updates if we're behind at least 2 tics

Duke3d: remove the do {} while (0) from the VM_ASSERT macro This makes the macro less flexible, but I'm not sure that using __builtin_expect within the loop is doing what I want. I guess I should fire up godbolt.org...

Duke3d: remove a few instances of EDUKE32_PREDICT_FALSE

Duke3d: remove "printtimes" console command This information is pretty much useless compared to using the profiler.

Duke3d: split [get/set][wall/sector] into separate opcodes for direct access through VM_GetStruct()/VM_SetStruct() This makes them match [get/set]actor.

Duke3d: replace outdated "gamepad" terminology in menu with "controller"

Duke3d: make frag[] in DukeStatus_t unsigned

Duke3d: add STR_REVISION to CON_QGETSYSSTR It returns the binary's revision string without the "r".

Duke3d: allow setting the scale for the mouse axes to 0

Duke3d: update the types of the player's .horizAngleAdjust and .horizSkew to match current usage

Duke3d: remove dead function prototype

Duke3d: don't invalidate TILE_SAVESHOT until after G_ReadGLFrame() and don't set the cache1d entry lock byte to the unlocked state until the data has actually been used

Duke3d: fix issues with the player looking up/down after loading a save This limits the output of scaleAdjustmentToInterval() to a maximum of one second worth per frame drawn, clamps q16horizoff to the same range as q16horiz, and zeroes the last input clock after loading.

Duke3d: fix potential OOB memory access through player .wackedbyactor member

Duke3d: mark in_mousebias cvar as deprecated

glad: add GL_ARB_timer_query

Duke3d: move CON_SETVAR_GLOBAL/PLAYER/ACTOR out of #ifdef CON_DISCRETE_VAR_ACCESS These were verified as being useful via profiling.

CommonLoon102 commented 3 years ago

So it's not. Excluding, the remaining commits are:

It doesn't mean it is not. But you can try build eduke32 without that commit and see. It is still possible that that commit fixed the stuttering, and later they fixed the stuttering in a different way, and therefore this old fix was not needed anymore and that's why it got reverted.

Kappa971 commented 3 years ago

So it's not. Excluding, the remaining commits are:

It doesn't mean it is not. But you can try build eduke32 without that commit and see. It is still possible that that commit fixed the stuttering, and later they fixed the stuttering in a different way, and therefore this old fix was not needed anymore and that's why it got reverted.

Right. As soon as possible, I will try EDuke32 without that commit. By the way, where can I find it?

EDIT It Is this? https://github.com/nukeykt/NBlood/commit/ca76197c6ca48d4c4b5c86df53d155e45202dbfb

CommonLoon102 commented 3 years ago

I would recommend using the original git repo, the commit in question is here: https://voidpoint.io/terminx/eduke32/-/commit/a359877b2a01182f06e1db910c660a654a0727a2

Clone the repo with VS and then open the solution at \platform\Windows\eduke32.sln. Right click on master branch -> view history -> search for a359877b2, see the commit below it (ca76197c as you mentioned), right click, new branch, clean solution, rebuild solution, run, test. Then do the same with a359877b2.

CommonLoon102 commented 3 years ago

I don't want to disappoint you, but even if we find that a359877 fixes the stuttering for eDuke32, that won't mean much. A lot of other things can cause stutterings, and for rednukem it can be something completely different.

Kappa971 commented 3 years ago

There are some news that I hope will help to definitively solve this problem. I compiled the build with commit ca76197 and it is smooth so useless to test a359877. I compiled the build with commit fc43c04 and stuttering is present. I compiled the build with commit 9f44ddb (prior to ca76197) and stuttering is present, so commit ca76197 fixed the stuttering.

I hope you can apply the commit ca76197 to all Build Engine games.

Kappa971 commented 3 years ago

I found this commit https://github.com/nukeykt/NBlood/commit/0f928a1fd5eb23e37c48d7eb547a176ff9cf6eab and in EDuke32 source/duke3d/src/game.h it has been changed like this (so the commit ca76197 was changed):

duke3d

In Rednukem (source/rr/src/game.h) it is like this (similar to EDuke32 before commit ca76197):

rr

In the other Build Engine games I haven't been able to find this piece of code. Maybe the author of these commits can help make these changes to other games as well? I'm not a programmer so I think I can't do more than that :)

CommonLoon102 commented 3 years ago

It is not a simple issue at all. The eDuke32 dev team was struggling with stuttering issues in whole July. https://voidpoint.io/terminx/eduke32/-/issues/100 https://voidpoint.io/terminx/eduke32/-/issues/161 https://voidpoint.io/terminx/eduke32/-/issues/174

They fixed it first, it thought to be OK, but for some it wasn't, then they fixed it in some other way. Then reverted something and re-fixed. I cannot really follow it anymore, there are a lot of references to commits and everything in the mentioned issues. You can have your fun time there but I think this is still an open issue.

CommonLoon102 commented 3 years ago

NBlood doesn't use this calc_smoothratio function, that's why it is not stuttering. RR and Exhumed are using the old calc_smoothratio versions. I think if we would call the latest calc_smoothratio which was moved to the engine (into baselayer.h), the stuttering would be fixed in RR and Exhumed as well. Since I'm on Win7 where the issue (stuttering) doesn't happen (based on my perception and what I've read here: https://voidpoint.io/terminx/eduke32/-/issues/161), only you can test it. I will create a branch and you can try it.

Kappa971 commented 3 years ago

Thanks. While we may not have the solution yet, progress has been made. What about VoidSW? I guess it's like Rednukem and PCExhumed. I'll test the new branch when it's ready :)

CommonLoon102 commented 3 years ago

Here is the branch, you can test: https://github.com/CommonLoon102/NBlood/tree/cl102/calc-smoothratio-fix

I only applied the change for RR for now, if it solves the issue, the fix for Exhumed will be similar, I think.

Or you can just make the changes yourself, it is very little what was changed: https://github.com/CommonLoon102/NBlood/commit/51dcd759f496165f1b5c80a068c42b8a7ae3d82a

Kappa971 commented 3 years ago

Here is the branch, you can test: https://github.com/CommonLoon102/NBlood/tree/cl102/calc-smoothratio-fix

I only applied the change for RR for now, if it solves the issue, the fix for Exhumed will be similar, I think.

Or you can just make the changes yourself, it is very little what was changed: CommonLoon102@51dcd75

Fantastic! This solved the problem in Rednukem. I hope this fix can also be used in PCExhumed, VoidSW and other available games. Thanks!

CommonLoon102 commented 3 years ago

I've done the change for Exhumed too. I created a new branch which contains the fix for Rednukem and Exhumed as well. Can you please test Exhumed on your side? If it is OK, I'll open a pull request here. VoidSW must be fixed in upstream (at the voidpoint.io GitLab instance).

The branch: https://github.com/CommonLoon102/NBlood/tree/cl102/calc-smoothratio-fixes

Kappa971 commented 3 years ago

I've done the change for Exhumed too. I created a new branch which contains the fix for Rednukem and Exhumed as well. Can you please test Exhumed on your side? If it is OK, I'll open a pull request here. VoidSW must be fixed in upstream (at the voidpoint.io GitLab instance).

The branch: https://github.com/CommonLoon102/NBlood/tree/cl102/calc-smoothratio-fixes

Also for PCExhumed the issue is solved, thanks. As for VoidSW, do you warn the developers of the fix?

CommonLoon102 commented 3 years ago

Also for PCExhumed the issue is solved, thanks. As for VoidSW, do you warn the developers of the fix?

Yes, I'll let them know.

CommonLoon102 commented 3 years ago

Before I tell them anything, can you confirm if the change is fixing your issue in VoidSW? You need to modify draw.cpp: Change this line smoothratio = min(max(((int32_t) totalclock - ototalclock) * (65536 / synctics),0),65536); to this: smoothratio = calc_smoothratio(totalclock, ototalclock, 30); Then rebuild voidsw and test.

mjr4077au commented 3 years ago

to this: smoothratio = calc_smoothratio(totalclock, ototalclock, 30); Then rebuild voidsw and test.

Use 40 for the 3rd parameter, SW has a 40Hz timer 😉

Kappa971 commented 3 years ago

I replaced the line smoothratio = min (max (((int32_t) totalclock - ototalclock) * (65536 / synctics), 0), 65536); in draw.cpp with smoothratio = calc_smoothratio (totalclock, ototalclock, 40); (I also thank @mjr4077au) and solved the issue, but I also had to disable "on-disk texture cache" with the command r_texcache "0" otherwise there was stuttering (The issue was present even before the fix). Either way I think the fix works, it's smoother.

CommonLoon102 commented 3 years ago

Use 40 for the 3rd parameter, SW has a 40Hz timer

@mjr4077au What about PCExhumed? Is 30 OK there?

mjr4077au commented 3 years ago

Use 40 for the 3rd parameter, SW has a 40Hz timer

@mjr4077au What about PCExhumed? Is 30 OK there?

Yeah, it's 30. Witchaven is 40 as well, can't recall some of the other lesser titles

Kappa971 commented 3 years ago

If and where can I see if the fix has been applied to VoidSW? In the meantime I will continue to apply it myself, but I ask for it to eventually close this issue. Thanks

CommonLoon102 commented 3 years ago

Here: https://voidpoint.io/terminx/eduke32/-/blob/master/source/sw/src/draw.cpp#L2304 We can't fix it here, because NBlood doesn't contain Shadow Warrior (officially). It is maintained in a different project in a different version control system, see above. You need to open there an issue, or even a Merge Request.

This terminx guy mentioned in the URL above knows about the issue and the fix. He might apply it at some point. Or not. But he knows about it.

Kappa971 commented 3 years ago

OK thanks

Kappa971 commented 3 years ago

I opened a new problem on Gitlab: https://voidpoint.io/terminx/eduke32/-/issues/207, I hope I did well

CommonLoon102 commented 3 years ago

It is fine, they will know what to do.

Kappa971 commented 3 years ago

I'm sorry if I still haven't closed the problem, but unfortunately in the EDuke32 GitLab repository I see that things are working very slowly. There are fixes waiting to be merged for a year! (and this hasn't even been considered yet) Now I don't want to complain to you that you have nothing to do with it, indeed I can only thank you, but this is about replacing a line of code, I would do it so I would close the issue today but I don't think I have permission. I don't know if you will answer me, also because unfortunately you can't do anything about it, but it's just to let you know how things are going.

CommonLoon102 commented 3 years ago

You can learn git basics and open a Merge Request for your issue. There you can change that single line yourself and that would speed up the process. Then the maintainers would just need to click "Merge" and that's it.

You need to fork the eduke32 repo, then create a new branch from the master branch (name it sw-stutter-fix). Change that one line and push your changes. Then you can open a Merge Request for those changes. You can google the details.

You can do everything above with the GitLab web interface, no need to install anything onto your PC.

Kappa971 commented 3 years ago

You can learn git basics and open a Merge Request for your issue. There you can change that single line yourself and that would speed up the process. Then the maintainers would just need to click "Merge" and that's it.

You need to fork the eduke32 repo, then create a new branch from the master branch (name it sw-stutter-fix). Change that one line and push your changes. Then you can open a Merge Request for those changes. You can google the details.

You can do everything above with the GitLab web interface, no need to install anything onto your PC.

I will do a Google search, but I imagined that the maintainers would have implemented it immediately as the fix is ready and doesn't require massive changes. I'll try to do what you told me, but probably even done all this, it will still remain in the "Merge request" list for quite some time (some I've seen have been there for a year). Anyway thanks for the answer, always very helpful :)

Hendricks266 commented 3 years ago

I've seen your report, here and on the GitLab. I'll get to it, I need to fix the mouse sensitivity as well. My attention has been elsewhere recently.

Kappa971 commented 3 years ago

I created a merge request to fix the problem. I hope I have done everything correctly. @Hendricks266 could you check? https://voidpoint.io/terminx/eduke32/-/merge_requests/176

TerminX commented 3 years ago

I checked the merge request and something seems to be wrong with it that blocks it from rebasing against the current contents of the master branch. I don't know what could be wrong with it that would make that happen, being that the change is only one line. Regardless, I created a local commit for it and will manually push it to master later. Thanks for the contribution.

Kappa971 commented 3 years ago

I checked the merge request and something seems to be wrong with it that blocks it from rebasing against the current contents of the master branch. I don't know what could be wrong with it that would make that happen, being that the change is only one line. Regardless, I created a local commit for it and will manually push it to master later. Thanks for the contribution.

I helped find the problem, @CommonLoon102 and @mjr4077au created the fix. I don't know what's wrong with the merge request (never done this before), anyway thank you for creating a local commit. When this is applied, I will close all issues. Thanks

EDIT I think I have solved it, it should now be possible to merge to the master branch: https://voidpoint.io/terminx/eduke32/-/merge_requests/179

Kappa971 commented 3 years ago

Problem solved, thanks everyone.