[HDRP] Implement conservative voxelization for APV #6473

alelievr · 2021-12-03T12:40:55Z

Purpose of this PR

This PR improves the behavior of the voxelization used to place probes for APV when dealing with small objects.
The problem was that when an object is smaller than a brick (3 x min probe distance) it could be ignored by the probe placement system, this is caused by the rasterization of objects and can be solved by enabling conservative rasterization
There is also a small bugfix about cell position not being correct when enabling realtime voxelization
Replaced the geometry distance offset since it's not necessary anymore with the conservative rasterization and added a new field called "Minimum Renderer Volume Size" to ignore small renderers when placing probes.

You can override this new field per probe volume or set it globally in the baking set:

When the bounding box volume of a renderer in a probe volume is smaller than the value of this field, then the renderer is discarded from the list of objects to place probes around

Added a new LayerMask field in the APV window and an override on the probe volume component instead of only being able to set it on the probe volume.

technical details

The main issue with conservative rasterization is that it's not made to work in 3D and causes extreme artifacts when directly writing to a UAV in a fragment. There is no good solution for this as it's expected that conservative rasterization gives "bad" results for fragment interpolated values, especially when the triangle is close to being degenerate. So the only solution I found was to discard voxels written outside of the bounding box of the current rasterized triangle (this is done with a geometry shader, it's not optimal but still better than writing a software rasterizer in compute).

Note that in some cases this implementation can lead to false positives and generate more probes than before (especially in scenes with very large triangular shapes).

Testing status

Without conservative voxelization

WithoutConservative.mp4

With conservative voxelization

WithConservative.mp4

Minimum renderers size feature:

Renderers.volume.size.mp4

github-actions · 2021-12-03T12:41:10Z

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%23PR_HDRP_trunk
With changes to HDRP packages, you should also run
/jobDefinition/.yamato%2Fall-lightmapping.yml%23PR_Lightmapping_trunk

SRP Core
You could run ABV on your branch before merging your PR, but it will start A LOT of jobs. Please be responsible about it and run it only when you feel the PR is ready:
/jobDefinition/.yamato%252F_abv.yml%2523all_project_ci_trunk
Be aware that any modifications to the Core package impacts everyone in the Graphics repo so please discuss the PR with your lead.

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.

FrancescoC-unity · 2021-12-03T12:43:07Z

Because this can lead to more probes we should add this as an option (ON by default probably, but turn offable regardless)

JMargevics · 2021-12-03T12:57:49Z

Don't forget the changelog 👍

…ume field

FrancescoC-unity · 2021-12-03T17:22:58Z

com.unity.render-pipelines.core/Editor/Lighting/ProbeVolume/ProbeGIBaking.cs

                        var go = renderer.component.gameObject;
                        int rendererLayerMask = 1 << go.layer;
+                        renderer.volume.CalculateCenterAndSize(out _, out var rendererBoundsSize);
+                        float rendererBoundsVolume = rendererBoundsSize.x * rendererBoundsSize.y * rendererBoundsSize.z;


To avoid issues with planes I'd say we should min all dimensions to avoid 0, probably to 2/3 cm?

So, in this renderer size, there is already padding of 1cm in every axis

FrancescoC-unity · 2021-12-03T17:29:09Z

com.unity.render-pipelines.core/Editor/Lighting/ProbeVolume/ProbeVolumeUI.Skin.cs

            internal static readonly GUIContent s_ObjectLayerMask = new GUIContent("Layer Mask", "Control which layers will be used to select the meshes for the probe placement algorithm.");
-            internal static readonly GUIContent s_GeometryDistanceOffset = new GUIContent("Probe Placement Distance Offset", "Affects the minimum distance at which the subdivision system will attempts to place probes near the geometry. This value can be useful in situations where the generated probes don't fully cover an object.");
+            internal static readonly GUIContent s_MinRendererVolumeSize = new GUIContent("Minimum Renderer Volume Size", "Specifies the minimum bounding box volume of renderers to consider placing probes around.");
+            internal static readonly GUIContent s_OverrideMinRendererVolumeSize = new GUIContent("Override Renderer Filters", "Overrides the Minimum Renderer Volume Size specified in the Probe Volume Settings Window.");


I assume this

Override Renderer Filters

Should have been Minimum Renderer Volume Size?

Override Minimum Renderer Volume Size
Was too long in the inspector, needed to find another name

# Conflicts: # com.unity.render-pipelines.core/Runtime/Lighting/ProbeVolume/ProbeVolume.cs

FrancescoC-unity

LGTM waiting for QA testing.

TomasKiniulis

Haven't ran into any issues. Changes look good to me!

What I've checked:

Renderer Filter Settings - Layer Mask
Renderer Filter Settings - Minimum Renderer Volume Size, smaller objects are correctly ignored
- Bounding Box Of 0.5x0.5x0.5 - bricks are shown with Min Renderer Volume Size up to 0.13
- Bounding Box Of 1x1x1 - bricks are shown with Min Renderer Volume Size up to 1.03
- Bounding Box Of 5x5x5 - bricks are shown with Min Renderer Volume Size up to 125
- Bounding Box Of 5x10x50 - bricks are shown with Min Renderer Volume Size up to 2500
- Overlapping bounding boxes - overlap is ignored, setting only checks for individual renderer bounding boxes
Multiple local volumes with different settings
Complex shapes
Different Max Distance Between Probes settings

Probe comparison between master and PR in Classroom project - looks nearly identical

Classroom scene PR

Classroom scene master

sebastienlagarde · 2021-12-16T12:35:55Z

quesotin: is it possible to create an APV test like this ?

TomasKiniulis · 2021-12-16T12:59:21Z

quesotin: is it possible to create an APV test like this ?

I could try to see if I can add one in. Although do we have a way to visualize probe volume subdivision levels debug mode in Graphics Test? We have a Debug View Controller script in use for some scenes, but it seems to support only Materials, Lighting and Rendering section, will need to dig into it how to get that extended.

FrancescoC-unity · 2021-12-16T13:15:00Z

quesotin: is it possible to create an APV test like this ?

It'd be cool, the problem is that it'd also be fairly slow as we'd need to retrigger the baking.

Although I guess we can have a scene with realtime subdivision enabled, that won't be too slow. With realtime subdiv and no asset assigned in the scene (no baking ever triggered), it should work, no @alelievr ?

This reverts commit 1fe51a8. # Conflicts: # com.unity.render-pipelines.high-definition/CHANGELOG.md

alelievr and others added 5 commits December 2, 2021 19:03

Added conservative voxelization shader

fc2c0f8

Fix realtime cell debug

c004b08

Remove default geometry distance offset

37f0120

Enable conservative

433a624

comment debug

ea90f41

alelievr added the HDRP label Dec 3, 2021

alelievr requested review from a team, FrancescoC-unity, JulienIgnace-Unity and sebastienlagarde December 3, 2021 12:40

alelievr self-assigned this Dec 3, 2021

github-actions bot added the SRP label Dec 3, 2021

Antoine Lelievre added 3 commits December 3, 2021 18:03

Remove geometry distance offset and add min renderer bounding box vol…

55a6409

…ume field

Fix baking window style

025aebe

Updated changelog

8e9a21f

alelievr marked this pull request as ready for review December 3, 2021 17:09

FrancescoC-unity reviewed Dec 3, 2021

View reviewed changes

JulienIgnace-Unity approved these changes Dec 6, 2021

View reviewed changes

alelievr added 4 commits December 7, 2021 14:33

Merge branch 'master' into hd/apv-conservative-voxelization

2405fcd

# Conflicts: # com.unity.render-pipelines.core/Runtime/Lighting/ProbeVolume/ProbeVolume.cs

Added probe volume layer mask in APV profile

ce7f0e6

Updated changelog

0088431

Fix indent

270c53b

FrancescoC-unity approved these changes Dec 14, 2021

View reviewed changes

TomasKiniulis approved these changes Dec 15, 2021

View reviewed changes

Merge branch 'master' into hd/apv-conservative-voxelization

1293ff1

sebastienlagarde approved these changes Dec 16, 2021

View reviewed changes

sebastienlagarde merged commit 1fe51a8 into master Dec 16, 2021

sebastienlagarde deleted the hd/apv-conservative-voxelization branch December 16, 2021 12:38

FrancescoC-unity added a commit that referenced this pull request Jan 4, 2022

Revert "[HDRP] Implement conservative voxelization for APV (#6473)"

dac8552

This reverts commit 1fe51a8. # Conflicts: # com.unity.render-pipelines.high-definition/CHANGELOG.md

[HDRP] Implement conservative voxelization for APV #6473

[HDRP] Implement conservative voxelization for APV #6473

Uh oh!

Conversation

alelievr commented Dec 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this PR

technical details

Testing status

Uh oh!

github-actions bot commented Dec 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FrancescoC-unity commented Dec 3, 2021

Uh oh!

JMargevics commented Dec 3, 2021

Uh oh!

FrancescoC-unity Dec 3, 2021

Choose a reason for hiding this comment

Uh oh!

alelievr Dec 3, 2021

Choose a reason for hiding this comment

Uh oh!

FrancescoC-unity Dec 3, 2021

Choose a reason for hiding this comment

Uh oh!

alelievr Dec 3, 2021

Choose a reason for hiding this comment

Uh oh!

FrancescoC-unity left a comment

Choose a reason for hiding this comment

Uh oh!

TomasKiniulis left a comment

Choose a reason for hiding this comment

Uh oh!

sebastienlagarde commented Dec 16, 2021

Uh oh!

TomasKiniulis commented Dec 16, 2021

Uh oh!

FrancescoC-unity commented Dec 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

alelievr commented Dec 3, 2021 •

edited

Loading

github-actions bot commented Dec 3, 2021 •

edited

Loading