Already feature complete and workable, but API might be subject to change. We may want the API to get reviewed first before marking this PR as ready.

All the third-party content needs to be correctly attributed, this currently violates their license

Edit: especially seeing how it's so directly copied you even copied typos and TODOs from the original 🙃

All the third-party content needs to be correctly attributed, this currently violates their license

I'll do it.

Amazing work. I finally got to have a look, and I have a few changes to request:

1. motion_blur_jitter_tiles should not be an option, that is my bad. It should always be true. Without it the seams between tiles are obvious.
2. motoin_blur_velocity_depth_test is also a redundant setting, and should be true by default. Again, my bad.
3. target_framerate, framerate_independent, and uncapped_independence should be reintroduced, as they are key features that users thould be able to play with.
4. All of the motion separatiion setting, that is all of these

should be exposed. Their purpose is not clear, and I take the blame for not communicating them clearly enough. They are actually a key feature that make our motion blur effect as powerful as it is. They allow for movement separation between the camera's movement, rotation, and object's movement, allowing users to configure the contribution of each to the final blur separately!

The thresholds are also a cool feature that expand upon the separation, allowing you to configure a velocity threshold for the "activation" of blur for each of these components. This can yield granular control by the user to keep the game view crisp and blur free unless some extreme motions occur.
Something like this:

Since this can be a bit cumbersome to have 6 properties, and yield diminishing returns in terms of granularity, we can consider just having one set of lower and upper threshold parameters that would apply to all 3 components at the same time.

Edit: We can maybe come up with clearer way to expose the framerate related settings. It is not clear what these do and would be hard to communicate through doc comments. Also there are a lot of redundant configurations with them, so I imagine some enum to represent common "framerate modes"

Edit 2: For the framerate modes I am thinking:
NATIVE - the blur is determined purely based on the framerate of the game (final_intensity = intensity)
MIN_FRAMERATE - the blur is determined based on the framerate of the game, but below a minimum framerate limit the amount of blur to that framerate (so it is not abnoxious) (final_intensity = intensity * min(1 / minimum_framerate, time_step) / time_step)
FORCE_FRAMERATE - the blur is determined based on a fixed framerate defined by the user, so even at higher framerate it will maintain the same amount of blur (overblurring, but more consistent). (final_intensity = intensity / (target_framerate * time_step))

For the latter 2 modes, a framerate property should be dynamically exposed.

I apologize for the bother, I should have been cleaner and clearer about the setup.

@HydrogenC Please update the PR's description with the following:

(EDIT: THE CHANGES REQUESTED HAVE BEEN MERGED BY THE OP)

All the third-party content needs to be correctly attributed, this currently violates their license

Edit: especially seeing how it's so directly copied you even copied typos and TODOs from the original 🙃

Not really. I made plenty of modifications actually. The TODOs are there because they are not yet resolved so I keep them to hint future contributors. For the TODOs that I have resolved, I already removed them. This doesn't mean that this PR is underworked.
Btw the original author of the community addon @sphynx-owner is also helping us with this PR. Thanks for help. We'll be adding copyright headers for the GLSL files included right over.

This doesn't mean that this PR is underworked.

Nor did I say so!

Only works for PracticalCameraAttr? Also I notice that then v-sync is disabled I can barely notice the blur

Only works for PracticalCameraAttr? Also I notice that then v-sync is disabled I can barely notice the blur

We can move it elsewhere if required, like to environment. Now it's under practical camera attributes.

Only works for PracticalCameraAttr? Also I notice that then v-sync is disabled I can barely notice the blur

We can move it elsewhere if required, like to environment. Now it's under practical camera attributes.

Yeah I mean, I dont mind that is there, but its not present in PhysicalCameraAttr

When rendering driver is d3d12 this happens:
https://github.com/user-attachments/assets/d39eaf15-a490-4aea-be23-e23d49daa6bf

Properties do not appear until you reopen camera_attributes resource:
https://github.com/user-attachments/assets/da753048-45f6-45e3-b705-617775e0ee3d

This one is hard to see but when you run the project with motion blur enabled a little flickering can be seen:
https://github.com/user-attachments/assets/4757bb7e-e30b-47d4-9d7d-d354c5fd97ac

Only camera rotation gives effect:
https://github.com/user-attachments/assets/0cca69eb-285f-4579-96f8-d18538f4a767

I think some properties should be changed in rendering/camera instead of in the camera_attributes:

Wow this is a lot. The second issue can be fixed with calling notify_property_list_changed most likely. The last issue is not even just camera rotation, the entire blur is broken completely WTH.

Wow this is a lot. The second issue can be fixed with calling notify_property_list_changed most likely. The last issue is not even just camera rotation, the entire blur is broken completely WTH.

Intensity is set to 100.0 in that clip if you are talking about that

Only works for PracticalCameraAttr? Also I notice that then v-sync is disabled I can barely notice the blur

I think it is fine in PracticalCameraAttr, as the base class (CameraAttr) has only stuff regarding exposure & auto-exposure & PhysicalCameraAttr also has the same, only with frustum added. So, if required, we could change it.

When rendering driver is d3d12 this happens: https://github.com/user-attachments/assets/d39eaf15-a490-4aea-be23-e23d49daa6bf

@AThousandShips I have a small question. I've done some debugging on this problem and I found removing specification constants fixes the problem. Is specification constants unsupported in the D3D12 backend?

@AThousandShips I have a small question. I've done some debugging on this problem and I found removing specification constants fixes the problem. Is specification constants not supported in the D3D12 backend?

You may want to also ask @clayjohn

Properties do not appear until you reopen camera_attributes resource: https://github.com/user-attachments/assets/da753048-45f6-45e3-b705-617775e0ee3d

Is fixed with notify_property_list_changed()

I unfortunately can't help with rendering questions clay would be a better person to respond, and I'm sure he will when he has the time

@HydrogenC I have a fix for the 4th issue:

Until we figure out the framerate modes, this can be removed.

In addition, I set up a little side by side of the different blurs because something did not look good, and I found that you did not correctly remove the velocity_depth_test variable in the blur shader, causing overblurring over geometry:

sphynx-owner, I'm very happy to see your involvement in this PR, even if just from a testing and review perspective. This sort of testing and insight from an expert on the subject is invaluable to the development of these features! Thank you!

Every setting in rendering has this (Fastest/Faster/Fast/Average/Slow/Slower/Slowest/Custom) except rendering/camera/motion_blur/motion_blur_tile_size I feel like it breaks convention

Edit: motion_blur_tile_size does not affect performance that much

AMD Radeon RX 5700 XT - i5-11400F - 16 GB memory

Scene:
Preview Sun and Preview Environment
MeshInstance set to BoxMesh 5 meters away from camera
Camera that constantly rotates

Tile Size  20, Samples  4: 592 FPS
Tile Size 200, Samples  4: 577 FPS

Tile Size  20, Samples  8: 485 FPS
Tile Size 200, Samples  8: 455 FPS

Tile Size  20, Samples 16: 323 FPS
Tile Size 200, Samples 16: 292 FPS

Impact of increasing tile size by 10x:
4  samples, FPS drops by 15 (+0.044 ms)
8  samples, FPS drops by 30 (+0.136 ms)
16 samples, FPS drops by 31 (+0.329 ms)

Make sure to use git rebase to update the branch and to push with git push --force and this won't happen

I was using the Github App GUI to do the rebase, and it left a mess if the network fails in the middle. 😞

Docs not yet written, since the API itself may be subject to change during review.

When looking at a PR for the first time, I like to review the newly exposed public facing API by simply looking at the docs files to see what was added. Would it be possible to run doctool to generate the docs? There’s no need to fill in these docs with any final text, but just the empty stubs would make it much easier to, at a glance, see what this new feature adds to Godot’s API. By leaving the generated docs empty or similar, it will be clear to reviewers that this part simply hasn’t been finished yet.

When looking at a PR for the first time, I like to review the newly exposed public facing API by simply looking at the docs files to see what was added. Would it be possible to run doctool to generate the docs? There’s no need to fill in these docs with any final text, but just the empty stubs would make it much easier to, at a glance, see what this new feature adds to Godot’s API. By leaving the generated docs empty or similar, it will be clear to reviewers that this part simply hasn’t been finished yet.

@AR-DEV-1 Will you take this when you have time?

@AR-DEV-1 the latest commit seems broken. Roque curly brackets.

It feels like this excellent plugin is finally nearing completion, and the revival of HDDAGI is just around the corner.

Ye though a bit off topic , but keep expectation low for hddagi , nothing can be sure at all until there’s a new pr or some progress being shown.

One of the main reasons for a feature PR to be merged is that the feature cannot be implemented as a plugin or there are substantial compromises to implementing it as a plugin.

In the description, I see a mention that performance should be better with this PR compared to the plugin, but it would be good to see some real world and benchmark performance comparisons between this PR and the plugin to demonstrate how substantial the performance gains are. This will help validate that this PR is worth the long term maintenance cost.

It would also be good to have this PR's description list and detail any other improvements or benefits to having this feature added to Godot compared to what was possible with the plugin. These will, again, help demonstrate why the long term maintenance of the feature is truly required and cannot be handled by a plugin.

Let's stay on topic

File formatting & etc, failing. Will format them

Oh my, only noticed this PR today, you're the man @HydrogenC! Unfortunately busy with my day job but as soon as I'm able, I'm gonna get around to testing this PR thoroughly! ❤️

Regarding motion blur in the editor, IIRC in Unreal motion blur is rendered in editor, I think their reasoning is that it can affect how things like animations and camera pans look for example and you'd ideally wanna see how they look without having to run your game. However, in Unity, it's not rendered in editor. Ideally of course a project option that allows you to chose would be best.

Regarding the full suite of options @sphynx-owner has implemented in the plugin, I think for an initial motion blur implementation into core, just the core, fundamental effect should be the priority. Once that's done, we can then implement things like framerate independent motion blur, camera/object separated motion blur, ect, as additional PRs afterwards. I think this is also more in line with how features/enhancements are generally tackled with Godot development?

Regarding the full suite of options @sphynx-owner has implemented in the plugin, I think for an initial motion blur implementation into core, just the core, fundamental effect should be the priority. Once that's done, we can then implement things like framerate independent motion blur, camera/object separated motion blur, ect, as additional PRs afterwards.

Yeah, the current status is that all of these additional and advanced stuff are actually already implemented in this PR. Now the problem is that the number of parameters is above the number that keeps the effect simple so we have to scale them down (which means that the hardcoded parameters will possibility never be added back as configurable ones).

I personally believe that the current implementation is complete and coherent. There is still some discussion around the tile size property that needs resolving, but given the documentation I added we should be on the same page on what the different proeprties mean at least.

I think for an initial motion blur implementation into core, just the core, fundamental effect should be the priority.

Once that's done, we can then implement things like [...] camera/object separated motion blur

I think these are key features that shouldn't be stripped out of this PR. I hope the maintainers won't decide to remove them for the "initial" implementation.

What parameters will be configurable by users can and should be discussed and stripped down if it makes sense, but since motion blur is mostly an artistic tool, I'd argue, it should be treated as such (instead of only a tool to smoothen things out for 30fps games) since not all games will need the same look.

I forgot to mention: thanks for providing the generated docs for this PR! That makes it much easier for everyone to start review of the PR. I have been overwhelmed working on HDR ouptut, so I'm not sure if I'll be the person to review this PR, but the public-facing API is critically important to review, as this cannot easily change, if at all, between Godot versions.

I noticed that my previous comment received a lot of confused emojis, so I understand that there might be some confusion about what I wrote. I'll provide some clarification:

I wrote that note from the perspective that is generally taken with all PRs in Godot. This process helps us ensure a high quality, bug-free, maintainable game engine in the long run and is why we ask "Is there a reason why this should be core and not an add-on in the asset library?" in the proposal template. I noticed that the proposal for this PR has notes about how this PR should be better than the existing plugin.

All that said, I was not aware that a decision may have been made to include a motion blur feature in the Godot engine without the need for a plugin. So assuming that this decision has been made, it is still important to verify the performance characteristics of this PR and any differences/improvements/etc. that have been made compared to the plugin. This not only gives validation that integrating the feature into the core engine is valuable, but also ensures that the quality and characteristics of this PR are as-expected.

So, regardless of the exact reasons, seeing some performance comparisons and details regarding any other improvements or benefits compared to the plugin this PR is based off of will still be extremely valuable in the review of this PR.

If others who are interested in this PR are able, it would be good to test this PR with your own project to see how it performs, etc. Once a bug, problem, or limitation has been merged into Godot it becomes much harder to fix later on, so lots of testing of PRs is always appreciated!

All that said, I was not aware that a decision may have been made to include a motion blur feature in the Godot engine without the need for a plugin.

At the godot priorities, they mention the motion blur pr and that they wanted to add common effects as built in . See https://godotengine.org/priorities/#significantly-improve-post-processing-effects

All that said, I was not aware that a decision may have been made to include a motion blur feature in the Godot engine without the need for a plugin.

In our priorities, it is clearly mentioned the need for built-in common effects & the motion blur PR is also mentioned there.
https://godotengine.org/priorities/#significantly-improve-post-processing-effects

(Edit: Saul said the same thing as me, I just found about it now)