From 2ef982ef275be924c2d8cb1a070d772e807e0b2c Mon Sep 17 00:00:00 2001 From: bd_ Date: Fri, 9 Aug 2024 18:08:00 -0700 Subject: [PATCH] fix: refresh logic failed to detect matching groups correctly Closes: #318 --- Editor/PreviewSystem/IRenderFilter.cs | 25 +++++++++++++++--- .../PreviewSystem/Rendering/ProxyPipeline.cs | 26 ++++++++++++++++--- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/Editor/PreviewSystem/IRenderFilter.cs b/Editor/PreviewSystem/IRenderFilter.cs index 8d144a2..1894c7e 100644 --- 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() 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 renderers, T context) : base(render private bool Equals(RenderGroup 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().SequenceEqual(ol.Cast()); + } + return base.Equals(other) && EqualityComparer.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.Default.GetHashCode(Context); + return l.Cast().Aggregate(base.GetHashCode(), (acc, o) => HashCode.Combine(acc, o.GetHashCode())); } + + return HashCode.Combine(base.GetHashCode(), EqualityComparer.Default.GetHashCode(Context)); } } diff --git a/Editor/PreviewSystem/Rendering/ProxyPipeline.cs b/Editor/PreviewSystem/Rendering/ProxyPipeline.cs index a1c174e..de1a28a 100644 --- 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 fil private async Task Build(ProxyObjectCache proxyCache, IEnumerable 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> 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 } 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 }); 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 .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,6 +205,8 @@ private async Task Build(ProxyObjectCache proxyCache, IEnumerable } } } + + Profiler.EndSample(); await Task.WhenAll(_stages.SelectMany(s => s.NodeTasks)) .ContinueWith(result => @@ -196,6 +214,8 @@ await Task.WhenAll(_stages.SelectMany(s => s.NodeTasks)) _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) {