Add motion_draw_disabled render_mode to Spatial Shaders.

[EMBYRDEV]

This PR adds a new motion_draw_disabled render_mode that allows materials to skip drawing motion vectors.

This is required for post processing effects to play nicely with TAA. It also can help with the appearance of some translucent materials.

There are currently two methods of doing post processing:

• A quad with a vertex shader positioning it infront of the camera.
  This works well with the editor camera however with TAA enabled the whole scene appears to shake.
• A quad as a child of the players camera.
  This solves the shaking issue however the motion vectors are no longer accurate, leading to horrible artifacts.

With this render_mode enabled post processing effects such as this outline get properly antialiased and remain crisp in motion.

TODO: Expose to BaseShader3D

• Production edit: This closes TAA causes jittering with "Advanced Post-Processing" #89118.

[EMBYRDEV]

@JFonS any other blockers?

[EMBYRDEV]

I have now exposed this to BaseMaterial3D.

[JFonS]

Looks great, thanks.

[YuriSizov]

@EMBYRDEV Could you please rebase against the master instead of merging it into your branch?

[EMBYRDEV]

@EMBYRDEV Could you please rebase against the master instead of merging it into your branch?

Done!

[EMBYRDEV]

Is there anything else needing done before looking at getting this merged?

[Calinou]

Do you have a test project available to make sure this PR works as intended? I'd like to test this locally. This can be a scene with your post-processing shader setup.

[EMBYRDEV]

@Calinou omg, I put it in my recycle bin last week and never got around to emptying it... lucky. 🤣

Here is a google drive link for the scene in the screenshot.

Both the post processing shader and the smoke on the torches have this render mode enabled. Remove the render mode to see the default behaviour of the engine prior to this PR.

[Calinou]

Tested locally (rebased on top of master 851bc64), it works as expected.

Both videos were recorded with the engine FPS capped at 30.

Without motion_draw_disabled on the post-processing shader

simplescreenrecorder-2023-07-17_03.31.59.mp4

With motion_draw_disabled on the post-processing shader

simplescreenrecorder-2023-07-17_03.32.08.mp4

Documentation looks good to me too.

One question that may arise in the future is how we'll want to extend this once FSR 2 support is added. FSR 2 recommends creating a manually computed reactive mask, as well as a transparency & composition mask. This will probably need separate output variables and render modes to handle.

[YuriSizov]

Is there anything else needing done before looking at getting this merged?

Clay needs to give it a review as the rendering lead. I asked and he said that he'd like to take a look.

[EMBYRDEV]

Rebased :)

@clayjohn any chance this can make it in for 4.2?

[clayjohn]

Yep, its tagged for "4.2". I just need to give it a review

[clayjohn]

This looks mostly good. It is a fairly simple change, but it seems necessary for implementing certain effects that should bypass motion vectors.

I suggest that, instead of using a render_mode_define you instead just select the non-Motion_Vector shader variant instead. Right now, this code compiles the following shader variants:

1. #define MOTION_VECTORS \n #define MOTION_DRAW_DISABLED
2. #define MOTION_VECTORS
3. #define MOTION_DRAW_DISABLED
4. ""
   But since MOTION_DRAW_DISABLED only serves to disable MOTION_VECTORS, we can get by without it.

Further, at draw time, we dynamically select our shader variant, based on whatever settings we choose. So instead of adding a new define, you can just avoid asking for the Motion Vector shader variant here:

godot/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp

Lines 384 to 386 in 169a28b

	if constexpr ((p_color_pass_flags & COLOR_PASS_FLAG_MOTION_VECTORS) != 0) {
	pipeline_color_pass_flags |= SceneShaderForwardClustered::PIPELINE_COLOR_PASS_FLAG_MOTION_VECTORS;
	}

Additionally, since the setting is a render_mode, at shader compile time, you know you will never select the TAA version, so you can just avoid compiling it by doing an early out here: https://github.com/godotengine/godot/blob/41dca2cbbeeb96c41aacbc7b0ef5c15c4f8dbfdf/servers/rendering/renderer_rd/forward_clustered/scene_shader_forward_clustered.cpp#L343-L345

I think using this approach both fits better with the current architecture and will be ideal with our plan to split the opaque render pass into two phases (1 with motion vectors and one without)

[akien-mga]

@EMBYRDEV Are you able to do the changes suggested by @clayjohn?

[EMBYRDEV]

@clayjohn is it worth me reworking this PR in the next week or so, or should I wait until #81197 has had some time in the oven?

[clayjohn]

@clayjohn is it worth me reworking this PR in the next week or so, or should I wait until #81197 has had some time in the oven?

I would save your time! I have a gut feeling this PR is still needed. But I don't have a specific use case in mind other than disabling motion vectors for objects in the transparent pass. So better to wait and see and avoid doing work in case it turns out to be unnecessary.

[RPicster]

Just to add some more feedback. I have a lot of problems with motion vectors on transparent materials with TAA.
While using this flag will have the drawback of destroying smaller particles, it is very needed for a lot of effects. TAA often introduces weird artefacts at the edges of transparent particles/quads. Is it planned to also add this flag to the StandardMaterial3D or just just in shaders?

[EMBYRDEV]

Is it planned to also add this flag to the StandardMaterial3D or just just in shaders?

This PR does expose the setting to StandardMaterial3D too. Just need to get some free time to rebase this PR if @clayjohn believes there is still a use case for this?

[clayjohn]

Just to add some more feedback. I have a lot of problems with motion vectors on transparent materials with TAA. While using this flag will have the drawback of destroying smaller particles, it is very needed for a lot of effects. TAA often introduces weird artefacts at the edges of transparent particles/quads. Is it planned to also add this flag to the StandardMaterial3D or just just in shaders?

It depends. As of 4.2, we never write motion vectors when using transparent materials. So if your need is just for disabling TAA on transparent materials, then it will be fixed in 4.2 without this PR.

If you are using one of the 4.2 betas, and are still having issues with transparent materials, then this PR won't help you.

If you need to disable motion vectors for another reason, then we can move forward with this PR. Basically this PR is just waiting until someone finds a need for it.

[PhilipWitte]

I want to add that this issue with Motion Vectors also applies to more than just post-processing effects. I've been working on a terrain system with polygons that follow the Camera as it moves (in unit steps), the motion vectors used for FSR 2 cause a lot of artifacts when the ground shifts position. I've included videos below to illustrate the issue:

No Upscale + FXAA (baseline, no artifacts):
https://github.com/godotengine/godot/assets/1392440/0254a1d2-8130-4322-97b1-43c24e085935

Upscale (0.5 renderscale) + FSR 2
https://github.com/godotengine/godot/assets/1392440/2f7990ac-7ed6-489f-a4be-cedfb364bb48

As you can see, FSR2 looks great while still.. but in motion these artifacts essentially make it unusable with this terrain technique without the ability to disable motion vectors. So I would love to see this render_mode motion_draw_disabled option added in the future, and would even suggest including an additional motion_draw_custom option in the future. That would allow shaders to output their own screen-space motion vectors explicitly, as any "one size fits all" solution will likely have performance/memory trade-offs and may still be unusable in some situations (such as ray-marched clouds, or other visual effects) where doing motion calculations in the shader is basically the only practical solution.

[EMBYRDEV]

I want to add that this issue with Motion Vectors also applies to more than just post-processing effects. I've been working on a terrain system with polygons that follow the Camera as it moves (in unit steps), the motion vectors used for FSR 2 cause a lot of artifacts when the ground shifts position.

Can you try this PR out and confirm that it does indeed help with your issue? I'm not convinced that using this shader flag on a large portion of the screen with an opaque material wont lead to smearing artifacts. Although, with motion vector passthrough, if we ever address #67332 then the sky might help some of those issues.

[PhilipWitte]

Can you try this PR out and confirm that it does indeed help with your issue?

I copied my Godot repo (up-to-date with master) and fetched this PR... however while the new motion_draw_disabled render_mode option is there... FSR 2 itself is not. I'm assuming this PR is from before FSR2 was added into master, but without FSR2 it's hard to test. While TAA does seem to have some subtle artifacts due to motions-vectors... it's not noticeable enough for me to do valid comparison. Certainly no where near the very obvious "fuzzing" FSR2 artifacts.

I tried merging in master.. but there's 5 different conflicts I'm not going to try and resolve myself. If there's an update to this PR with those issues resolved, ping me and I'll try again.

[EMBYRDEV]

@clayjohn I can confirm that this is still required as of 4.2rc1 With FSR2 enabled the problem isnt as pronounced but if you have FSR disabled and TAA enabled then we do still need this for post processing effects.

I'll take some time soon to rebase this against master to aim for 4.3.

[EMBYRDEV]

I have rebased against master and this should be safe to merge now.

As for @clayjohn's suggestion of switching the shader variant instead of using the define, this actually causes a problem as the rest of the pipeline seems to expect the shader to still have the vector in it's output layout.

godot/servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl

Lines 775 to 779 in 69d9bbe

	#ifdef MOTION_VECTORS
	layout(location = 2) out vec2 motion_vector;
	#endif

And since we don't check for the disabled flag here it works nicely. If you have any suggestions on how best to handle this then I'll be happy to implement but this seems like the most streamlined solution.

[DarioSamo]

As you can see, FSR2 looks great while still.. but in motion these artifacts essentially make it unusable with this terrain technique without the ability to disable motion vectors. So I would love to see this render_mode motion_draw_disabled option added in the future, and would even suggest including an additional motion_draw_custom option in the future. That would allow shaders to output their own screen-space motion vectors explicitly, as any "one size fits all" solution will likely have performance/memory trade-offs and may still be unusable in some situations (such as ray-marched clouds, or other visual effects) where doing motion calculations in the shader is basically the only practical solution.

I don't think disabling the motion vectors would result in any different result at all in your use case here: motion vectors are not generated for static geometry in the first place and are derived from depth buffer and the camera movement.

It sounds to me like your second suggestion is the one that applies the best: being able to output motion vectors value from the shader. I've suggested as much but it sounds like it'd have to be added by modifying parts of Godot I haven't really looked into yet (shader compiler, the shader template acknowledging the new shader variable, permutations, etc).

[PhilipWitte]

motion vectors are not generated for static geometry in the first place and are derived from depth buffer and the camera movement.

Yes I figured, but keep in mind the ground is not actually static, but a mesh which moves along with the camera at distinct unit steps. In between those steps things look fine, it's only for a split second after each "step" (where the ground actually shifts position) where the artifacts occur.

That being said, I agree that ultimately just disabling motion vectors might not fully solve everything here, as other parts (like portions of the grass) will do the same thing as the ground (in terms of following the camera), but also sway with the wind, and so dynamically move each frame. Ultimately having the ability to output correct motion vectors may be the only way to eliminate artifacts (tho for the most part it looks fine until the large snap happens).

[TokisanGames]

Terrain3D has issues with TAA/FSR. Like Phil, our terrain is a clipmap. The meshes follow the camera and the vertices move potentially every frame. I'd like a method to disable motion vectors on the terrain while allowing the user to have them for the rest of their scene. Or a method to clear, overwrite or insert an inverted motion vector so these modes are usable.

Downstream issue
TokisanGames/Terrain3D#302

[EMBYRDEV]

So I havent tested the new CompositorEffect API but I imagine that removed my usecase for this PR, being PostProcessing shaders that play nicely with AA.

That being said this may still be useful for some particle types where smearing is desired over rippling effects (EG. Small smoke particles & sparks)

As for Terrain3D I have a feeling that enabling this mode would potentially lead to the terrain itself becomign a smeary mess.

@TokisanGames would you be able to give this PR a test to see if this actually solves your issue or if we need a differing approach?

[TokisanGames]

I built this PR rebased off of the 4.2 branch. With TAA enabled on my 3070, motion vectors are mostly calculated correctly, with flickering only in some areas. This PR does disable motion vectors, but it turns my terrain into a blurry mess.

TAA

godot.windows.editor.x86_64_exIgOdNXk9.mp4

TAA Motion Vectors

godot.windows.editor.x86_64_lZcMtSnwEc.mp4

TAA Motion vectors disabled

godot.windows.editor.x86_64_SnNx7EOyAz.mp4

[EMBYRDEV]

Ahh looks liek this isnt the solution to your problem. I think it is still super important to expose the ability to passthrough motion vectors from behind the object as some large billboard sprites like lens flares can cause big problems with TAA but I think in addition to this we need to expose a way to write custom motion vectors from shader.

I'll get this rebased and implement feedback soon so that this can make it in for 4.4 sorry for the delay.