blender/blender

Fix #127590: EEVEE: Wrong background motion vectors #127718

Merged

Miguel Pozo merged 3 commits from pragma37/blender:pull-fix-background-motion-vectors into main

2024-09-17 19:33:54 +02:00

Miguel Pozo commented

2024-09-16 19:04:45 +02:00

Member

Compute the background plane position in world space and reproject it to the camera perspective matrices.
Remove outdated comments.

Note: The code uses points instead of directions. I've tested by offsetting the test scene 1000 units in every axis and it doesn't seem to cause troubles.
The code could be updated to use view directions projected into the far plane (since that's what I think the original intent was?), but I don't think the perspective matrix can be skipped.

Compute the background plane position in world space and reproject it to the camera perspective matrices. Remove outdated comments. Note: The code uses points instead of directions. I've tested by offsetting the test scene 1000 units in every axis and it doesn't seem to cause troubles. The code could be updated to use view directions projected into the far plane (since that's what I think the original intent was?), but I don't think the perspective matrix can be skipped.

background-motion-vector.blend

2.8 MiB

Miguel Pozo added the

Viewport & EEVEE

labels 2024-09-16 19:04:45 +02:00

Miguel Pozo added 1 commit 2024-09-16 19:04:57 +02:00

Use world space points for velocity_background c29e2a2478

Miguel Pozo requested review from Clément Foucault 2024-09-16 19:06:06 +02:00

Miguel Pozo added 1 commit 2024-09-17 14:03:28 +02:00

Skip camera world offset 8f0206b0f1

Miguel Pozo commented

2024-09-17 14:06:21 +02:00

Author

Member

I've updated the code to avoid the world space translation, but it requires extra matrix multiplications, so I'm not sure it's worth it.
It can be reverted.

I've updated the code to avoid the world space translation, but it requires extra matrix multiplications, so I'm not sure it's worth it. It can be reverted.

Clément Foucault approved these changes 2024-09-17 18:45:26 +02:00

Clément Foucault left a comment

Member

I think I may have think that winmat was persmat.

This new iteration of the patch is the correct fix.
We should not transform the background as points as it would interact during translation of the camera (where the background should have no velocity).

I think I may have think that `winmat` was `persmat`. This new iteration of the patch is the correct fix. We should not transform the background as points as it would interact during translation of the camera (where the background should have no velocity).

source/blender/draw/engines/eevee_next/shaders/eevee_velocity_lib.glsl Outdated

						
				@ -53,4 +52,1 @@

				{

				  /* Only transform direction to avoid losing precision. */

				  vec3 V = transform_direction(camera_curr.viewinv, vV);

				  /* NOTE: We don't use the drw_view.winmat to avoid adding the TAA jitter to the velocity. */

Clément Foucault commented

2024-09-17 18:42:59 +02:00

Member

I don't see why this is outdated? It is an explanation to why we use camera_curr.winmat instead of drw_view.winmat.

I don't see why this is outdated? It is an explanation to why we use `camera_curr.winmat` instead of `drw_view.winmat`.

Miguel Pozo commented

2024-09-17 19:21:12 +02:00

Author

Member

I misread what the comment meant. I've added it back with a small clarification.

I misread what the comment meant. I've added it back with a small clarification.

source/blender/draw/engines/eevee_next/shaders/eevee_velocity_lib.glsl Outdated

						
				@ -46,3 +45,3 @@

				/**

				 * Given a view space view vector \a vV, compute the previous and next motion vectors for

				 * Given a background view space point \a vP, compute the previous and next motion vectors for

Clément Foucault commented

2024-09-17 18:38:21 +02:00

Member

I am not sure this does any difference. In fact, I prefer keeping the transform on the normalized version vV. It makes more sense to transform vV as a direction than vP.

Also keeps the patch smaller for back-porting.

I am not sure this does any difference. In fact, I prefer keeping the transform on the normalized version `vV`. It makes more sense to transform `vV` as a direction than `vP`. Also keeps the patch smaller for back-porting.

Miguel Pozo commented

2024-09-17 19:23:23 +02:00

Author

Member

It skips one normalization, but other than that it's the same. I've reverted the change.

It skips one normalization, but other than that it's the same. I've reverted the change.

pragma37 marked this conversation as resolved

source/blender/draw/engines/eevee_next/shaders/eevee_velocity_lib.glsl Outdated

						
				@ -57,3 +53,1 @@

				  vec2 prev_uv = project_point(camera_prev.winmat, V).xy;

				  vec2 curr_uv = project_point(camera_curr.winmat, V).xy;

				  vec2 next_uv = project_point(camera_next.winmat, V).xy;

				  /** NOTE: Transform as a direction to avoid imprecision if camera is far from origin. */

Clément Foucault commented

2024-09-17 18:42:10 +02:00

Member

Nitpick: Reminder that /** is only for multiline comments.

Nitpick: Reminder that `/**` is only for multiline comments.

pragma37 marked this conversation as resolved

Clément Foucault commented

2024-09-17 18:46:23 +02:00

Member

BTW it is better for precision to use 2 matrix transform instead of persmat. I think this is fine as is.

BTW it is better for precision to use 2 matrix transform instead of `persmat`. I think this is fine as is.

Miguel Pozo added 1 commit 2024-09-17 19:17:22 +02:00

Feedback 147a1a8cd2

Miguel Pozo commented

2024-09-17 19:26:53 +02:00

Author

Member

I think I may have think that winmat was persmat.

I get confused about this all the time as well. I always have to check what each one means.
I wish we had something like view_mat, projection_mat and view_projection_mat or something along those lines.

> I think I may have think that `winmat` was `persmat`. I get confused about this all the time as well. I always have to check what each one means. I wish we had something like `view_mat`, `projection_mat` and `view_projection_mat` or something along those lines.

Miguel Pozo merged commit 9c54ddff69 into main

2024-09-17 19:33:54 +02:00

Miguel Pozo deleted branch pull-fix-background-motion-vectors

2024-09-17 19:33:57 +02:00

Miguel Pozo referenced this issue from a commit

2024-09-17 19:33:58 +02:00

Fix #127590: EEVEE: Wrong background motion vectors

Clément Foucault commented

2024-09-17 20:01:31 +02:00

Member

I wish we had something like view_mat, projection_mat and view_projection_mat or something along those lines.

I think we can make a todo about this mass renaming.

> I wish we had something like view_mat, projection_mat and view_projection_mat or something along those lines. I think we can make a todo about this mass renaming.

Miguel Pozo referenced this pull request

2024-09-18 15:49:26 +02:00

Rename camera matrix terminology #127806

Miguel Pozo commented

2024-09-18 15:50:43 +02:00

Author

Member

Done! #127806

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Viewport & EEVEE

Virtual Reality

Asset Browser Project

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Viewport & EEVEE

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#127718

No description provided.