Skip to content

Commit

Permalink
fix: refresh logic failed to detect matching groups correctly
Browse files Browse the repository at this point in the history
Closes: #318
  • Loading branch information
bdunderscore committed Aug 10, 2024
1 parent 19ec6c5 commit 2ef982e
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 7 deletions.
25 changes: 21 additions & 4 deletions Editor/PreviewSystem/IRenderFilter.cs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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;
}
}

Expand All @@ -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);
}

Expand All @@ -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));
}
}

Expand Down
26 changes: 23 additions & 3 deletions Editor/PreviewSystem/Rendering/ProxyPipeline.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Threading.Tasks;
using UnityEditor;
using UnityEngine;
using UnityEngine.Profiling;
using Debug = System.Diagnostics.Debug;

#endregion
Expand Down Expand Up @@ -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++)
{
Expand All @@ -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 =>
{
Expand Down Expand Up @@ -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;
}

Expand All @@ -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());
Expand All @@ -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)
{
Expand Down

0 comments on commit 2ef982e

Please sign in to comment.