godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
86.38k stars 19.24k forks source link

Forward Plus Renderer causes frameskips/jitter/judder/stutter, GPU frames aren't sorted correctly on Windows #84137

Open Cyangmou opened 8 months ago

Cyangmou commented 8 months ago

Godot version

4.1.3

System information

Windows 11, NVIDIA GeForce GTX 1070, i7-7700K CPU 4.20GHz, 1920x1080 60Hz IPS monitor & a 2560x1440 adaptive 100Hz monitor

Issue description

The forward plus renderer, causes jitters in movement. It's especially visible when V-Sync is switched on. This happens in 2D and 3D Those jitters are more or less pronounced, depending on which screen you use, they might not be very visible on a 60HZ screen but are still there.

!

when i talk about "jitter" i am talking about this effect happening in the video above or clearly observable in this older video here at 5, 8, 11 and 13 seconds: Video

The problem seems to be tha tthe renderer has a cache of 4 GPU frames and that the sorting of those frames is jumbled up. So basically by design we seem to have an reoccuring order of 0, 1, 3, 2, 3, ... 4, 5, 8, 7, 8... This means some frames don't get shown, others get shown twice and the judder is caused by a step back

Currently it's not clear if it's a problem with the way how the frames are put together, or if it's a driver related issue related to memory. It could be 2 bugs, as comment sin the thread show other bugs play into this

Steps to reproduce

Can this be circumvented?

No. It's a very critical and very deep sitting bug. With the forward plus renderer there is no way to circumvent this and it will happen on any hardware. THe higher the resolution, the less obvious the bug is, however it's always there. The forward plus renderer is simply broken and can't be used in the current state.

You could Use the gl_compatibility renderer, which doesn't have this problem, but this is not a good solution either.

Minimal reproduction project:

I made a simple project in which the character can only run left and right per key input. That's literally all the code we need to test the issue properly, the setup described above is key for making it visible.

Conclusion

  1. Only by enabling the gl_compatibility renderer the jitter seems to be solved. The visibility of the problem and the intensity the skips are happening is however different based on the settings.
  2. It happens in 2D and 3D
  3. Disabling V-Sync with the forward plus renderer makes issues less pronounced than with having V-Sync enabled...
  4. ...especially on a 60Hz screen on windows disabling the V-Sync leads to way less experienced jitter, it's still there though
  5. The problem happens with process function and with physics_process() function.

Hypothesis

I think the core of the problem lies with the forward plus renderer (There however might be additional problems with V_Sync, Camera or Physics)

Minimal reproduction project

Project Download This includes a small platformer project. Project settings need to be adjusted according to my tests to reproduce the issues.

golddotasksquestions commented 8 months ago

Would it be possible for you to use your phone filming your monitors to record all those various tests and post the videos here under each setting description?

We all have different hardware and it is close to impossible to fully replicate your result, or be able to tell if we have replicated the same result without seeing yours. If you record your monitors with your phone we would have the best chance to see what you are seeing as screen recording software would likely alter the result as well.

Cyangmou commented 8 months ago

Initial Tests (Outdated, lower in the thread are actual videos, much better than anything written here

-Forward Plus renderer was used -The camera is conneced to the character. -you can see the positioning issues just with moving the character -If we connect the camera to the character and have bunch of sprites in the game it's easier to see if there are visual positioning problems. -sprite scaling set to nearest neighbor -Display -> Window -> Size: Window Width Override: 0 -Display -> Window -> Size: Window Height Override: 0

Test 2.1: Basic window size, physics loop, 100%, v-sync on

-Display -> Window -> Size: Viewport Width: 640 -Display -> Window -> Size: Viewport Height: 360 -Display -> Window -> Stretch: Mode: viewport -Display -> Window -> Stretch: Scale: 1 -Display -> Window -> V-Sync: enabled -character code with func _physics_process(delta): -display scaling: viewport

Here the character displays the jitter It however is barely visible on the 60hz monitor and very visible on the 100 Hz monitor

Test 2.2: Basic window size, process loop, 100%, v-sync on

-Display -> Window -> Size: Viewport Width: 640 -Display -> Window -> Size: Viewport Height: 360 -Display -> Window -> Stretch: Mode: viewport -Display -> Window -> Stretch: Scale: 1 -Display -> Window -> V-Sync: enabled -character code with func_process(delta): -display scaling: viewport

Here the character displays the jitter It however is slightly more visible on the 60hz monitor than without phsics process and disturbingly well visible on the 100 Hz monitor

Test 2.3: zoomed window size, process loop, 300%, v-sync on

-Display -> Window -> Size: Viewport Width: 1920 -Display -> Window -> Size: Viewport Height: 1080 -Display -> Window -> Stretch: Mode: viewport -Display -> Window -> Stretch: Scale: 3 -Display -> Window -> V-Sync: enabled -character code with func_process(delta): -display scaling: viewport

Here the character displays the jitter, very clearly

Test 2.4: zoomed window size, physics process loop, 300%, v-sync on

-Display -> Window -> Size: Viewport Width: 1920 -Display -> Window -> Size: Viewport Height: 1080 -Display -> Window -> Stretch: Mode: viewport -Display -> Window -> Stretch: Scale: 3 -Display -> Window -> V-Sync: enabled -character code with func_physics_process(delta): -display scaling: viewport

Here the character displays the jitter, very clearly

Test 2.5: Basic window size, physics loop, 100%, v-sync off

-Display -> Window -> Size: Viewport Width: 640 -Display -> Window -> Size: Viewport Height: 360 -Display -> Window -> Stretch: Mode: viewport -Display -> Window -> Stretch: Scale: 1 -Display -> Window -> V-Sync: disabled -character code with func _physics_process(delta): -display scaling: viewport

Here the character displays no jitter

Test 2.2: Basic window size, process loop, 100%, v-sync off

-Display -> Window -> Size: Viewport Width: 640 -Display -> Window -> Size: Viewport Height: 360 -Display -> Window -> Stretch: Mode: viewport -Display -> Window -> Stretch: Scale: 1 -Display -> Window -> V-Sync: disabled -character code with func_process(delta): -display scaling: viewport

Here the character displays no jitter

Test 2.3: zoomed window size, process loop, 300%, v-sync off

-Display -> Window -> Size: Viewport Width: 1920 -Display -> Window -> Size: Viewport Height: 1080 -Display -> Window -> Stretch: Mode: viewport -Display -> Window -> Stretch: Scale: 3 -Display -> Window -> V-Sync: disabled -character code with func_process(delta): -display scaling: viewport

Here the character displays no jitter

Test 2.4: zoomed window size, physics process loop, 300%, v-sync off

-Display -> Window -> Size: Viewport Width: 1920 -Display -> Window -> Size: Viewport Height: 1080 -Display -> Window -> Stretch: Mode: viewport -Display -> Window -> Stretch: Scale: 3 -Display -> Window -> V-Sync: disabled -character code with func_physics_process(delta): -display scaling: viewport

Here the character displays no jitter

The whole issue also comes up in a ton of other issue reports with physics, sprite 2d and whatever. It#s not only me experiencing those issues, I also have it on my laptop and found a bunch of similar bug reports, but I found out that it's the renderer

If I use the compatibility renderer all is smooth

So by sheer logic that's where the issue must lie.

reduz commented 8 months ago

I am not familiar with the code, but I hypothesize that the swapchain is using the wrong monitor hz for the window.

Cyangmou commented 8 months ago

Also because it came up: changing the project settings to fullscreen also produces the jitter, having fullscreen without forward plus v-sync runs smooth again.

Cyangmou commented 8 months ago

I got another very interesting observation: After I was switching the project settings to fullscreen mode and then setting them back to windowed the game suddenly ran smooth. Just before it did not run smooth in the fullscreen.

So it also could be that the V-Sync is fine, but just something with the initial initialization of the windowed mode Hz might be wrong. I started this as a clean project and did not touch the window mode, just let it run in windowed. Now for this project it seems to be fixed (not the uploaded one, but the one I had on my HD)

I quickly created another new project and the issue again applies

My main system monitor is the 100Hz monitor, my secondar ymonitor is the 60Hz monitor.

Exact steps which accidentally seem to solve this issue: 1.) switching the renderer to compatibility renderer, save project, restart 2.) switching the render back to forward_plus renderer, save project, restart 3.) set window mode in the project settings to fullscreen, again choose viewport in the stretch mode settings, playtest 4.) set window mode in the project settings back to windowed, playtest (now it ran smooth for me)

chutneyio commented 8 months ago

Hi @Cyangmou , I have just run the demo and got jittered whenever stretch mode set to viewport no matter v-sync is enabled or not. After changing to canvas_item there is no jitter anymore. I only have a 60 Hz monitor so this could be a multiple monitors problem.

Btw, I'm usingSubviewport in a top-down game to deal with jittering because canvas_item will not work with diagonal movement. This is the demo:
JitterTest_Subviewport.zip

I'm kinda concerning about the multiple monitors issue but I could not verify it because I don't have the setup to test myself so your result on this would be much appreciated!

Cyangmou commented 8 months ago

I just ran that linked demo how it came out of the box. I performed all tests on my 100Hz 2560x1440 monitor

Test 1: out of the box jitter

what is different here: Window Stretch Mode: disabled Renderer: forward plus Window Height Override: 1920 Window Width Override: 1080 V-Sync: enabled

Test 1: disabling V-Sync very soft, barely noticeable jitter, jitter became a lot softer than with v-sync enabled but is not gone

Test 2: v sync enabled, but using process instead of physics process solved the jitter, I could not detect any jitter

Test 3: v sync diabled, physics-process a few stutters could be observed

So with this viewport override it seems to make a difference if you use a physics process or no physics process As with the physics process the jitter is way more pronounced than with a normal process But in both cases the jitter can be observed!

Also I additionally tried then to set the stretch mode to viewport and to canvas items in both cases I would neithe rsee the gameworld correctly positioned nor the character, only the numbers running you added in the output.

chutneyio commented 8 months ago

@Cyangmou Thanks so much!

So with Subviewport I can still achieve pixel perfect on multiple monitors setup with v-sync enabled by using _process for both player and camera as you mentioned in Test 2 right? Don't know if this is still a bug or not because I expect movement code with physics to happen in _physics_process. But at least there is a workaround for it.

Cyangmou commented 8 months ago

The point is that not using the physics process further in dev could drag down the game, as running physics operations in frames is costly. I mean smoothness could be the case with this small project on my machine, which is probably more powerful than the machine most people play those small indiegames on. But if the game scales up and you put all of your stuff in the process loop, it's literally in terms of performance an insanely bad idea.

I would not call it a solution, rather something which unexpectedly worked. The important parts would be the physics ticks.

chutneyio commented 8 months ago

@Cyangmou I don't know how costly physics operations is but the way Godot handle pixel perfect is very similar to GameMaker to me. There is no _physics_process function in GameMaker, only Step event that is very much like _process, and I need to do almost the same things in GameMaker to make the game pixel perfect (scale the application surface, rounding the camera position). Also _process in Godot run like 60 times per second so I don't think there will be problem if we put all the code in _process.

Cyangmou commented 8 months ago

You can of course do it. It's just neither professional nor smart to do it. And yes I agree about GameMaker, but GameMaker really scales badly for big projects and that's why it's uninteresting for any medium to bigger sized game. There is a reason why a physics tick exists and why both Unreal and Unity which have the bigge rshare on the market make use of this optimization.

chutneyio commented 8 months ago

Oh @Cyangmou wait... In my demo I forgot to change player to _physics_process which results in player and camera running in different update function. Did you test the case where both of them using _physics_process yet? Edit: Nevermind, player is using _physics_process already, just my mistake after messing things around.

CattyClouds commented 8 months ago

I am also experiencing this issue and did some testing and may have found a hint to the issue. It appears that when changing the Max FPS setting to be 60 to bump up against VSync's max stepping, the issue seems to disappear. I'm not 100% sure if it actually disappears after looking at it for so long, but, the jitter is 99% reduced in conjunction with the Physics Jitter Fix setting at 0.5 (default).

Monitor 1: 2560x1440p @ 60hz Monitor 2: 2560x1440p @ 60hz Godot: v4.1.2.stable.mono.official [399c9dc39]

Settings changed: Project > Project Settings > General Tab > Application > Run > Max FPS Project > Project Settings > General Tab > Display > Window > V-Sync > V-Sync Mode

When VSync is enabled and Max FPS is set to 0, the jitter is there. When VSync is enabled and Max FPS is set to 60, the jitter disappears When VSync is enabled and Max FPS is set to 61, the jitter is there

When VSync is disabled and Max FPS is set to 0 (goes to around 3k+), the jitter disappears When VSync is disabled and Max FPS is set to 90, the jitter disappears

Cyangmou commented 8 months ago

Yep, if you lock a 60Hz game on a 60Hz screen and your hardware can blow the specs, for sure it's running smooth. I mean that's within the expectation. But then it's running smooth for you only and it's not an overarching solution for releasing a game for a lot of different hardware setups. This is not even a bandaid, it's just that with this specific setting and setup the issue would be greatly reduced.

Cyangmou commented 8 months ago

Ok, I decided to run other tests with setups with 4 different (close to) 60HZ screens

On Windows you can check in the Advanced display settings the exact Hz the screen runs.

Adding those 2 lines of code in godot should give us a good idea about the return

print(DisplayServer.screen_get_refresh_rate(0))
print(DisplayServer.screen_get_refresh_rate(1))

Setup 1:

screen1: 1920x1080, on Windows 11: 59,95Hz screen2: 1920x1080 on Windows 11: 60Hz

results in: 59,95 Hz screen results in a value Godot returns of 59 60.00 Hz screen results in a value godot returns of 60


Setup 2:

59,97 Hz screen results in a value godot returns of 60 59,95 Hz screen results in a value godot returns of 59

Conclusion:

Jitter could be experienced on all screens What's interesting about the return number of the engine is that it's an integer. It's not aligning with the system information. The rounding when sorted also looks totally weird: 60.00 Hz screen results in a value godot returns of 60 59,97 Hz screen results in a value godot returns of 60 59,95 Hz screen results in a value godot returns of 59

Something is really really off here This might or might not have an impact on the issue, i don't know if the output of the DisplayServer.screen_get_refresh_rate() is rounded or not, but I have the feleing it should align with the OS information, which it doesn't

RPicster commented 8 months ago

I did some tests on my own. I can confirm that there is definitely something very broken here.

I have a lot of experience with pixelart games in Godot 3 and never had the feeling that anything was problematic there and so far have not worked with pixelart in Godot 4.

I "ported" the MRP to Godot 3 and used settings that were as close as possible to the MRP. I disabled camera smoothing on both projects as I found it makes the issue much more obvious.

Here is footage that is slowed down to 1000% from both versions: Testing it in Godot 4 I had "position skips" very, very regulary where the sprite skipped a pixel and jumped to the next pixel immediately. And even when not skipping, it didn't feel as smooth as the test in Godot 3. jitter_godot4 It didn't always skip a pixel of course... jitter_godo4-2

In Godot 3 I was not able to reproduce any of these problem - it was super smooth. jitter_godot3

My screens are both 144 fps and I disabled VSync for both tests.

⚡⚡⚡ EDIT: Sorry, my bad. I enabled pixel snapping for transforms which led to these bad stutters, without this setting I have smooth movement. ⚡⚡⚡

Cyangmou commented 8 months ago

Here is a lossless recording with camera smoothing switched off There are multiple framelosses, which are very hard to see unless your eye is trained and a 2 frame loss at around 15 seconds, when the character is mid jump - focus on the environment and this will be really easy to spot.

60hz screen forward plus renderer 1920x1080, 300% windowed mode, stretch mode viewport v-sync enabled

lossless-video-recording

Cyangmou commented 8 months ago

Edited the name that the description of the issue is in line with all known test results.

byte-arcane commented 8 months ago

Regarding the issue of refresh rate appearing incorrectly, it looks like that Godot uses DEVMODEW here and this structure seems to used a DWORD for the storage of the refresh rate, which means that it can't deal with fractional refresh rates.

Instead, the structure DWM_TIMING_INFO seems to provide "UNSIGNED_RATIO rateRefresh;" which supports fractional refresh rates, so this looks like it should be used instead

RPicster commented 8 months ago

Here is a lossless recording with camera smoothing switched off There are multiple framelosses, which are very hard to see unless your eye is trained and a 2 frame loss at around 15 seconds, when the character is mid jump - focus on the environment and this will be really easy to spot.

60hz screen forward plus renderer 1920x1080, 300% windowed mode, stretch mode viewport v-sync enabled

lossless-video-recording

I watched the footage very carefully... had to do it a couple of times to spot it. But I put my observation into a short gif so it's very obvious.

Frame 7 is dropped/skipped 2023_10_30_14_04_17

belzecue commented 8 months ago

it looks like that Godot uses DEVMODEW here and this structure seems to used a DWORD for the storage of the refresh rate, which means that it can't deal with fractional refresh rates.

Godot 3.5.3 similarly uses DEVMODW, and I note that _MonitorEnumProcRefreshRate is unchanged between 3.5.3 and 4. Does 3.x have this issue too?

Cyangmou commented 8 months ago

Can't say as the first Godot version I use is 4.x All tests I performed were on 4.x Somebody else would need to confirm this.

Cyangmou commented 8 months ago

I can confirm that it's also the case for 3D games. @RPicster built a level in the project for 3d I tested the setups. Same problems as for 2D apply. It's a renderer / screenrate refresh issue.

Project Lossless Video In the video you can see a bad stutter right at the start and one at the end, there are a few single frame losses inbetween. So exact same behaviour as 2D

For further tests anyone wants to perform, I personally think that testing with a low resolution is the best. My recommendation would be something like 160*80 scaled by 12, as something like this would make the frameloss mistake way better visible. The higher the resolution and the smaller the steps between frames, the more this mistake gets hidden. It's however always there.

RPicster commented 8 months ago

2023_10_30_14_04_19 I checked the footage and again could spot one very, very obvious frame drop. I highlighted the motion difference with red.

akien-mga commented 8 months ago

Regarding the issue of refresh rate appearing incorrectly, it looks like that Godot uses DEVMODEW here and this structure seems to used a DWORD for the storage of the refresh rate, which means that it can't deal with fractional refresh rates.

Instead, the structure DWM_TIMING_INFO seems to provide "UNSIGNED_RATIO rateRefresh;" which supports fractional refresh rates, so this looks like it should be used instead

That's an interesting finding, but it shouldn't be related to this bug in the end. DisplayServer::screen_get_refresh_rate() is a convenience method for users, but it's not used anywhere internally in the engine. So if it reports a wrong refresh rate, that's worth tracking in a separate bug report but it shouldn't influence the issue with VSync in the RenderingDevice backends.

bruvzg commented 8 months ago

Regarding the issue of refresh rate appearing incorrectly, it looks like that Godot uses DEVMODEW here and this structure seems to used a DWORD for the storage of the refresh rate, which means that it can't deal with fractional refresh rates.

Instead, the structure DWM_TIMING_INFO seems to provide "UNSIGNED_RATIO rateRefresh;" which supports fractional refresh rates, so this looks like it should be used instead

This seems to be correct, DEVMODEW can't return a fractional rate. DwmGetCompositionTimingInfo / DWM_TIMING_INFO is only usable for the main display (window argument was removed since Windows 8.1), so getting a proper fractional rate will likely require use of CCD API (DISPLAYCONFIG_PATH_TARGET_INFO).

Calinou commented 8 months ago

@RPicster What did you use to record the footage? Are you 100% certain there were no frame drops or jitter incurred due to the recording software?

Cyangmou commented 8 months ago

@Calinou the videos I made were recorded with OBS screen recording, set to lossless, framerate same as my screen on a 60hz monitor to ensure consistency and reproduction for the broadest amount of users. Also the video is way more soft, than the perception of the frameskips on the actual screen.


I just learned of another problem which might have also derailed the 2D tests a bit. As framejumps however were experienced during motion, I don' tthink this interferes with the renderer, but is another thing which needs fixing for smooth rendering.

apparently the camera is rendering 1 frame too late:, this issue here: issue #74203 NOTE: I just foun dout that this project uses the mobile renderer, so might or might not be an issue with the forward plus renderer I also assume the same 1 frame delay is happening with the 3D camera, for the reason that it probably is way harder to see in 3D, this we would need to test with a low-res 3D setup to be 100% sure.

RPicster commented 8 months ago

@RPicster What did you use to record the footage? Are you 100% certain there were no frame drops or jitter incurred due to the recording software?

I didn't record this footage - I can't reproduce the bug here. I just looked through the footage very closely and tried to spot it. It was hard for me to see the problem on the video, so I looked at it frame by frame in After Effects and tried to make it very clear where it happens.

Deozaan commented 8 months ago

Sorry if this is a stupid/invalid question, but does the jitter still occur when using Godot's built-in video recorder?

Cyangmou commented 8 months ago

Sorry if this is a stupid/invalid question, but does the jitter still occur when using Godot's built-in video recorder?

I decided to use an external video software, because I don't know if the Godot video editor uses the frames the GPU produces or if it's capturing it from the screen output.

Since with OBs I know that at lossless quality it just captures every frame the screen puts out, I know that it will be correct. I'd like to stay on topic of the Jitter, which is clearly experienced by so many people, I don't want to go down a rabbithole if the screen captuere I made is correct or not, I only provided it on request. The mistake is there.

thygrrr commented 8 months ago

I can confirm strong jitter in 3D with Vsync on, even at 120 Hz. It's much better with Vscnc off. In both Godot 4.1.x and 4.2.x.

There's also no visible tearing whatsoever with Vsync off, which makes me think Vsync in general isn't really communicated right to the driver? (I just checked: I have Gsync turned explicitly off, and would expect tearing.)

It appears as if delta is just outright wrong (perhaps almost as if the wrong portion was kept?).

However, this is with the Windows compositor (windowed / full window), so maybe that is the culprit in the errors of the delta calculation. So possibly the wrong value/signal is taken from the driver/os/vulkan to delineate the actual frame flip time.

Smooth delta does not improve the jitter, I feel it draws it out even more. The jitter is so intense that when writing a smooth camera follow is impossible with Vsync on, because it will appear to judder back and forth all the time. (the back & forth is likely a visual illlusion, it just lags behind because it can't properly calculate its position in the game world for the next frame, because the current delta value is all over the place and incorrect - I'm using an algorithm that cannot overshoot)

Nukiloco commented 8 months ago

I'm exactly getting this issue, even without vsync enabled. I'm using linux mint with x11 currently which might have to do with this issue instead https://github.com/godotengine/godot/issues/62821 but im not entirely sure. It doesn't matter if the project is being debugged in the editor, or exported with debug or release mode. The issue seems to still happen.

Cyangmou commented 8 months ago

@Nukiloco I just edited the bug description, as over the last days we found out it's a problem with the renderer. If you use the compatibility renderer instead of the forward plus renderer (game settings -> renderer) do you still experience the issue on your system?

Nukiloco commented 8 months ago

@Cyangmou The issue is still happening even in compatibility mode.

Cyangmou commented 8 months ago

@Nukiloco Thanks for confirming, I got a bunch more questions. First I assume you got multiple screens (issue 62821). 1.) Do those screens have different Hz amounts?

Does the issue still happen on Linux with: 2.) a single screen connected and compatibility renderer? 3,) a single screen connected and forward plus renderer?

Nukiloco commented 8 months ago

@Nukiloco Thanks for confirming, I got a bunch more questions. First I assume you got multiple screens (issue 62821). 1.) Do those screens have different Hz amounts?

Does the issue still happen on Linux with: 2.) a single screen connected and compatibility renderer? 3,) a single screen connected and forward plus renderer?

The screens do not but because of an X11 issue, one of them is forced to run at 60 HZ. I know that there is ways to bypass the issue, though for me on Linux Mint, this will just freeze the monitor instead.

Edit: It seems like a newer version of Linux Mint fixed the monitor issue. Though both monitors now running at 144 HZ doesn't fix the issue still. Still nice to know that I can now run my second monitor at 144 HZ.

I unplugged my second monitor and tested the project, it still was stuttering with forward mode and compatibility mode.

Calinou commented 8 months ago

On Linux, make sure you're playing in fullscreen as frametimes will be a lot less stable in a window, even with compositing disabled. (This is not exclusive to Godot.)

You can check this using MangoHud's framerate graph.

Nukiloco commented 8 months ago

Just tested it out with fullscreen mode with both forward and compatibility with one monitor and I'm still having stutter issues. And I also had mango hud.

Cyangmou commented 8 months ago

Hmm then i don't know what this is, it might be just related to Linux. Point stands, there are stutter issues.

thygrrr commented 8 months ago

Here's a reproduction project from me that cycles through all VSYNC modes. jittery-vsync-bug-repro.zip

Just hit play, it will cycle through each with 5 seconds in between each switch.

Result (tested on 60 and 120 Hz display, the bug is consistent)

There are 2 objects because I had one of them calculate its own delta time from microseconds, suspecting a delta time issue - there is one, but it is unrelated to the visual judder.

(Windows) Video of VSYNC_ENABLED, judder visible

(recorded at 8x slow motion on 120 Hz display, note the peculiar motion at 3 and 6 seconds, which almost seems like the object is jumping forward and back again; it may be an optical illusion caused by a simple frame skip, but read my theory at the end of the post)

https://github.com/godotengine/godot/assets/8904620/397d1df2-dc21-4b54-b33d-3f207c22f9f4

(Windows) Video of VSYNC_MAILBOX, very smooth

(also 8x slow motion & 120 Hz, looks similar for VSYNC_DISABLED, including that there is never any discernible tearing which would be expected: Thus, VSYNC_DISABLED may erroneously be using a fence/semaphore somewhere in the render process or driver. However, G-Sync is turned off, and there's also no tearing on my second monitor, which is a 60 Hz non-VRR model)

https://github.com/godotengine/godot/assets/8904620/f2fd5163-07b0-4bb2-a11e-46f214497290

Interestingly, the engine-delta object (using delta from _process) will start lagging in the vsync and adaptive vsync modes (I would expect it to be ahead, or equal), yet it will mysteriously catch up when mailbox is on, or vsync is disabled. I cannot explain this, if there was a discrepancy introduced during the vsync mode switches, that error would just stay and accumulate, not go away again. (due to how the code is written - time just accumulates individually for each object, and is never reset)

You also can't record this with the builtin recorder, because that messes with delta as well.

There's also a slight amount of drift you may observe over time, which probably shouldn't be there in a perceptible fashion, either.

Not a bug: If you move the window, put it on another screen, or anything that causes delta to get frozen vs. the wall clock / Time.usec value, then it will go totally out of sync - one might argue that there either the delta or usec value is wrong in that case. This is probably due to _provess delta being clamped to a maximum value, so works as expected.

Theory?

Perhaps an out-of-order image is submitted to the swap chain. That might explain the weird "back & forth" jitter one seems to perceive. If the "back & forth" is an optical illusion (and is only a "lag/skip" judder with catchup), then the theory would be that an image simply isn't submitted to the swap chain at all, or doesn't make it through presentation. Either assumption could also begin to explain why the wallclock (custom delta ) dependent object also jitters.

Edit: Frames are indeed shown out of order, see below. 🤯

Cyangmou commented 8 months ago

@thygrrr - thanks for the great explanation. And the videos. This is exactly what I was complaining about, plus some new nice insights and better explanations.

thygrrr commented 8 months ago

I made a quick experiment and spun it up in an nonlinear video editor to really look at those frames.

Turns out...

Frames are often skipped, shown out of order, or repeated

https://github.com/godotengine/godot/assets/8904620/7d55abb8-e767-468a-836b-b3a6b99df7d8

... 225 226 227 226 repeated 227 repeated 228 skipped 230 too early 229 230 repeated 231 skipped 232 skipped 233 too early 234 235 234 repeated 235 repeated ...

I hope this insight helps the renderer gurus debug this issue!

There's a chance that this shuffling also happens behind the scenes for other vsync modes, but because there are many more (to the tune of 5x to 10x more on my machine) images actually rendered with VSYNC_DISABLED or VSYNC_MAILBOX, both the perceptible error is much smaller and repeats are maybe less likely to be obvious as well (basic stochastics, there is only 1 slot in the mailbox that holds one of 10x as many frames). I'll try to capture that on video to confirm, but I'm at the limit of what my phone camera can do. :)

Gnumaru commented 8 months ago

@thygrrr Are you using Engine.get_process_frames() to update the label text? or are you incrementing a script variable each frame? How about testing with two labels, one using Engine.get_process_frames() and other with a script property incremented each frame? There is also Engine.get_frames_drawn(), but I don't know what's the difference between it and get_process_frames (except for what is already stated in the docs, that get_frames_drawn always return 0 on headless platforms and when the render loop is disabled).

thygrrr commented 8 months ago

I'm incrementing a script variable in _process, because that's where I also calculate the positions for the cubes. If it's the same number, it's the exact same content (the same number NEVER occurs twice at runtime, these repeat and out of order images are definitively repeated and/or out of order)

Either should, at the very least, grow strictly monotonically.

I don't think we can trust the Engine functions here, and I still have no theory how the same image can get submitted to the swapchain twice. (I have a mental model for out of order submission, but not for repeat submission 🤔 )

EDIT: I tested it, the values are the same (constant difference between my frame count and Engine.get_frames_drawn(); I test this right after I increment mine, so 1 is the expected value)

thygrrr commented 8 months ago

Culprit is possibly some sort of thread race condition. (big surprise, I know 😬)

However, mind that I observe this is with Single-Safe or Single-Unsafe threading model, as Multithreaded renderer just crashes in my 4.2 build!

Let's look at...

RenderingDeviceVulkan::swap_buffers()

I think that function is the actual culprit here, because it uses macro _THREAD_SAFE_METHOD_ but never actually uses the Mutex and there are possibly a few issues with the way MutexLock may be operated.

I found this by uncommenting the print_line("swapbuffers?"); in the RenderingDevice's callee VulkanContext::swap_buffers(); and appending a parameter there (it doesn't help without the parameter!?).

Sadly, adding the appropriate _thread_safe_.lock() and _thread_safe_.unlock() calls will not resolve the issue; even using _global_lock() will not stop frames from getting shuffled.

Things get weird:

(Schrödinger's Bug weird, I'm still new to Godot and its Multithreading):

This improves (bordering on fixes) the judder:

diff --git a/drivers/vulkan/rendering_device_vulkan.cpp b/drivers/vulkan/rendering_device_vulkan.cpp
index a9f77c9072..1892481f83 100644
--- a/drivers/vulkan/rendering_device_vulkan.cpp
+++ b/drivers/vulkan/rendering_device_vulkan.cpp
@@ -8753,6 +8753,7 @@ void RenderingDeviceVulkan::swap_buffers() {

        screen_prepared = false;
        // Swap buffers.
+       print_line("swapbuffers is magic? ", frame);
        context->swap_buffers();

        frame = (frame + 1) % frame_count;
@@ -8998,6 +8999,7 @@ void RenderingDeviceVulkan::initialize(VulkanContext *p_context, bool p_local_de

        context = p_context;
        device = p_context->get_device();

        if (p_local_device) {
                frame_count = 1;
                local_device = p_context->local_device_create();

Classic race condition shenanigans ensue: This magical log statement it ONLY fixes it if the console that runs the process / shows this output is fully visible on the screen. (using Windows Terminal)

It does NOT work with the console window on the screen entirely, and no print_line statement. Also, print_line kind of either has to be at the beginning of VulkanContext::swap_buffers() or right before it is called.

There's also no way to short cirquit whatever print_line seems to do, it doesn't directly seem related to the calls to _global_lock(), but rather some OS stuff.

This console madness affects both the _console and vanilla Godot executables. (testing mostly with mono here, but the bug also exists on pure GDscript Godot)

thygrrr commented 8 months ago

This race condition is weird. This patch reliably removes the jitter (but still has occasional stutter) for me (but leads to some strange delta time deviations - only backwards in time, not forward, probably relating to smoothed delta time in the engine?)

Oftentimes JUST the two log statements alleviate the issues (but somehow not always, or not precisely).

I added debug info to see if the frames go out of sync, and they do not. (however, instead of 2 and 3 frames, we render 3 and 3 frames, because the chain should be the right size (I don't quite understand where the "frames" from the rendering device are actually associated with the images, but I believe the bug may be somewhere around there.

diff --git a/drivers/vulkan/rendering_device_vulkan.cpp b/drivers/vulkan/rendering_device_vulkan.cpp
index a9f77c9072..5b8eac1063 100644
--- a/drivers/vulkan/rendering_device_vulkan.cpp
+++ b/drivers/vulkan/rendering_device_vulkan.cpp
@@ -8753,7 +8753,8 @@ void RenderingDeviceVulkan::swap_buffers() {

        screen_prepared = false;
        // Swap buffers.
-       context->swap_buffers();
+       print_line("device  frame: ", frame);
+       context->swap_buffers(frame);

        frame = (frame + 1) % frame_count;

@@ -8998,12 +8999,13 @@ void RenderingDeviceVulkan::initialize(VulkanContext *p_context, bool p_local_de

        context = p_context;
        device = p_context->get_device();
+
        if (p_local_device) {
                frame_count = 1;
                local_device = p_context->local_device_create();
                device = p_context->local_device_get_vk_device(local_device);
        } else {
-               frame_count = p_context->get_swapchain_image_count() + 1; // Always need one extra to ensure it's unused at any time, without having to use a fence for this.
+               frame_count = p_context->get_swapchain_image_count(); // no +1 here, the swapchain should be sized correctly instead
        }
        limits = p_context->get_device_limits();
        max_timestamp_query_elements = 256;
diff --git a/drivers/vulkan/vulkan_context.cpp b/drivers/vulkan/vulkan_context.cpp
index 7a397a170d..64e3465c6b 100644
--- a/drivers/vulkan/vulkan_context.cpp
+++ b/drivers/vulkan/vulkan_context.cpp
@@ -2072,7 +2072,7 @@ Error VulkanContext::_update_swap_chain(Window *window) {
        // buffering.
        uint32_t desiredNumOfSwapchainImages = 3;
        if (desiredNumOfSwapchainImages < surfCapabilities.minImageCount) {
-               desiredNumOfSwapchainImages = surfCapabilities.minImageCount;
+               desiredNumOfSwapchainImages = surfCapabilities.minImageCount + 1; //best practice,avoid waiting on the driver to complete internal operations.
        }
        // If maxImageCount is 0, we can ask for as many images as we want;
        // otherwise we're limited to maxImageCount.
@@ -2468,12 +2468,13 @@ Error VulkanContext::prepare_buffers() {
        return OK;
 }

-Error VulkanContext::swap_buffers() {
+Error VulkanContext::swap_buffers(int index) {
        if (!queues_initialized) {
                return OK;
        }

-       //      print_line("swapbuffers?");
+       if (index != frame_index) print_line("swapbuffers index deviation: ", index, "!=", frame_index);
+       else print_line("context frame: ", frame_index);
        VkResult err;

 #if 0
diff --git a/drivers/vulkan/vulkan_context.h b/drivers/vulkan/vulkan_context.h
index 2ccfd13739..0fd92b2a31 100644
--- a/drivers/vulkan/vulkan_context.h
+++ b/drivers/vulkan/vulkan_context.h
@@ -100,7 +100,8 @@ private:
        enum {
                MAX_EXTENSIONS = 128,
                MAX_LAYERS = 64,
-               FRAME_LAG = 2
+               FRAME_LAG = 3 // swapchain is usually 3 images (on affected system of bug exploration)
+               //we size the swap chain CORRECTLY and we also size the fence/semaphore count correctly (?)
        };

        static VulkanHooks *vulkan_hooks;
@@ -328,7 +329,7 @@ public:
        void resize_notify();
        void flush(bool p_flush_setup = false, bool p_flush_pending = false);
        Error prepare_buffers();
-       Error swap_buffers();
+       Error swap_buffers(int index);
        Error initialize();

        void command_begin_label(VkCommandBuffer p_command_buffer, String p_label_name, const Color p_color);

Note: This is likely just something that shifts the timings of the race condition favorably. I haven't found the place where the frame ordering is actually jumbled up yet.

Nukiloco commented 8 months ago

Oftentimes JUST the two log statements alleviate the issues (but somehow not always, or not precisely).

I'm not an engine developer or a person that messes around with programming languages, but I can assume this means something funky is having with any threads, locks, or some data race issue is happening. However you said that you tested this out with single-threaded rendering, so maybe this isn't an issue with threads but some locking mechanism, or a previous frame is being shown back to the front?

~EDIT: However the stuttering issue i was having also happened on the other renderers as well from what I tested.~ EDIT: Never mind I have just realized my whole PC has been having this issue across all applications now and somehow I haven't even noticed this.

thygrrr commented 8 months ago

It may be a race condition, also just against something that happens in the driver or another process, but perhaps it's really just a indexing bug.

I think at this point that possibly the engine just literally jumbles up its frames:

Quick summary:

4 command buffers, 3 swapchain images, 2 semaphores, and a partridge in a pear tree.

When it acquires its THIRD current_buffer index, it actually uses its own FIRST "frame_index" indexer to determine which acquisition semaphore this is for, and the SECOND index to submit the command buffers.

err = fpAcquireNextImageKHR(device, w->swapchain, UINT64_MAX,
w->image_acquired_semaphores[frame_index], VK_NULL_HANDLE, &w->current_buffer);             

I think I'm not alone saying that sounds, at the very least, confusing.❓

Vulkan's own examples never appear to do it this way (but examples are simplistic compared to the realities of an engine)

Thoughts

The last one, current_buffer, is what's received from vkAcquireNextImageKHR, and I think THAT should be the one and only authoritative "frame index".

This is because it is at the discretion of the implementation what order vkAcquireNextImageKHR actually returns its images.

Vulkan Spec: The presentation engine controls the order in which presentable images are acquired for use by the application. This allows the platform to handle situations which require out-of-order return of images after presentation. At the same time, it allows the application to generate command buffers referencing all of the images in the swapchain at initialization time, rather than in its main loop.

(and there's also an issue that it's just initialized to 0 for the first frame and seemingly never acquired; I think it should also be formally acquired from the chain. in case anyone has had any first rendered frame issues on some hardware...)

I will try to just use a single shared index somehow as a test.

Calinou commented 8 months ago

Thanks for carrying out this testing! I recommend repeating the same tests with https://github.com/godotengine/godot/pull/80566. There are known issues with how the swapchain is managed in master, and that PR fixes this issue.