fix: refresh logic failed to detect matching groups correctly (#320)

Closes: #318
bdunderscore · Aug 10, 2024 · 7bf328f · 7bf328f
1 parent 19ec6c5
commit 7bf328f
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 ### Fixed
+- [#320] Render nodes are not correctly reused across frames
 
 ### Changed
 

diff --git a/Editor/PreviewSystem/IRenderFilter.cs b/Editor/PreviewSystem/IRenderFilter.cs
@@ -1,11 +1,13 @@
 #region
 
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
 using System.Threading.Tasks;
 using JetBrains.Annotations;
+using NUnit.Framework;
 using UnityEngine;
 
 #endregion
@@ -52,7 +54,7 @@ public T GetData<T>()
 
         protected bool Equals(RenderGroup other)
         {
-            return Equals(Renderers, other.Renderers);
+            return Renderers.SequenceEqual(other.Renderers);
         }
 
         public override bool Equals(object obj)
@@ -65,7 +67,13 @@ public override bool Equals(object obj)
 
         public override int GetHashCode()
         {
-            return (Renderers != null ? Renderers.GetHashCode() : 0);
+            var hashCode = 0;
+            foreach (var renderer in Renderers)
+            {
+                hashCode = HashCode.Combine(hashCode, renderer.GetInstanceID());
+            }
+
+            return hashCode;
         }
     }
 
@@ -80,6 +88,13 @@ internal RenderGroup(ImmutableList<Renderer> renderers, T context) : base(render
 
         private bool Equals(RenderGroup<T> other)
         {
+            if (Context is IEnumerable l && other.Context is IEnumerable ol)
+            {
+                // This is a common mistake; List does not implement a useful Equals for us. Work around it
+                // on behalf of our consumers...
+                return base.Equals(other) && l.Cast<object>().SequenceEqual(ol.Cast<object>());
+            }
+
             return base.Equals(other) && EqualityComparer<T>.Default.Equals(Context, other.Context);
         }
 
@@ -90,10 +105,12 @@ public override bool Equals(object obj)
 
         public override int GetHashCode()
         {
-            unchecked
+            if (Context is IEnumerable l)
             {
-                return (base.GetHashCode() * 397) ^ EqualityComparer<T>.Default.GetHashCode(Context);
+                return l.Cast<object>().Aggregate(base.GetHashCode(), (acc, o) => HashCode.Combine(acc, o.GetHashCode()));
             }
+
+            return HashCode.Combine(base.GetHashCode(), EqualityComparer<T>.Default.GetHashCode(Context));
         }
     }
 

diff --git a/Editor/PreviewSystem/Rendering/ProxyPipeline.cs b/Editor/PreviewSystem/Rendering/ProxyPipeline.cs
@@ -8,6 +8,7 @@
 using System.Threading.Tasks;
 using UnityEditor;
 using UnityEngine;
+using UnityEngine.Profiling;
 using Debug = System.Diagnostics.Debug;
 
 #endregion
@@ -104,12 +105,16 @@ public ProxyPipeline(ProxyObjectCache proxyCache, IEnumerable<IRenderFilter> fil
         private async Task Build(ProxyObjectCache proxyCache, IEnumerable<IRenderFilter> filters,
             ProxyPipeline priorPipeline)
         {
+            Profiler.BeginSample("ProxyPipeline.Build.Synchronous");
             var context = new ComputeContext();
             _ctx = context; // prevent GC
 
             var filterList = filters.Where(f => f.IsEnabled(context)).ToList();
 
             Dictionary<Renderer, Task<NodeController>> nodeTasks = new();
+            int total_nodes = 0;
+            int reused = 0;
+            int refresh_failed = 0;
 
             for (int i = 0; i < filterList.Count(); i++)
             {
@@ -126,8 +131,11 @@ private async Task Build(ProxyObjectCache proxyCache, IEnumerable<IRenderFilter>
                 }
 
                 int groupIndex = -1;
-                foreach (var group in stage.Originals)
+                foreach (var group in stage.Originals.OrderBy(g => g.GetHashCode()))
                 {
+                    total_nodes++;
+
+
                     groupIndex++;
                     var resolved = group.Renderers.Select(r =>
                     {
@@ -159,8 +167,10 @@ private async Task Build(ProxyObjectCache proxyCache, IEnumerable<IRenderFilter>
                     });
 
                     var priorNode = prior?.NodeTasks.ElementAtOrDefault(groupIndex);
-                    if (priorNode?.IsCompletedSuccessfully != true || !Equals(priorNode?.Result.Group, group))
+                    var sameGroup = Equals(priorNode?.Result.Group, group);
+                    if (priorNode?.IsCompletedSuccessfully != true || !sameGroup)
                     {
+                        //System.Diagnostics.Debug.WriteLine("Failed to reuse node: priorNode != null: " + (priorNode != null) + ", sameGroup: " + sameGroup);
                         priorNode = null;
                     }
 
@@ -174,7 +184,13 @@ private async Task Build(ProxyObjectCache proxyCache, IEnumerable<IRenderFilter>
                                     .Aggregate((a, b) => a | b);
 
                                 var node = await priorNode.Result.Refresh(proxies, changeFlags);
-                                if (node != null) return node;
+                                if (node != null)
+                                {
+                                    reused++;
+                                    return node;
+                                }
+
+                                refresh_failed++;
                             }
 
                             return await NodeController.Create(filter, group, items.Result.ToList());
@@ -189,13 +205,17 @@ private async Task Build(ProxyObjectCache proxyCache, IEnumerable<IRenderFilter>
                     }
                 }
             }
+
+            Profiler.EndSample();
 
             await Task.WhenAll(_stages.SelectMany(s => s.NodeTasks))
                 .ContinueWith(result =>
                 {
                     _completedBuild.TrySetResult(null);
                     EditorApplication.delayCall += () => { EditorApplication.delayCall += SceneView.RepaintAll; };
                 });
+
+            //Debug.WriteLine($"Total nodes: {total_nodes}, reused: {reused}, refresh failed: {refresh_failed}");
 
             foreach (var stage in _stages)
             {