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.

Sign up for GitHub

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

Jump to bottom

Maintain APV cells loaded if at least one scene still references that cell #5900

Merged
merged 5 commits into from Oct 8, 2021
Merged

Maintain APV cells loaded if at least one scene still references that cell #5900

merged 5 commits into from Oct 8, 2021

Conversation

FrancescoC-unity
Copy link
Contributor

Fix for https://fogbugz.unity3d.com/f/cases/1367910/

The issue is that a cell can be shared between many assets, that also meant that if a scene is unloaded and it shared a cell with a loaded scene, then that cell will be lost for scenes that are still loaded.

This PR enables a reference counting system for cells so that they are unloaded only when reference count is actually 0.

What did I test the repro, plus heavy load/unload of scenes to see if something was wrong.

Before:

UnloadScene_broken.mp4

After

UnloadScene_fix.mp4

@FrancescoC-unity FrancescoC-unity self-assigned this Oct 4, 2021
@github-actions
Copy link

github-actions bot commented Oct 4, 2021

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)

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.

@github-actions github-actions bot added the SRP label Oct 4, 2021
@github-actions
Copy link

github-actions bot commented Oct 4, 2021

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!

@JulienIgnace-Unity JulienIgnace-Unity merged commit 07b140f into master Oct 8, 2021
@JulienIgnace-Unity JulienIgnace-Unity deleted the HDRP/fix-scene-unload branch October 8, 2021 08:24
sebastienlagarde pushed a commit that referenced this pull request Oct 18, 2021
@FrancescoC-unity @sebastienlagarde
Maintain APV cells loaded if at least one scene still references that…
4cd363d 
… cell (#5900)

* Add override checkbox.

* Fix for problem when max subdiv is smaller than index voxel update size.

* Revert "Merge branch 'HDRP/add-override-checkbox-for-pv' into HDRP/investigate-issue-with-faulty-index"

This reverts commit e33a421, reversing
changes made to 924763f.
@sebastienlagarde sebastienlagarde mentioned this pull request Oct 18, 2021
Merged
sebastienlagarde added a commit that referenced this pull request Oct 18, 2021
@sebastienlagarde @adrien-de-tocqueville
@FrancescoC-unity @anisunity @emilybrown1 @eturquin @alelievr @JulienIgnace-Unity @Vic-Cooper @kecho @nerites
[HDRP Backport 21.2] Backport various PRs (#6043)
fa135fd 
* Fixed initial decal position #5889

* Fix APV issue spewing asserts when baking with max subdiv level of 2 (#5888)

* Add override checkbox.

* Fix for problem when max subdiv is smaller than index voxel update size.

* Revert "Merge branch 'HDRP/add-override-checkbox-for-pv' into HDRP/investigate-issue-with-faulty-index"

This reverts commit e33a421, reversing
changes made to 924763f.

* Skip refresh ops if feature is disabled. (#5886)

* Fixed the clouds not taking properly into account the fog when in distant mode and with a close far plane (case 1367993). (#5884)

* Fixed the clouds not taking properly into account the fog when in distant mode and with a close far plane (case 1367993).

* Fix formatting

* Update CHANGELOG.md

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* To info box (#5879)

* Add override checkbox. (#5878)

* corrected links to HDAdditionalLightData and HDLightTypeAndShape (#5861)

* updated link to expert guide (#5857)

* Add comment (#5853)

* [HDRP][Path Tracing] Added selection of light types for Unlit shadow mattes. #5855

* Maintain APV cells loaded if at least one scene still references that cell  (#5900)

* Add override checkbox.

* Fix for problem when max subdiv is smaller than index voxel update size.

* Revert "Merge branch 'HDRP/add-override-checkbox-for-pv' into HDRP/investigate-issue-with-faulty-index"

This reverts commit e33a421, reversing
changes made to 924763f.

* Add a basic APV test to runtime tests (#5952)

* New test

* Tentative ref images

* Change threshold to be non-zero.

* Forgot one threshold...

* Add test filter.

* [HDRP] APV - Fix voxelization issues with planes at the origin (#5897)

* Fix precision issues with the scene voxelization, especially with geometry at the origin.

* Updated changelog

* Fix renderer bounds for the plane case

* disable debug

* rename epsilon

Co-authored-by: JulienIgnace-Unity <julien@unity3d.com>

* Fix another issue with probe volume subdiv multiplier when changing to a lower subdiv level (#5893)

* Add override checkbox.

* Fix for problem when max subdiv is smaller than index voxel update size.

* Revert "Merge branch 'HDRP/add-override-checkbox-for-pv' into HDRP/investigate-issue-with-faulty-index"

This reverts commit e33a421, reversing
changes made to 924763f.

* Fix issue

* Revert "Fix for problem when max subdiv is smaller than index voxel update size."

This reverts commit 03feabd.

* Fixed the volumetric clouds debug view not taking into account the exposure and leading to Nans (case 1365054). (#5940)

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Change order of fields in the cbuffer. (#5980)

Co-authored-by: FrancescoC-Unity <francescoc@unity3d.com>

* Fixed the dependency between transparent SSR and transparent depth prepass being implicit (case 1365915). (#5898)

* Fixed the dependency between transparent SSR and transparent depth prepass being implicit (case 1365915).

* review corrections

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Updating docs for merged PRs (#5821)

* PR #5459

Updated the docs changed in "Add high quality antialiasing example using the accumulation api" PR.

* Doc fixes

Fixed typo and changed SuperSampling "effect" to "method"

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* [Fogbugz # 1365687] Fixing depth pyramid with multicamera on hardware drs. #5902

* Fix (#5994)

* release cmd (#5993)

* test linux vulkan and OSX filter out. (#5989)

* Fixed removal of depth buffer binding (#5910)

* Fixed removal of depth buffer binding

* Add object in graphic test

* screenshots

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* [HDRP][Path Tracing] Added orthographic camera support #5944

* Fixed a regression in the transparent SSR color pyramid usage. (#6001)

* Fixed a regression in the transparent SSR color pyramid usage.

* Update CHANGELOG.md

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* [HDRP] Reenable crossfade on the HD SpeedTree 8 shader. #5957

* Force scalarization of shadow index data off (#6007)

* Fixes for light anchor #5915

* Minor performance improvements to SSGI (case 1367144). #5921

* Update TestCaseFilters.asset

Co-authored-by: Adrien de Tocqueville <adrien.tocqueville@unity3d.com>
Co-authored-by: FrancescoC-unity <43168857+FrancescoC-unity@users.noreply.github.com>
Co-authored-by: anisunity <42026998+anisunity@users.noreply.github.com>
Co-authored-by: emilybrown1 <88374601+emilybrown1@users.noreply.github.com>
Co-authored-by: Emmanuel Turquin <emmanuel@turquin.org>
Co-authored-by: Antoine Lelievre <antoinel@unity3d.com>
Co-authored-by: JulienIgnace-Unity <julien@unity3d.com>
Co-authored-by: FrancescoC-Unity <francescoc@unity3d.com>
Co-authored-by: Vic Cooper <63712500+Vic-Cooper@users.noreply.github.com>
Co-authored-by: Kleber Garcia <kleber.garcia@unity3d.com>
Co-authored-by: Tianliang Ning <paula.s.ning@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alelievr alelievr alelievr approved these changes

@sebastienlagarde sebastienlagarde sebastienlagarde approved these changes

@JulienIgnace-Unity JulienIgnace-Unity JulienIgnace-Unity approved these changes

Assignees

@FrancescoC-unity FrancescoC-unity

Labels
backported-2021.2 HDRP SRP
Projects
None yet
Milestone
No milestone
4 participants
@FrancescoC-unity @alelievr @sebastienlagarde @JulienIgnace-Unity