Mask Volume visibility fixes (#84)

AndrewSaraevUnity · web-flow · commit a847d9b59e0f · 2022-05-14T02:22:59.000+01:00
* Don't render Mask Volume hidden in scene.

* Fixed Mask Volume flickering when two volume instances are trying to render the same asset.
diff --git a/com.unity.render-pipelines.high-definition/Runtime/Lighting/ProbeVolume/ProbeVolumeHandle.cs b/com.unity.render-pipelines.high-definition/Runtime/Lighting/ProbeVolume/ProbeVolumeHandle.cs
@@ -60,7 +60,7 @@ public OrientedBBox ConstructOBBEngineData(Vector3 cameraOffset)
         }
 
 #if UNITY_EDITOR
-        public bool IsHiddesInScene() => m_List.IsHiddenInScene(m_Index);
+        public bool IsHiddenInScene() => m_List.IsHiddenInScene(m_Index);
 #endif
     }
 }
diff --git a/com.unity.render-pipelines.high-definition/Runtime/Lighting/ProbeVolume/ProbeVolumeLighting.cs b/com.unity.render-pipelines.high-definition/Runtime/Lighting/ProbeVolume/ProbeVolumeLighting.cs
@@ -1228,7 +1228,7 @@ void PrepareVisibleProbeVolumeListBuffers(HDCamera hdCamera, CommandBuffer immed
                     if (
 #if UNITY_EDITOR
                         volume.IsAssetCompatible() &&
-                        !volume.IsHiddesInScene() &&
+                        !volume.IsHiddenInScene() &&
 #endif
                         volume.IsDataAssigned() &&
                         blendModeIsSupported &&
diff --git a/com.unity.render-pipelines.high-definition/Runtime/Material/MaskVolume/IMaskVolumeList.cs b/com.unity.render-pipelines.high-definition/Runtime/Material/MaskVolume/IMaskVolumeList.cs
@@ -1,10 +1,7 @@
-using static UnityEngine.Rendering.HighDefinition.VolumeGlobalUniqueIDUtils;
-
 namespace UnityEngine.Rendering.HighDefinition
 {
     interface IMaskVolumeList
     {
-        void ReleaseRemovedVolumesFromAtlas();
         int GetVolumeCount();
         
         bool IsDataAssigned(int i);
@@ -15,13 +12,13 @@ interface IMaskVolumeList
         Quaternion GetRotation(int i);
 
         ref MaskVolumeArtistParameters GetParameters(int i);
-
-        VolumeGlobalUniqueID GetAtlasID(int i);
         MaskVolume.MaskVolumeAtlasKey ComputeMaskVolumeAtlasKey(int i);
-        MaskVolume.MaskVolumeAtlasKey GetMaskVolumeAtlasKeyPrevious(int i);
-        void SetMaskVolumeAtlasKeyPrevious(int i, MaskVolume.MaskVolumeAtlasKey key);
 
         int GetDataSHL0Length(int i);
         void SetDataSHL0(CommandBuffer cmd, int i, ComputeBuffer buffer);
+
+#if UNITY_EDITOR
+        bool IsHiddenInScene(int i);
+#endif
     }
 }
diff --git a/com.unity.render-pipelines.high-definition/Runtime/Material/MaskVolume/MaskVolume.cs b/com.unity.render-pipelines.high-definition/Runtime/Material/MaskVolume/MaskVolume.cs
@@ -1,5 +1,4 @@
 using System;
-using System.Collections.Generic;
 using System.Runtime.CompilerServices;
 using static UnityEngine.Rendering.HighDefinition.VolumeGlobalUniqueIDUtils;
 
@@ -347,6 +346,8 @@ internal MaskVolumeAtlasKey ComputeMaskVolumeAtlasKey()
             if (maskVolumeAsset == null)
                 return MaskVolumeAtlasKey.zero;
 
+            // Use the payloadID, rather than the probe volume ID to uniquely identify data in the atlas.
+            // This ensures that if 2 mask volumes exist that point to the same data, that data will only be uploaded once.
             return ComputeMaskVolumeAtlasKey(GetPayloadID(), maskVolumeAsset.resolutionX, maskVolumeAsset.resolutionY, maskVolumeAsset.resolutionZ);
         }
 
@@ -361,30 +362,11 @@ internal static MaskVolumeAtlasKey ComputeMaskVolumeAtlasKey(VolumeGlobalUniqueI
             };
         }
 
-        private MaskVolumeAtlasKey maskVolumeAtlasKeyPrevious;
-
-        internal MaskVolumeAtlasKey GetMaskVolumeAtlasKeyPrevious()
-        {
-            return maskVolumeAtlasKeyPrevious;
-        }
-
-        internal void SetMaskVolumeAtlasKeyPrevious(MaskVolumeAtlasKey key)
-        {
-            maskVolumeAtlasKeyPrevious = key;
-        }
-
         private VolumeGlobalUniqueID GetPayloadID()
         {
             return (maskVolumeAsset == null) ? VolumeGlobalUniqueID.zero : maskVolumeAsset.GetID();
         }
 
-        internal VolumeGlobalUniqueID GetAtlasID()
-        {
-            // Use the payloadID, rather than the probe volume ID to uniquely identify data in the atlas.
-            // This ensures that if 2 mask volumes exist that point to the same data, that data will only be uploaded once.
-            return GetPayloadID();
-        }
-
         internal Vector3Int GetResolution()
         {
             return new Vector3Int(maskVolumeAsset.resolutionX, maskVolumeAsset.resolutionY, maskVolumeAsset.resolutionZ);
@@ -533,8 +515,6 @@ internal void ResampleAsset()
                         }
                     }
                 }
-
-                MaskVolumeManager.manager.ReleaseVolumeFromAtlas(this);
             }
 
             maskVolumeAsset.resolutionX = parameters.resolutionX;
diff --git a/com.unity.render-pipelines.high-definition/Runtime/Material/MaskVolume/MaskVolumeHandle.cs b/com.unity.render-pipelines.high-definition/Runtime/Material/MaskVolume/MaskVolumeHandle.cs
@@ -1,5 +1,3 @@
-using static UnityEngine.Rendering.HighDefinition.VolumeGlobalUniqueIDUtils;
-
 namespace UnityEngine.Rendering.HighDefinition
 {
     struct MaskVolumeHandle
@@ -21,13 +19,13 @@ public MaskVolumeHandle(IMaskVolumeList list, int index)
         public Quaternion rotation => m_List.GetRotation(m_Index);
 
         public ref MaskVolumeArtistParameters parameters => ref m_List.GetParameters(m_Index);
-
-        public VolumeGlobalUniqueID GetAtlasID() => m_List.GetAtlasID(m_Index);
         public MaskVolume.MaskVolumeAtlasKey ComputeMaskVolumeAtlasKey() => m_List.ComputeMaskVolumeAtlasKey(m_Index);
-        public MaskVolume.MaskVolumeAtlasKey GetMaskVolumeAtlasKeyPrevious() => m_List.GetMaskVolumeAtlasKeyPrevious(m_Index);
-        public void SetMaskVolumeAtlasKeyPrevious(MaskVolume.MaskVolumeAtlasKey key) => m_List.SetMaskVolumeAtlasKeyPrevious(m_Index, key);
 
         public int DataSHL0Length => m_List.GetDataSHL0Length(m_Index);
         public void SetDataSHL0(CommandBuffer cmd, ComputeBuffer buffer) => m_List.SetDataSHL0(cmd, m_Index, buffer);
+
+#if UNITY_EDITOR
+        public bool IsHiddenInScene() => m_List.IsHiddenInScene(m_Index);
+#endif
     }
 }
diff --git a/com.unity.render-pipelines.high-definition/Runtime/Material/MaskVolume/MaskVolumeManager.cs b/com.unity.render-pipelines.high-definition/Runtime/Material/MaskVolume/MaskVolumeManager.cs
@@ -1,5 +1,4 @@
 using System.Collections.Generic;
-using static UnityEngine.Rendering.HighDefinition.VolumeGlobalUniqueIDUtils;
 
 namespace UnityEngine.Rendering.HighDefinition
 {
@@ -32,7 +31,6 @@ internal List<MaskVolumeHandle> CollectVolumesToRender()
                 m_VolumeHandles.Add(new MaskVolumeHandle(this, i));
             foreach (var list in m_AdditionalMaskLists)
             {
-                list.ReleaseRemovedVolumesFromAtlas();
                 count = list.GetVolumeCount();
                 for (int i = 0; i < count; i++)
                     m_VolumeHandles.Add(new MaskVolumeHandle(list, i));
@@ -54,24 +52,8 @@ internal void DeRegisterVolume(MaskVolume volume)
             if (index == -1)
                 return;
 
-            ReleaseVolumeFromAtlas(new MaskVolumeHandle(this, index));
             m_Volumes.RemoveAt(index);
         }
-        
-        public void ReleaseVolumeFromAtlas(MaskVolume volume)
-        {
-            var index = m_Volumes.IndexOf(volume);
-            if (index == -1)
-                return;
-
-            ReleaseVolumeFromAtlas(new MaskVolumeHandle(this, index));
-        }
-
-        public void ReleaseVolumeFromAtlas(MaskVolumeHandle volume)
-        {
-            if (RenderPipelineManager.currentPipeline is HDRenderPipeline hdrp)
-                hdrp.ReleaseMaskVolumeFromAtlas(volume);
-        }
 
         public void AddMaskList(IMaskVolumeList list)
         {
@@ -82,8 +64,7 @@ public void RemoveMaskList(IMaskVolumeList list)
         {
             m_AdditionalMaskLists.Remove(list);
         }
-        
-        void IMaskVolumeList.ReleaseRemovedVolumesFromAtlas() { }
+
         int IMaskVolumeList.GetVolumeCount() => m_Volumes.Count;
         bool IMaskVolumeList.IsDataAssigned(int i) => m_Volumes[i].IsDataAssigned();
         bool IMaskVolumeList.IsDataUpdated(int i) => m_Volumes[i].dataUpdated;
@@ -92,12 +73,13 @@ void IMaskVolumeList.ReleaseRemovedVolumesFromAtlas() { }
         Vector3 IMaskVolumeList.GetPosition(int i) => m_Volumes[i].transform.position;
         Quaternion IMaskVolumeList.GetRotation(int i) => m_Volumes[i].transform.rotation;
         ref MaskVolumeArtistParameters IMaskVolumeList.GetParameters(int i) => ref m_Volumes[i].parameters;
-        VolumeGlobalUniqueID IMaskVolumeList.GetAtlasID(int i) => m_Volumes[i].GetAtlasID();
         MaskVolume.MaskVolumeAtlasKey IMaskVolumeList.ComputeMaskVolumeAtlasKey(int i) => m_Volumes[i].ComputeMaskVolumeAtlasKey();
-        MaskVolume.MaskVolumeAtlasKey IMaskVolumeList.GetMaskVolumeAtlasKeyPrevious(int i) => m_Volumes[i].GetMaskVolumeAtlasKeyPrevious();
-        void IMaskVolumeList.SetMaskVolumeAtlasKeyPrevious(int i, MaskVolume.MaskVolumeAtlasKey key) => m_Volumes[i].SetMaskVolumeAtlasKeyPrevious(key);
 
         int IMaskVolumeList.GetDataSHL0Length(int i) => m_Volumes[i].GetPayload().dataSHL0.Length;
         void IMaskVolumeList.SetDataSHL0(CommandBuffer cmd, int i, ComputeBuffer buffer) => cmd.SetComputeBufferData(buffer, m_Volumes[i].GetPayload().dataSHL0);
+
+#if UNITY_EDITOR
+        bool IMaskVolumeList.IsHiddenInScene(int i) => UnityEditor.SceneVisibilityManager.instance.IsHidden(m_Volumes[i].gameObject);
+#endif
     }
 }
diff --git a/com.unity.render-pipelines.high-definition/Runtime/Material/MaskVolume/MaskVolumeRendering.cs b/com.unity.render-pipelines.high-definition/Runtime/Material/MaskVolume/MaskVolumeRendering.cs
@@ -2,7 +2,6 @@
 using System.Collections.Generic;
 using System.Runtime.InteropServices;
 using UnityEngine.Experimental.Rendering.RenderGraphModule;
-using static UnityEngine.Rendering.HighDefinition.VolumeGlobalUniqueIDUtils;
 
 namespace UnityEngine.Rendering.HighDefinition
 {
@@ -122,6 +121,7 @@ public partial class HDRenderPipeline
         static int s_MaxMaskVolumeMaskCount;
         RTHandle m_MaskVolumeAtlasSHRTHandle;
 
+        List<MaskVolume.MaskVolumeAtlasKey> keysInAtlas;
         Texture3DAtlasDynamic<MaskVolume.MaskVolumeAtlasKey> maskVolumeAtlas = null;
 
         bool isClearMaskVolumeAtlasRequested = false;
@@ -207,6 +207,7 @@ internal void CreateMaskVolumeBuffers()
                 name:              "MaskVolumeAtlasSH"
             );
 
+            keysInAtlas = new List<MaskVolume.MaskVolumeAtlasKey>();
             maskVolumeAtlas = new Texture3DAtlasDynamic<MaskVolume.MaskVolumeAtlasKey>(s_MaskVolumeAtlasResolution, s_MaskVolumeAtlasResolution, s_MaskVolumeAtlasResolution, k_MaxVisibleMaskVolumeCount, m_MaskVolumeAtlasSHRTHandle);
         }
 
@@ -221,6 +222,7 @@ internal void DestroyMaskVolumeBuffers()
             if (m_MaskVolumeAtlasSHRTHandle != null)
                 RTHandles.Release(m_MaskVolumeAtlasSHRTHandle);
 
+            keysInAtlas = null;
             if (maskVolumeAtlas != null)
                 maskVolumeAtlas.Release();
 
@@ -332,36 +334,8 @@ private static void DoPushMaskVolumesGlobalParams(
             cmd.SetGlobalTexture(HDShaderIDs._MaskVolumeAtlasSH, maskVolumeAtlas);
         }
 
-        internal void ReleaseMaskVolumeFromAtlas(MaskVolumeHandle volume)
+        bool EnsureMaskVolumeInAtlas(CommandBuffer immediateCmd, RenderGraph renderGraph, ref MaskVolumesResources resources, MaskVolumeHandle volume)
         {
-            if (!m_SupportMaskVolume)
-                return;
-
-            MaskVolume.MaskVolumeAtlasKey key = volume.ComputeMaskVolumeAtlasKey();
-            MaskVolume.MaskVolumeAtlasKey keyPrevious = volume.GetMaskVolumeAtlasKeyPrevious();
-
-            if (maskVolumeAtlas.IsTextureSlotAllocated(key)) { maskVolumeAtlas.ReleaseTextureSlot(key); }
-            if (maskVolumeAtlas.IsTextureSlotAllocated(keyPrevious)) { maskVolumeAtlas.ReleaseTextureSlot(keyPrevious); }
-        }
-
-        internal void EnsureStaleDataIsFlushedFromAtlases(MaskVolumeHandle volume)
-        {
-            MaskVolume.MaskVolumeAtlasKey key = volume.ComputeMaskVolumeAtlasKey();
-            MaskVolume.MaskVolumeAtlasKey keyPrevious = volume.GetMaskVolumeAtlasKeyPrevious();
-            if (!key.Equals(keyPrevious))
-            {
-                if (maskVolumeAtlas.IsTextureSlotAllocated(keyPrevious))
-                {
-                    maskVolumeAtlas.ReleaseTextureSlot(keyPrevious);
-                }
-
-                volume.SetMaskVolumeAtlasKeyPrevious(key);
-            }
-        }
-
-        internal bool EnsureMaskVolumeInAtlas(CommandBuffer immediateCmd, RenderGraph renderGraph, ref MaskVolumesResources resources, MaskVolumeHandle volume)
-        {
-            VolumeGlobalUniqueID id = volume.GetAtlasID();
             var resolution = volume.GetResolution();
             int size = resolution.x * resolution.y * resolution.z;
             Debug.Assert(size > 0, "MaskVolume: Encountered mask volume with resolution set to zero on all three axes.");
@@ -376,14 +350,13 @@ internal bool EnsureMaskVolumeInAtlas(CommandBuffer immediateCmd, RenderGraph re
 
             if (isSlotAllocated)
             {
+                // Sync local key list with any allocated slot, no matter if we will actually upload anything to it.
+                // Needed to identify and free the slot later when no volumes with this key is present in a frame.
+                if (!keysInAtlas.Contains(key))
+                    keysInAtlas.Add(key);
+                
                 if (isUploadNeeded || volume.IsDataUpdated())
                 {
-                    if (!volume.IsDataAssigned())
-                    {
-                        ReleaseMaskVolumeFromAtlas(volume);
-                        return false;
-                    }
-
                     int sizeSHCoefficientsL0 = size * MaskVolumePayload.GetDataSHL0Stride();
                     Debug.AssertFormat(volume.DataSHL0Length == sizeSHCoefficientsL0, "MaskVolume: The mask volume data and its resolution are out of sync! Volume data length is {0}, but resolution * SH stride size is {1}.", volume.DataSHL0Length, sizeSHCoefficientsL0);
 
@@ -432,14 +405,13 @@ internal bool EnsureMaskVolumeInAtlas(CommandBuffer immediateCmd, RenderGraph re
                     }
 
                     return true;
-
                 }
                 return false;
             }
 
             if (!isSlotAllocated)
             {
-                Debug.LogWarningFormat("MaskVolume: Texture Atlas failed to allocate space for texture (id: {0}, width: {1}, height: {2}, depth: {3})", id, resolution.x, resolution.y, resolution.z);
+                Debug.LogWarningFormat("MaskVolume: Texture Atlas failed to allocate space for texture (id: {0}, width: {1}, height: {2}, depth: {3})", key.id, resolution.x, resolution.y, resolution.z);
             }
 
             return false;
@@ -497,6 +469,7 @@ internal void ClearMaskVolumeAtlasIfRequested(CommandBuffer immediateCmd, Render
             if (!isClearMaskVolumeAtlasRequested) { return; }
             isClearMaskVolumeAtlasRequested = false;
 
+            keysInAtlas.Clear();
             maskVolumeAtlas.ResetAllocator();
 
             if (renderGraph != null)
@@ -614,7 +587,12 @@ void PrepareVisibleMaskVolumeListBuffers(HDCamera hdCamera, CommandBuffer immedi
                 {
                     MaskVolumeHandle volume = volumes[maskVolumesIndex];
 
-                    var isVisible = volume.parameters.weight >= 1e-5f && volume.IsDataAssigned();
+                    var isVisible =
+                        volume.parameters.weight >= 1e-5f &&
+#if UNITY_EDITOR
+                        !volume.IsHiddenInScene() &&
+#endif
+                        volume.IsDataAssigned();
 
                     // When hdCamera is null we are preparing for some view-independent baking, so we consider all valid volumes visible.
                     if (isViewDependent && isVisible)
@@ -640,9 +618,27 @@ void PrepareVisibleMaskVolumeListBuffers(HDCamera hdCamera, CommandBuffer immedi
                         var logVolume = CalculateMaskVolumeLogVolume(volume.parameters.size);
                         m_MaskVolumeSortKeys[sortCount++] = PackMaskVolumeSortKey(logVolume, maskVolumesIndex);
                     }
-                    else
+                }
+
+                for (int keyIndex = keysInAtlas.Count - 1; keyIndex >= 0; keyIndex--)
+                {
+                    var key = keysInAtlas[keyIndex];
+                    var dataIsNeededInAtlas = false;
+                    for (int sortIndex = 0; sortIndex < sortCount; sortIndex++)
+                    {
+                        var sortKey = m_MaskVolumeSortKeys[sortIndex];
+                        UnpackMaskVolumeSortKey(sortKey, out var maskVolumesIndex);
+                        var volumeKey = volumes[maskVolumesIndex].ComputeMaskVolumeAtlasKey();
+                        if (volumeKey.Equals(key))
+                        {
+                            dataIsNeededInAtlas = true;
+                            break;
+                        }
+                    }
+                    if (!dataIsNeededInAtlas)
                     {
-                        ReleaseMaskVolumeFromAtlas(volume);
+                        keysInAtlas.RemoveAt(keyIndex);
+                        maskVolumeAtlas.ReleaseTextureSlot(key);
                     }
                 }
 
@@ -688,8 +684,6 @@ void PrepareVisibleMaskVolumeListBuffers(HDCamera hdCamera, CommandBuffer immedi
 
                     MaskVolumeHandle volume = volumes[maskVolumesIndex];
 
-                    EnsureStaleDataIsFlushedFromAtlases(volume);
-
                     if (volumeUploadedToAtlasSHCount < volumeUploadedToAtlasCapacity)
                     {
                         bool volumeWasUploaded = EnsureMaskVolumeInAtlas(immediateCmd, renderGraph, ref maskVolumes.resources, volume);

Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ public OrientedBBox ConstructOBBEngineData(Vector3 cameraOffset)`
`60`	`60`	`}`
`61`	`61`
`62`	`62`	`#if UNITY_EDITOR`
`63`		`- public bool IsHiddesInScene() => m_List.IsHiddenInScene(m_Index);`
	`63`	`+ public bool IsHiddenInScene() => m_List.IsHiddenInScene(m_Index);`
`64`	`64`	`#endif`
`65`	`65`	`}`
`66`	`66`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`	`1`	`using System;`
`2`		`-using System.Collections.Generic;`
`3`	`2`	`using System.Runtime.CompilerServices;`
`4`	`3`	`using static UnityEngine.Rendering.HighDefinition.VolumeGlobalUniqueIDUtils;`
`5`	`4`
`@@ -347,6 +346,8 @@ internal MaskVolumeAtlasKey ComputeMaskVolumeAtlasKey()`
`347`	`346`	`if (maskVolumeAsset == null)`
`348`	`347`	`return MaskVolumeAtlasKey.zero;`
`349`	`348`
	`349`	`+ // Use the payloadID, rather than the probe volume ID to uniquely identify data in the atlas.`
	`350`	`+ // This ensures that if 2 mask volumes exist that point to the same data, that data will only be uploaded once.`
`350`	`351`	`return ComputeMaskVolumeAtlasKey(GetPayloadID(), maskVolumeAsset.resolutionX, maskVolumeAsset.resolutionY, maskVolumeAsset.resolutionZ);`
`351`	`352`	`}`
`352`	`353`
`@@ -361,30 +362,11 @@ internal static MaskVolumeAtlasKey ComputeMaskVolumeAtlasKey(VolumeGlobalUniqueI`
`361`	`362`	`};`
`362`	`363`	`}`
`363`	`364`
`364`		`- private MaskVolumeAtlasKey maskVolumeAtlasKeyPrevious;`
`365`		`-`
`366`		`- internal MaskVolumeAtlasKey GetMaskVolumeAtlasKeyPrevious()`
`367`		`- {`
`368`		`- return maskVolumeAtlasKeyPrevious;`
`369`		`- }`
`370`		`-`
`371`		`- internal void SetMaskVolumeAtlasKeyPrevious(MaskVolumeAtlasKey key)`
`372`		`- {`
`373`		`- maskVolumeAtlasKeyPrevious = key;`
`374`		`- }`
`375`		`-`
`376`	`365`	`private VolumeGlobalUniqueID GetPayloadID()`
`377`	`366`	`{`
`378`	`367`	`return (maskVolumeAsset == null) ? VolumeGlobalUniqueID.zero : maskVolumeAsset.GetID();`
`379`	`368`	`}`
`380`	`369`
`381`		`- internal VolumeGlobalUniqueID GetAtlasID()`
`382`		`- {`
`383`		`- // Use the payloadID, rather than the probe volume ID to uniquely identify data in the atlas.`
`384`		`- // This ensures that if 2 mask volumes exist that point to the same data, that data will only be uploaded once.`
`385`		`- return GetPayloadID();`
`386`		`- }`
`387`		`-`
`388`	`370`	`internal Vector3Int GetResolution()`
`389`	`371`	`{`
`390`	`372`	`return new Vector3Int(maskVolumeAsset.resolutionX, maskVolumeAsset.resolutionY, maskVolumeAsset.resolutionZ);`
`@@ -533,8 +515,6 @@ internal void ResampleAsset()`
`533`	`515`	`}`
`534`	`516`	`}`
`535`	`517`	`}`
`536`		`-`
`537`		`- MaskVolumeManager.manager.ReleaseVolumeFromAtlas(this);`
`538`	`518`	`}`
`539`	`519`
`540`	`520`	`maskVolumeAsset.resolutionX = parameters.resolutionX;`