Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Motion Vectors for Universal MVP #4408

Merged
merged 44 commits into from May 17, 2021

Conversation

he-ax
Copy link
Contributor

@he-ax he-ax commented May 3, 2021

Checklist for PR maker

  • Have you updated the changelog? Each package has a CHANGELOG.md file.
  • Have you updated or added the documentation for your PR? When you add a new feature, change a property name, or change the behavior of a feature, it's best practice to include related documentation changes in the same PR. If you do add documentation, make sure to add the relevant Graphics Docs team member as a reviewer of the PR. If you are not sure which person to add, see the Docs team contacts sheet.
  • Have you added a graphic test for your PR (if needed)? When you add a new feature, or discover a bug that tests don't cover, please add a graphic test.

Purpose of this PR

Adding Motion Vector pass to Universal Render Pipeline. This is needed for URP to support features like Motion Blur and TAA. This is a MVP and has some restrictions.
Does not support custom shaders that modify vertex positions.
Only supports R16G16_SFloat for render target.
Only supports per object motion vectors for objects that are moving.
For 3.0 target and above.
Disabled for xr.


Testing status

Motion Vector pass is disabled by default and need to be enabled thorough script. Test scene contains a script that does this and then renders Motion Vectors on top of the regular scene.

Added test scene in _Foundation project: 206_Motion_Vectors
image
Test displays motion vectors target rendered to the game view. It displays both per object and camera motion vectors, but since the camera is not moving it only shows per object motion vectors right now.

Link to yamato for c++ branch testing against Graphics master:
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/master/.yamato%252F_abv.yml%2523all_project_ci_CUSTOM-REVISION/6733935/job/full-pipeline
Testing this PR against same trunk revision.


Comments to reviewers

Technical Design Document: https://docs.google.com/document/d/1P3HstSS7MFpMW9qT5ty8fC8_c15nTIqgIJxEnomNRS8/edit?usp=sharing

Relies on trunk change: https://ono.unity3d.com/unity/unity/pull-request/125001/_/graphics/srp/motion-vectors-fallback-material

@@ -62,6 +62,13 @@ public sealed class ShaderResources
public Shader coreBlitPS;
[Reload("Shaders/Utils/CoreBlitColorAndDepth.shader")]
public Shader coreBlitColorAndDepthPS;


[Reload("Shaders/CameraMotionVectors.shader")]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better place to keep these shaders?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is fine for them to be in this folder, since motion vectors are not specific to only post process.

@nigeljw-unity nigeljw-unity self-requested a review May 7, 2021 10:02
@erikabar erikabar self-requested a review May 7, 2021 10:12
@Verasl Verasl self-requested a review May 10, 2021 09:08
@ernestasKupciunas ernestasKupciunas self-requested a review May 12, 2021 10:02
Copy link
Member

@Verasl Verasl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Would be nice to have more effects to validate but we can do any fixes to this once we look at adding Per Object Motion Blur and TAA etc

// If the flag hasn't been set yet on this camera, motion vectors will skip a frame.
camera.depthTextureMode |= DepthTextureMode.MotionVectors | DepthTextureMode.Depth;

// TODO: add option to only draw either one?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a nice option to have.

Copy link

@ernestasKupciunas ernestasKupciunas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did some smoke testing on mobiles. Looks good. Visually scene was rendered as expected. No errors in the logcat (Android) nor in the Xcode(iOS).
Android Gles3 and Vulkan:
ezgif com-gif-maker

iOS:
ezgif com-gif-maker (1)

Unity used for testing:
Version: 2021.2.0a17.2411.graphics/srp/motion-vectors-fallback-material.10
Revision: graphics/srp/motion-vectors-fallback-material 439ecc0b6d35
Built: Mon, 10 May 2021 12:43:49 GMT

Devices under testing:
iPad Air4, SoC: A14, iOS: 14.0
iPhone 12Pro, SoC: A14, iOS: 14.1
VLNQA00217, Razer Phone 2, 9.0.0, Snapdragon 845 SDM845, Adreno 630
VLNQA00268, Samsung Galaxy S10+, 10.0.0, Exynos 9 9820, Mali-G76
OnePlus Nord, 10, Snapdragon 765G SM7250-AB, Adreno 620

Copy link

@erikabar erikabar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no issues found. Tested custom project with moving objects, moving camera, camera stacking with renderer feature dev provided in automated scene

@he-ax he-ax requested a review from thomas-zeng May 17, 2021 07:13
@phi-lira phi-lira marked this pull request as ready for review May 17, 2021 09:02
@phi-lira phi-lira requested review from a team as code owners May 17, 2021 09:02
@phi-lira phi-lira merged commit 4a4b558 into master May 17, 2021
@phi-lira phi-lira deleted the universal/motion-vectors-mvp branch May 17, 2021 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants