Minimal enclosing sphere backport to 2021.2 #5722

nigeljw-unity · 2021-09-20T08:41:49Z

Backport of minimal enclosing sphere solution for shadow frustum culling.

Checklist for PR maker

Have you added a backport label (if needed)? For example, the need-backport-* label. After you backport the PR, the label changes to backported-*.
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

Fixes bug where shadows disappear in the frustum corners from false positives in the shadow culling.
fogbugz.unity3d.com/f/cases/1153151

Testing status

Tested with URP default template and custom cascade positioning.

Comments to reviewers

Dependent on this change to trunk.
https://ono.unity3d.com/unity/unity/pull-request/131872/_/2021.2/minimal_enclosing_cascade_sphere

Backport of minimal enclosing sphere solution for shadow frustum culling. #4436 Co-authored-by: Erik Hakala <erik.hakala@unity3d.com> Co-authored-by: Oleksandr Kokoshyn <oleksandr.kokoshyn@unity3d.com>

github-actions · 2021-09-20T08:42:03Z

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://yamato.cds.internal.unity3d.com/jobs/902-Graphics
Search for your PR branch using the sidebar on the left, 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
/.yamato%252Fall-hdrp.yml%2523PR_HDRP_2021.2

URP
/.yamato%252Fall-urp.yml%2523PR_URP_2021.2

Shader Graph
/.yamato%252Fall-shadergraph.yml%2523PR_ShaderGraph_2021.2
Depending on your PR, you may also want
/.yamato%252Fall-shadergraph_builtin_foundation.yml%2523PR_ShaderGraph_BuiltIn_Foundation_2021.2
/.yamato%252Fall-shadergraph_builtin_lighting.yml%2523PR_ShaderGraph_BuiltIn_Lighting_2021.2

VFX
/.yamato%252Fall-vfx.yml%2523PR_VFX_2021.2

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:
/.yamato%252F_abv.yml%2523all_project_ci_2021.2
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.

com.unity.render-pipelines.universal/Runtime/Data/UniversalRenderPipelineAsset.cs

phi-lira · 2021-09-23T07:29:24Z

com.unity.render-pipelines.universal/Runtime/UniversalRenderPipeline.cs

            get => 2.0f;
        }

+        public static int maxNumIterationsEnclosingSphere


Missing public API docs.

sandy-carter-unity · 2021-09-23T17:34:15Z

com.unity.render-pipelines.universal/Runtime/Data/UniversalRenderPipelineAsset.cs

+            set { m_ConservativeEnclosingSphere = value; }
+        }
+
+        public int numItertionsEnclosingSphere


Missing an "a" in "Iterations"

Thank you for catching this!!!

nigeljw-unity · 2021-09-24T15:17:41Z

com.unity.render-pipelines.universal/Runtime/Data/UniversalRenderPipelineAsset.cs


+        /// <summary>
+        /// Set to true to enable a conservative method for calculating the size and position of the minimal enclosing sphere around the frustum cascade corner points for shadow culling.
+        /// </summary>


@oleks-k can you review this?

…cs into universal/minimal_enclosing_sphere_backport

simon-engelbrecht-soerensen

Ran it through the same tests as for the original version, and toggling on enclosing spheres fixes the shadow issues. Nice!

…cs into universal/minimal_enclosing_sphere_backport

oleks-k

Looks good.

…_backport

Minimal enclosing sphere fix

7c5c650

Backport of minimal enclosing sphere solution for shadow frustum culling. #4436 Co-authored-by: Erik Hakala <erik.hakala@unity3d.com> Co-authored-by: Oleksandr Kokoshyn <oleksandr.kokoshyn@unity3d.com>

nigeljw-unity requested review from a team as code owners September 20, 2021 08:41

github-actions bot added the universal label Sep 20, 2021

nigeljw-unity requested a review from ellioman September 20, 2021 08:42

ellioman approved these changes Sep 20, 2021

View reviewed changes

phi-lira reviewed Sep 23, 2021

View reviewed changes

sandy-carter-unity reviewed Sep 23, 2021

View reviewed changes

Fixed API typo, added docs, and bumped min version

60efdc9

nigeljw-unity commented Sep 24, 2021

View reviewed changes

nigeljw-unity added 2 commits September 24, 2021 17:19

Removed dead code

f7d99fd

Merge branch '2021.2/staging' of github.com:Unity-Technologies/Graphi…

1d1cd96

…cs into universal/minimal_enclosing_sphere_backport

nigeljw-unity requested a review from oleks-k September 28, 2021 08:53

phi-lira approved these changes Sep 28, 2021

View reviewed changes

simon-engelbrecht-soerensen approved these changes Sep 29, 2021

View reviewed changes

nigeljw-unity added 2 commits September 29, 2021 15:33

Merge branch '2021.2/staging' of github.com:Unity-Technologies/Graphi…

16ed179

…cs into universal/minimal_enclosing_sphere_backport

Missing package bumps

00b46f7

nigeljw-unity requested a review from a team as a code owner September 29, 2021 13:39

sebastienlagarde approved these changes Sep 29, 2021

View reviewed changes

marctem approved these changes Sep 29, 2021

View reviewed changes

oleks-k approved these changes Sep 30, 2021

View reviewed changes

Merge branch '2021.2/staging' into universal/minimal_enclosing_sphere…

636b7fc

…_backport

nigeljw-unity merged commit 3639f88 into 2021.2/staging Oct 4, 2021

nigeljw-unity deleted the universal/minimal_enclosing_sphere_backport branch October 4, 2021 17:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Minimal enclosing sphere backport to 2021.2 #5722

Minimal enclosing sphere backport to 2021.2 #5722

Uh oh!

nigeljw-unity commented Sep 20, 2021

Uh oh!

github-actions bot commented Sep 20, 2021 •

edited

Loading

Uh oh!

Uh oh!

phi-lira Sep 23, 2021

Uh oh!

sandy-carter-unity Sep 23, 2021 •

edited

Loading

Uh oh!

nigeljw-unity Sep 24, 2021

Uh oh!

nigeljw-unity Sep 24, 2021

Uh oh!

simon-engelbrecht-soerensen left a comment

Uh oh!

oleks-k left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Minimal enclosing sphere backport to 2021.2 #5722

Minimal enclosing sphere backport to 2021.2 #5722

Uh oh!

Conversation

nigeljw-unity commented Sep 20, 2021

Checklist for PR maker

Purpose of this PR

Testing status

Comments to reviewers

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

phi-lira Sep 23, 2021

Choose a reason for hiding this comment

Uh oh!

sandy-carter-unity Sep 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nigeljw-unity Sep 24, 2021

Choose a reason for hiding this comment

Uh oh!

nigeljw-unity Sep 24, 2021

Choose a reason for hiding this comment

Uh oh!

simon-engelbrecht-soerensen left a comment

Choose a reason for hiding this comment

Uh oh!

oleks-k left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

github-actions bot commented Sep 20, 2021 •

edited

Loading

sandy-carter-unity Sep 23, 2021 •

edited

Loading