Add motion blur compositor effect. #92711

fire · 2024-06-03T01:57:01Z

An implementation of godotengine/godot-proposals#2933.

See the proposal for alternatives.

RedMser

Not sure if you're looking for review, but I looked over everything except the implementation details of the algorithm and shader.

scene/resources/motion_blur_compositor_effect.cpp

RedMser · 2024-06-03T05:47:11Z

doc/classes/CompositorEffect.xml

 			If [code]true[/code] this rendering effect is applied to any viewport it is added to.
 		</member>
-		<member name="needs_motion_vectors" type="bool" setter="set_needs_motion_vectors" getter="get_needs_motion_vectors">
+		<member name="needs_motion_vectors" type="bool" setter="set_needs_motion_vectors" getter="get_needs_motion_vectors" default="true">


The changes to this file seem wrong. Since doctool didn't fail the pipeline, it seems to be a problem of how the fields are initialized (maybe your code isn't at fault but simply reveals the issue?).

If a class has not been used doctool can't find the default values.

RedMser · 2024-06-03T05:48:23Z

scene/resources/motion_blur_compositor_effect.cpp

+MotionBlurCompositorEffect::MotionBlurCompositorEffect() {
+	set_effect_callback_type(EFFECT_CALLBACK_TYPE_POST_TRANSPARENT);
+	set_needs_motion_vectors(true);
+	if (!DisplayServer::get_singleton()->window_can_draw()) {


This check is already done inside the initialize compute method, no need for it in both places.

This is a bug in initialize compute, so until that is fixed I cannot remove it. It fails the mono build.

Some sort of recursive mutex deadlock.

scene/resources/motion_blur_compositor_effect.cpp

scene/resources/motion_blur_compositor_effect.h

scene/resources/motion_blur_compositor_effect.cpp

scene/resources/motion_blur_compositor_effect.h

scene/resources/motion_blur_compositor_effect.cpp

fire · 2024-06-03T12:06:22Z

I had a conversation with shakesoda around the potential improvements in the World Environment effects of a game engine.

We discussed the possibility of moving most, if not all, of the currently built-in effects in the World Environment to the Compositor Effects API. We believe that this could be done with some effort, and it would be a strategy @shakesoda would implement in his own engine.

We think this is the intended direction of the Compositor Effects API because the hard-coded effects are detrimental to performance and flexibility. We appreciate the ability to swap in effects as needed, which enhances customization.

However, there's a debate on whether if Motion Blur should be an in-engine PR. While it's a good example of the Compositor Effects API and has been well-received, it's also something that can be achieved in user land now. Despite this, it's straightforward, often requested, and practical for games as it stands so I made a pr.

jcostello · 2024-06-03T12:57:06Z

However, there's a debate on whether if Motion Blur should be an in-engine PR.

What are the oppositions to add motion blur to the engine?

fire · 2024-06-03T13:17:24Z

There's not been any opposition. I was following through with the logical conclusion of how hard-coded effects are detrimental to performance and flexibility of the rendering when they can be extracted like this.

The contributor guidelines has paragraphs to say about this so I think the pr is fine.

QbieShay · 2024-06-03T13:44:31Z

Having it built-in for the engine makes it more complicated for people to possibly extract and modify it. Supplying ti as a plugin would make it easier for people to just take it and change it. I don't think it's a strong opposition, and I don't think that one is exclusive with the other.

Co-Authored-By: Colby Klein <shakesoda@gmail.com>

fire · 2024-06-03T16:20:05Z

scene/resources/motion_blur_compositor_effect.cpp

+	if (!DisplayServer::get_singleton()->window_can_draw()) {
+		return;
+	}
+	_initialize_compute();


Suggested change

_initialize_compute();

RenderingServer::get_singleton()->call_on_render_thread(callable_mp(this, &MotionBlurCompositorEffect::_initialize_compute));

Was previously this.

Sal-GC · 2024-06-03T22:01:13Z

Hi there everyone. As @passivestar mentioned above, I can attest, as an end-user of the engine, that the need of motion blur is very real. Very sorely missed, to come as an in-engine feature. I have been dabbling with Godot for quite some time(used Cocos Creator, Flax, Defold and UE as well), and this is one of my most expected features.

I am not as technically versed as you guys here in this discussion, but I can surely tell you motion blur as part of the World Environment node is what most users would expect and make use of more readily. I see the points you make of making it go in the compositor instead, makes sense very much as well. However. as mentioned above, most users would expect it as an in-engine graphics feature and this would certainly push Godot's 3D graphics to a more modern look, that and a more robust TAA perhaps.

Those are my two cents, and thank you all very much for all the efforts you make building this engine and making it great. It is very much appreciated despite perhaps it is not expressed enough. :) Hopefully, in the future I will get to contribute to it as well. Thanks!

Calinou

Tested locally, this is very encouraging 🙂

I noticed an issue (on top of what has been said above): it looks broken when FSR2 is used at any 3D scale factor at or below 1.0 (since values above 1.0 disable FSR2).

Other than that, quality is quite good in best-case scenarios (example with 16 samples):

60 FPS video made with Movie Maker mode on https://github.com/Calinou/godot-reflection (--write-movie output.avi -- --benchmark CLI arguments):

motion_blur.mp4

Motion blur has a rendering issue in the first frames of the project (after startup). To resolve this, motion blur intensity should probably be set to 0 initially and ramp up over the number of frames defined by the sample count.

The motion blur falls apart when there are transparent surfaces close to the camera (such as particles or glass), which is an unfortunate consequence of motion vectors not being written by transparent surfaces. I don't know a good solution to this, but we should see if it's possible to skip motion blur entirely on those pixels. This should only be done in certain scenarios though, not on all transparent materials (e.g. you wouldn't want a car's windshield to ignore motion blur when viewed from outside). I wonder if #77523 would help here.

The motion blur intensity is framerate-dependent in this PR, which I think is a reasonable default for the reasons outlined in the proposal's comments. The default intensity seems a bit low for 60 FPS – I bumped it to 1.0 personally (e.g. in the above video), but I would make it 0.5 at the very least. I'm using a 120 Hz OLED display, so your perception of motion blur may differ from mine (particularly if you're using a VA display, due to their slow response times).

Motion blur doesn't appear to be resolution-dependent, which is good.

Also, I think this should be implemented as a core camera effect as opposed to a compositor effect. The effect is hardcoded either way, so there isn't much benefit to having it as a built-in compositor effect. If we intend to migrate core effects to compositor effects, we should do that in separate PRs and do this for all effects, not just one.

shakesoda · 2024-06-04T01:55:57Z

The default intensity seems a bit low for 60 FPS – I bumped it to 1.0 personally (e.g. in the above video), but I would make it 0.5 at the very least. I'm using a 120 Hz OLED display, so your perception of motion blur may differ from mine (particularly if you're using a VA display, due to their slow response times).

0.25-0.5 is what i was using for 30-60fps. up to 1.0 should be OK but beyond that's bad, it's dialed down as far as i found helpful for lower framerates without being overly visible, i'm not using it for stylistic reasons but just for smoothing. i'm using an OLED display as well, although my gpu can't run it at 120hz.

realcoloride · 2024-06-04T02:01:16Z

Oh my god its finally happening

Props to everyone working on this PR 🎉

DigitalN8m4r3 · 2024-06-04T21:45:30Z

I had a conversation with shakesoda around the potential improvements in the World Environment effects of a game engine.

We discussed the possibility of moving most, if not all, of the currently built-in effects in the World Environment to the Compositor Effects API. We believe that this could be done with some effort, and it would be a strategy @shakesoda would implement in his own engine.

We think this is the intended direction of the Compositor Effects API because the hard-coded effects are detrimental to performance and flexibility. We appreciate the ability to swap in effects as needed, which enhances customization.

2nd this, the idea of moving evrything out of the world env would free it up

passivestar · 2024-06-08T11:08:29Z

btw I noticed that particles don't contribute to motion vectors, would be nice to see that fixed in the future too

Calinou · 2024-06-08T21:06:04Z

btw I noticed that particles don't contribute to motion vectors, would be nice to see that fixed in the future too

Opaque particles (both CPU and GPU) should already contribute motion vectors, but transparent materials don't contribute to motion vectors in general. This is unfortunately a difficult problem to solve.

passivestar · 2024-06-10T05:42:58Z

Opaque particles (both CPU and GPU) should already contribute motion vectors

Doesn't look like it, unless you mean it was added very recently after beta1

Calinou · 2024-06-10T17:06:14Z

Doesn't look like it, unless you mean it was added very recently after beta1

In 4.2.2.stable and 4.3.beta 5833f59, I get motion vectors with both TAA and FSR2:

Testing project: test_motion_vectors.zip

passivestar · 2024-06-11T06:24:20Z

In 4.2.2.stable and 4.3.beta 5833f59, I get motion vectors with both TAA and FSR2

oh, me too. but it has to be a bug though, right? you shouldn't need TAA or FSR to be enabled to be able to use motion vectors for particles in your shaders? everything else contributes to motion vectors with those disbled

QbieShay · 2024-06-11T16:13:28Z

We discussed motion blur in the rendering meeting, and there's been a couple of takeaways from this discussion and the discussion in the issue.

While we can see the obvious support and interest for this effect, right now there's discrepancies on people's needs and wants for how motion blur should look like, and the current implementation seems to have issues that still need to be addressed.

What we'd like to see, is the following:

Move this compositor effect to use user-space API (GDScript), and have a faster iteration process. Have multiple artists play with it.
Once that reaches maturity, it will probably need to be transformed into the worldenvironment pipeline and brought into the engine "core". We want to keep the experience consistent with the rest (plus some performance considerations with world environment vs compositor).
If there's any feature missing in userland for this to work, we can address it separately.

The rationale: there is no one way to do art, and while there's established technologies out there, people often resort to their own custom code. We don't want to provide an effect that's too slow for production, or too ugly but fast enough. If we take on the burden of maintaining something, there needs to be at least some form of certainty on the actual benefit of the feature, and not that we end up with a lukewarm solution that benefits nobody. The worst case for us is to rush an implementation of this, and then having to maintain default parameters and legacy behaviour that then looks bad. There's already a good number of places in Godot where defaults are a bit meh.

The compositor existing now gives new opportunities to have users pilot effects and iterate separately from a complex cpp codebase and clunky testing process. We want to see more people test the effects and more iteration to ensure that it's something that would effectively be viable in production, both aestethics-wise, and performance-wise. We also want to be sure that the solution is not too narrow for a specific aestethic.

So the suggested path forward (looking for volunteers <3 )

~~Convert this PR into gdscript+glsl~~
Form a workgroup with testers, with a chamption to be their producer.
Iterate until success conditions are met
At that point, we can look into moving it into engine and into the worldenvironment directly.

I'm happy to help for organizing this workgroup, we can coordinate on RC, however I will not take on the actual conversion task, for that I'm hoping that one of you either volunteers, or finds a friend who will volunteer.
https://chat.godotengine.org/channel/vfx-tech-art

shakesoda · 2024-06-11T18:34:51Z

my original gdscript implementation of the effect is posted in the proposal: godotengine/godot-proposals#2933 (comment)

QbieShay · 2024-06-11T18:38:15Z

Thank you, properly catching up now. That's great and removes one step of the process already ^^

clayjohn · 2024-06-11T19:25:16Z

In 4.2.2.stable and 4.3.beta 5833f59, I get motion vectors with both TAA and FSR2

oh, me too. but it has to be a bug though, right? you shouldn't need TAA or FSR to be enabled to be able to use motion vectors for particles in your shaders? everything else contributes to motion vectors with those disbled

Motion vectors should be present when using the motion vector debug draw mode, so this indeed looks like a bug.

edit: Its an easy fix: PR here #93055

clayjohn · 2024-06-11T20:00:48Z

As Passivestar has highlighted, we need to ensure that motion vectors are enabled by Particles, MultiMesh, and Skeletons when motion blur is enabled. Right now we accomplish that by requesting motion vectors when registering the compositor effect. Unfortunately, the way that is implemented only enables drawing motion vectors at draw time. It doesn't signal to the Particle or Mesh processing stages that the additional buffers for Motion Vectors should be allocated. Which means that Particles, MultiMesh, and Skeletons will be missing motion vectors if neither TAA or FSR is enabled.

realcoloride · 2024-06-11T22:05:59Z

Question, would it also be interesting to be able to add motion blur on UI related nodes?

Calinou · 2024-06-12T01:30:05Z

Question, would it also be interesting to be able to add motion blur on UI related nodes?

If you set the environment background mode to Canvas, compositor effects will also affect 2D. However, 2D elements don't generate motion vectors, so motion blur will not work on them.

What you can do instead is:

Draw sprites several times with lower opacity when they move (each sprite simulates a motion blur sample). Sprites should trail behind each other relative to time to simulate motion blur. This doesn't require modifying textures and works with movement on both axes. Drawing lots of sprites can eventually have a performance cost (especially on mobile), so be careful not to use too many samples.
Draw the same sprite once when it moves, but replace its texture with a manually edited texture that blurs it along one direction. This has virtually no performance cost, but it's a more limited approach.
- Using a custom CanvasItem shader during motion is an alternative approach, although care must be taken at the sprite's edges to avoid visibly reaching the sprite's borders. High sample counts can also be expensive on the GPU.
- This texture replacement is the approach used in the 2001 build of Duke Nukem Forever when the weapon UI (dis)appears:

dnf2001-ui-motion-blur.mp4

Video is at half speed to make the effect more noticeable.

realcoloride · 2024-06-12T01:40:21Z

Thank you for this, and its useful. I wondered if it could be done natively in the motion blur system and the response is excellent, quote:

If you set the environment background mode to Canvas, compositor effects will also affect 2D.

I was mostly inspired by games that have a motion blur whenever you look somewhere, the UI would reflect the motion blur (3D / FPS).

QbieShay · 2024-06-19T12:34:12Z

Due to popular demand, discussion has moved in the VFX server https://discord.gg/HX4xAGaGjm

EDIT: fixed link

BuyMyMojo · 2024-07-16T07:29:31Z

Due to popular demand, discussion has moved in the VFX server https://discord.gg/4JBkykG

Sorry for the erant addition to the issue but where exactly is the discussion there?
I am unable to find a thread relatiung to motion blur and if it's in one of the many channels it will be hard to find and reference later.

I do feel having the discussion around this issue in the discord does somewhat make it moer seperated and ephemeral compared to how this thread will be "archived" upon merge.

If I misinterpreted what part was actually moved to the Discord I appologise and will remove this post to keep the thread clean

QbieShay · 2024-07-16T08:26:00Z

No problem. There is a channel called "wg-motion-blur". I understand that splitting discussion across platforms is not ideal but I want it to be easy for people to communicate and iterate and it seems like discord was the preferred place for it.

BuyMyMojo · 2024-07-16T20:43:52Z

No problem. There is a channel called "wg-motion-blur". I understand that splitting discussion across platforms is not ideal but I want it to be easy for people to communicate and iterate and it seems like discord was the preferred place for it.

Using the search feature bring up no results for that name?

I found one mention of this channel and it was another user just as confused

I'm not going to protest the moving to discord as that is a perfectly valid reason but this does make things a little harder

Calinou · 2024-07-16T21:04:49Z

The #wg-motion-blur channel is on the Godot Effects and Shaders Discord, not on Godot Café.

BuyMyMojo · 2024-07-16T22:15:55Z

The previous invite link and your invite link takes me directly to the cafe

cafe.mp4

Calinou · 2024-07-16T22:19:22Z

Fixed invite link: https://discord.gg/HX4xAGaGjm

fire · 2024-08-21T15:04:04Z

https://github.com/sphynx-owner/JFA_driven_motion_blur_addon has a better implementation now.

I will close the pull request as it did the job of sparking interest in motion blur in Godot Engine.

Feel free to salavage / remake using JFA_driven_motion_blur and this pull request.

fire requested a review from a team as a code owner June 3, 2024 01:57

fire force-pushed the vsk-motion-blur-4.3-beta1 branch from 7ffba3e to b615997 Compare June 3, 2024 02:12

fire added the salvageable label Jun 3, 2024

fire force-pushed the vsk-motion-blur-4.3-beta1 branch 8 times, most recently from e236904 to e633c3f Compare June 3, 2024 04:02

RedMser reviewed Jun 3, 2024

View reviewed changes

AThousandShips reviewed Jun 3, 2024

View reviewed changes

fire force-pushed the vsk-motion-blur-4.3-beta1 branch 3 times, most recently from bf88167 to bcaac1a Compare June 3, 2024 11:35

AThousandShips reviewed Jun 3, 2024

View reviewed changes

scene/resources/motion_blur_compositor_effect.cpp Show resolved Hide resolved

fire force-pushed the vsk-motion-blur-4.3-beta1 branch 2 times, most recently from 5ad0776 to 46c2c99 Compare June 3, 2024 11:44

Add motion blur compositor effect.

1992006

Co-Authored-By: Colby Klein <shakesoda@gmail.com>

fire force-pushed the vsk-motion-blur-4.3-beta1 branch from 46c2c99 to 1992006 Compare June 3, 2024 14:50

fire commented Jun 3, 2024

View reviewed changes

Calinou added enhancement topic:rendering topic:3d labels Jun 3, 2024

Calinou added this to the 4.x milestone Jun 3, 2024

Calinou requested changes Jun 3, 2024

View reviewed changes

clayjohn mentioned this pull request Jun 11, 2024

Ensure Motion Vectors are enabled by particles and skeletons when using the Motion Vector debug draw option #93055

Merged

fire closed this Aug 21, 2024

AThousandShips added the archived label Aug 21, 2024

AThousandShips removed this from the 4.x milestone Aug 21, 2024

	_initialize_compute();
	RenderingServer::get_singleton()->call_on_render_thread(callable_mp(this, &MotionBlurCompositorEffect::_initialize_compute));

Add motion blur compositor effect. #92711

Add motion blur compositor effect. #92711

Conversation

fire commented Jun 3, 2024

RedMser left a comment

Choose a reason for hiding this comment

RedMser Jun 3, 2024

Choose a reason for hiding this comment

fire Jun 3, 2024

Choose a reason for hiding this comment

RedMser Jun 3, 2024

Choose a reason for hiding this comment

fire Jun 3, 2024

Choose a reason for hiding this comment

fire Jun 3, 2024

Choose a reason for hiding this comment

fire commented Jun 3, 2024

jcostello commented Jun 3, 2024

fire commented Jun 3, 2024 • edited Loading

QbieShay commented Jun 3, 2024

fire Jun 3, 2024

Choose a reason for hiding this comment

Sal-GC commented Jun 3, 2024

Calinou left a comment • edited Loading

Choose a reason for hiding this comment

shakesoda commented Jun 4, 2024

realcoloride commented Jun 4, 2024

DigitalN8m4r3 commented Jun 4, 2024

passivestar commented Jun 8, 2024

Calinou commented Jun 8, 2024 • edited Loading

passivestar commented Jun 10, 2024

Calinou commented Jun 10, 2024

passivestar commented Jun 11, 2024

QbieShay commented Jun 11, 2024 • edited Loading

shakesoda commented Jun 11, 2024

QbieShay commented Jun 11, 2024

clayjohn commented Jun 11, 2024 • edited Loading

clayjohn commented Jun 11, 2024

realcoloride commented Jun 11, 2024

Calinou commented Jun 12, 2024 • edited Loading

realcoloride commented Jun 12, 2024

QbieShay commented Jun 19, 2024 • edited Loading

BuyMyMojo commented Jul 16, 2024

QbieShay commented Jul 16, 2024

BuyMyMojo commented Jul 16, 2024

Calinou commented Jul 16, 2024 • edited Loading

BuyMyMojo commented Jul 16, 2024

Calinou commented Jul 16, 2024

fire commented Aug 21, 2024 • edited Loading

fire commented Jun 3, 2024 •

edited

Loading

Calinou left a comment •

edited

Loading

Calinou commented Jun 8, 2024 •

edited

Loading

QbieShay commented Jun 11, 2024 •

edited

Loading

clayjohn commented Jun 11, 2024 •

edited

Loading

Calinou commented Jun 12, 2024 •

edited

Loading

QbieShay commented Jun 19, 2024 •

edited

Loading

Calinou commented Jul 16, 2024 •

edited

Loading

fire commented Aug 21, 2024 •

edited

Loading