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

Acceleration Structure setup using RayTracingAccelerationStructure.CullInstances #5988

Merged
merged 9 commits into from Oct 15, 2021

Conversation

INedelcu
Copy link
Contributor

@INedelcu INedelcu commented Oct 12, 2021

Use RayTracingAccelerationStructure.CullInstances function (latest trunk - 2022.a13) to filter Renderers and populate the acceleration structure used for ray tracing (DXR). This greatly increases CPU performance on the main thread when using ray tracing effects.

On my PC with i9 with 12 cores, the performance improvement can vary between 8X and 30X or more, depending on scene size.
On a quad core laptop the CPU speedup for acceleration structure setup on the HDRP template scene is around 16x.

More information about testing and CPU profiling is on this PR which was merged already to trunk : https://ono.unity3d.com/unity/unity/pull-request/132659/_/graphics/raytracing/rtas-cull-instances

HDRP DXR tests on trunk and this branch is green: https://unity-ci.cds.internal.unity3d.com/job/9294644

@github-actions
Copy link

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://unity-ci.cds.internal.unity3d.com/project/902/
Search for your PR branch using the search bar at the top, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

HDRP
/jobDefinition/.yamato%2Fall-hdrp.yml%23HDRP_trunk
With changes to HDRP packages, you should also run
/jobDefinition/.yamato%252Fall-lightmapper.yml%2523PR_LightMapper_trunk

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

@github-actions github-actions bot added the HDRP label Oct 12, 2021
@github-actions
Copy link

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page).
See the PR template for more information.
Thank you!

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to the Unity SRP repo!
Please make sure to fill out the PR template as best you can to give reviewers as much information as possible.
If you have any questions (and you are a Unity employee) go to "#devs-renderpipe"

@anisunity anisunity added the dxr label Oct 12, 2021
@IonutNedelcuUnity IonutNedelcuUnity changed the title Hdrp/raytracing/rtas cullinstances Acceleration Structure setup using RayTracingAccelerationStructure.CullInstances Oct 12, 2021
@anisunity
Copy link
Contributor

did a pass on the PR, tests are passing locally, @remi-chapelain you can check it out

@anisunity anisunity requested a review from a team October 13, 2021 12:14
@TomasKiniulis
Copy link
Contributor

@INedelcu any notes what to focus on for qa here?

@IonutNedelcuUnity
Copy link
Contributor

IonutNedelcuUnity commented Oct 13, 2021

@INedelcu any notes what to focus on for qa here?

@TomasKiniulis if you can test a project that uses path tracing, make sure that path tracing convergence is reset when changing the Transform by moving the object around or changing Material properties. I tested this already locally but there are no graphics tests that do this exactly. You can also use one of the path tracing tests that we have in HDRP_DXR_Tests project, click on Play and play with the Material (change properties or just delete whole Materials) and move objects around. This should reset path tracing convergence.
HDRP DXR tests were green on my initial commits - https://unity-ci.cds.internal.unity3d.com/job/9294644

Here's a video - https://drive.google.com/file/d/1RSUtATyYybvRQpa15J9jNXAHEVdH0-WC/view

Copy link
Contributor

@remi-chapelain remi-chapelain left a comment

Choose a reason for hiding this comment

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

Here's what I tried:

  • Adding pathtracer and testing the dirtiness of most change (transform, materials, volume...) to reset accumulation ✔️
  • Checked performance in the HDRP template (not much difference, makes sense since building RTAS is probably not the bottleneck here)
  • Tested performance (FPS) with huge number of objects ✔️ (details below)
  • Let the project run in player and editor a bit to see if performance was staying the same ✔️
  • Checked RaytracingBuildAccelerationStructure new markers timings. Going from 52.88ms to 4.02ms so a 13x decrease ! ✔️

Before
image

After
image

10k emissive sphere in a box.
Without PR: average frame time 225ms
With PR: average frame time 136ms
40% decrease ! ✔️
https://user-images.githubusercontent.com/57442369/137454155-6b7b8a30-434e-4650-9904-9e85028bf6f1.mp4

Complex static scene lighted only with emissive objects
Without PR: average frame time 196ms
With PR: average frame time: 99ms
50% decrease ! ✔️
RemiCHAPELAIN_Direct Lighting + Probe + RTGI + RTR

@anisunity anisunity force-pushed the HDRP/raytracing/rtas-cullinstances branch from a8d3043 to 2066ada Compare October 15, 2021 14:39
@anisunity
Copy link
Contributor

Merged with master and did the formatting

@anisunity anisunity merged commit 005f37a into master Oct 15, 2021
@anisunity anisunity deleted the HDRP/raytracing/rtas-cullinstances branch October 15, 2021 16:40
public List<RayTracingInstanceCullingTest> instanceTestArray = new List<RayTracingInstanceCullingTest>();

// Culling tests
RayTracingInstanceCullingTest ShT_CT = new RayTracingInstanceCullingTest();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry but from the comment I don't understand why this code is here?
But I guess "Test" here doesn't mean "Testing" right? i.e not part of our test framework

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah not part of the test framework, it's just the set of tests that are used to inject into the RTAS @sebastienlagarde

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants