Skip to content

Commit

Permalink
fix: Z-fighting in prefab isolation view (#327)
Browse files Browse the repository at this point in the history
* chore: add a bunch of debugging to the preview system

* fix: Z-fighting in prefab isolation view

Closes: #325
  • Loading branch information
bdunderscore authored Aug 14, 2024
1 parent af1647d commit 389e6b9
Show file tree
Hide file tree
Showing 10 changed files with 188 additions and 32 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- [#320] Render nodes are not correctly reused across frames
- [#321] Fix GetTargetGroup being called on every pipeline invalidation
- [#327] Z-fighting occurs in prefab isolation view

### Changed

Expand Down
28 changes: 28 additions & 0 deletions Editor/ChangeStream/ListenerSet.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#region

using System;
using System.Collections.Generic;
using nadena.dev.ndmf.preview;
using UnityEditor;

Expand All @@ -25,6 +26,13 @@ ComputeContext ctx
_ctx = ctx == null ? null : new WeakReference<ComputeContext>(ctx);
}

public override string ToString()
{
if (_ctx.TryGetTarget(out var target))
return $"Listener for {target}";
return "Listener (GC'd)";
}

public void Dispose()
{
if (_next != null)
Expand All @@ -41,6 +49,9 @@ internal void MaybePrune()
{
if (!_ctx.TryGetTarget(out var ctx) || ctx.IsInvalidated)
{
#if NDMF_DEBUG
System.Diagnostics.Debug.WriteLine($"{this} is invalid, disposing");
#endif
Dispose();
}
}
Expand All @@ -50,10 +61,17 @@ internal void MaybeFire(T info)
{
if (!_ctx.TryGetTarget(out var ctx) || ctx.IsInvalidated)
{
#if NDMF_DEBUG
System.Diagnostics.Debug.WriteLine($"{this} is invalid, disposing");
#endif
Dispose();
}
else if (_filter(info))
{
#if NDMF_DEBUG
System.Diagnostics.Debug.WriteLine($"{this} is firing");
#endif

ctx.Invalidate();
// We need to wait two frames before repainting: One to process task callbacks, then one to actually
// repaint (and update previews).
Expand Down Expand Up @@ -116,5 +134,15 @@ public void Prune()
listener = next;
}
}

internal IEnumerable<string> GetListeners()
{
var ptr = _head._next;
while (ptr != _head)
{
yield return ptr.ToString();
ptr = ptr._next;
}
}
}
}
96 changes: 96 additions & 0 deletions Editor/ChangeStream/ShadowGameObject.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#region

#if NDMF_DEBUG
using System.Text;
#endif
using System;
using System.Collections.Generic;
using System.Threading;
Expand Down Expand Up @@ -56,6 +59,91 @@ internal class ShadowHierarchy

int lastPruned = Int32.MinValue;

#if NDMF_DEBUG
[MenuItem("Tools/NDM Framework/Debug: Dump shadow hierarchy")]
static void StaticDumpShadowHierarchy()
{
ObjectWatcher.Instance.Hierarchy.DumpShadowHierarchy();
}

void DumpShadowHierarchy()
{
int indent = 0;
StringBuilder sb = new StringBuilder();

sb.AppendLine("[Shadow Hierarchy Dump]");
sb.AppendLine("Root set listeners:");
indent += 2;
DumpListenerSet(_rootSetListener);
sb.AppendLine();
indent -= 2;

sb.AppendLine("GameObjects:");
foreach (var obj in _gameObjects.Values)
{
if (obj.Parent == null)
{
DumpShadowGameObject(obj);
}
}

sb.AppendLine("Other objects:");
foreach (var obj in _otherObjects.Values)
{
DumpShadowObject(obj);
}

sb.AppendLine("[End Shadow Hierarchy Dump]");

Debug.Log(sb.ToString());


void DumpShadowObject(ShadowObject obj)
{
sb.Append(' ', indent);
sb.Append("+ ");
if (obj.Object == null)
{
sb.AppendLine("<" + obj.InstanceID + ">");
}
else if (obj.Object is Component c)
{
sb.AppendLine(c.gameObject.name + " (" + c.GetType().Name + ")");
}
else
{
sb.AppendLine(obj.Object.name);
}
}

void DumpShadowGameObject(ShadowGameObject obj)
{
sb.Append(' ', indent);
sb.Append("+ ");
sb.AppendLine(obj.GameObject == null ? "<" + obj.InstanceID + ">" : obj.GameObject.name);
indent += 2;
DumpListenerSet(obj._listeners);
sb.AppendLine();
sb.Append(' ', indent);
sb.AppendLine("Children:");
foreach (var child in obj.Children)
{
DumpShadowGameObject(child);
}
indent -= 2;
}

void DumpListenerSet<T>(ListenerSet<T> set)
{
foreach (var listener in set.GetListeners())
{
sb.Append(' ', indent);
sb.AppendLine(listener.ToString());
}
}
}
#endif

internal IDisposable RegisterRootSetListener(ListenerSet<HierarchyEvent>.Filter filter, ComputeContext ctx)
{
if (ctx.IsInvalidated) return new NullDisposable();
Expand Down Expand Up @@ -290,6 +378,10 @@ internal void FireDestroyNotification(int instanceId)

void ForceInvalidateHierarchy(ShadowGameObject obj)
{
#if NDMF_DEBUG
Debug.Log("=== Force invalidate: " + (obj.GameObject?.name ?? "<null>"));
#endif

obj._listeners.Fire(HierarchyEvent.ForceInvalidate);
_gameObjects.Remove(obj.InstanceID);

Expand Down Expand Up @@ -322,6 +414,10 @@ internal void FireStructureChangeEvent(int instanceId)

internal void InvalidateAll()
{
#if NDMF_DEBUG
Debug.Log("=== Invalidate all ===");
#endif

var oldDict = _gameObjects;
_gameObjects = new Dictionary<int, ShadowGameObject>();

Expand Down
15 changes: 12 additions & 3 deletions Editor/PreviewSystem/ComputeContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ public sealed class ComputeContext

static ComputeContext()
{
NullContext = new ComputeContext(null);
NullContext = new ComputeContext("null", null);
}

internal string Description { get; }

/// <summary>
/// An Action which can be used to invalidate this compute context (possibly triggering a recompute).
/// </summary>
Expand All @@ -35,16 +37,18 @@ static ComputeContext()

public bool IsInvalidated => OnInvalidate.IsCompleted;

internal ComputeContext()
internal ComputeContext(string description)
{
Invalidate = () => _invalidater.TrySetResult(null);
OnInvalidate = _invalidater.Task;
Description = description;
}

private ComputeContext(object nullToken)
private ComputeContext(string description, object nullToken)
{
Invalidate = () => { };
OnInvalidate = Task.CompletedTask;
Description = description;
}

/// <summary>
Expand All @@ -55,5 +59,10 @@ internal void Invalidates(ComputeContext other)
{
OnInvalidate.ContinueWith(_ => other.Invalidate());
}

public override string ToString()
{
return "<context: " + Description + ">";
}
}
}
3 changes: 3 additions & 0 deletions Editor/PreviewSystem/ProxyManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ private static void OnPreCull(Camera cam)

if (EditorApplication.isPlayingOrWillChangePlaymode) return;

// TODO: fully support prefab isolation view
if (PrefabStageUtility.GetCurrentPrefabStage() != null) return;

var sess = PreviewSession.Current;
if (sess == null) return;

Expand Down
9 changes: 6 additions & 3 deletions Editor/PreviewSystem/Rendering/NodeController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,11 @@ public static async Task<NodeController> Create(
IRenderFilter filter,
RenderGroup group,
ObjectRegistry registry,
List<(Renderer, ProxyObjectController, ObjectRegistry)> proxies)
List<(Renderer, ProxyObjectController, ObjectRegistry)> proxies
)
{
ComputeContext context = new ComputeContext();
var context =
new ComputeContext("NodeController for " + filter + " on " + group.Renderers[0].gameObject.name);

IRenderFilterNode node;
using (var scope = new ObjectRegistryScope(registry))
Expand All @@ -104,7 +106,8 @@ RenderAspects changes
)
{
var registry = ObjectRegistry.Merge(null, proxies.Select(p => p.Item3));
ComputeContext context = new ComputeContext();
var context = new ComputeContext("NodeController (refresh) for " + _filter + " on " +
_group.Renderers[0].gameObject.name);

IRenderFilterNode node;

Expand Down
7 changes: 4 additions & 3 deletions Editor/PreviewSystem/Rendering/ProxyObjectController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,10 @@ public ProxyObjectController(ProxyObjectCache cache, Renderer originalRenderer,

private void SetupRendererMonitoring(Renderer r)
{
_monitorRenderer = new ComputeContext();
_monitorMaterials = new ComputeContext();
_monitorMesh = new ComputeContext();
var gameObjectName = r.gameObject.name;
_monitorRenderer = new ComputeContext("Renderer Monitor for " + gameObjectName);
_monitorMaterials = new ComputeContext("Material Monitor for " + gameObjectName);
_monitorMesh = new ComputeContext("Mesh Monitor for " + gameObjectName);

_monitorRenderer.Observe(r);
if (r is SkinnedMeshRenderer smr)
Expand Down
23 changes: 19 additions & 4 deletions Editor/PreviewSystem/Rendering/ProxyPipeline.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public StageDescriptor(IRenderFilter filter, ComputeContext parentContext)
{
Filter = filter;

Context = new ComputeContext();
Context = new ComputeContext("StageDescriptor for " + Filter + " on " + parentContext);
Context.Invalidates(parentContext);

var unsorted = filter.GetTargetGroups(Context);
Expand All @@ -51,7 +51,7 @@ public StageDescriptor(IRenderFilter filter, ComputeContext parentContext)
public StageDescriptor(StageDescriptor prior, ComputeContext parentContext)
{
Filter = prior.Filter;
Context = new ComputeContext();
Context = new ComputeContext("StageDescriptor for " + Filter + " on " + parentContext);
Context.Invalidates(parentContext);

Originals = prior.Originals;
Expand Down Expand Up @@ -84,9 +84,11 @@ internal class ProxyPipeline
internal ImmutableDictionary<GameObject, GameObject> ProxyToOriginalObject =
ImmutableDictionary<GameObject, GameObject>.Empty;

private readonly long _generation;

// ReSharper disable once NotAccessedField.Local
// needed to prevent GC of the ComputeContext
private ComputeContext _ctx = new();
private ComputeContext _ctx;
internal bool IsInvalidated => _ctx.OnInvalidate.IsCompleted;

internal void Invalidate()
Expand All @@ -104,6 +106,7 @@ internal void Invalidate()

public ProxyPipeline(ProxyObjectCache proxyCache, IEnumerable<IRenderFilter> filters, ProxyPipeline priorPipeline = null)
{
_generation = (priorPipeline?._generation ?? 0) + 1;
InvalidateAction = Invalidate;

_buildTask = Task.Factory.StartNew(
Expand All @@ -119,9 +122,13 @@ private async Task Build(ProxyObjectCache proxyCache, IEnumerable<IRenderFilter>
ProxyPipeline priorPipeline)
{
Profiler.BeginSample("ProxyPipeline.Build.Synchronous");
var context = new ComputeContext();
var context = new ComputeContext($"ProxyPipeline {_generation}");
_ctx = context; // prevent GC

#if NDMF_DEBUG
System.Diagnostics.Debug.WriteLine($"Building pipeline {_generation}");
#endif

var filterList = filters.Where(f => f.IsEnabled(context)).ToList();

Dictionary<Renderer, Task<NodeController>> nodeTasks = new();
Expand Down Expand Up @@ -205,6 +212,10 @@ private async Task Build(ProxyObjectCache proxyCache, IEnumerable<IRenderFilter>
{
var proxies = items.Result.ToList();

#if NDMF_DEBUG
System.Diagnostics.Debug.WriteLine($"Creating node for {filter} on {group.Renderers[0].gameObject.name} for generation {_generation}");
#endif

if (priorNode != null)
{
RenderAspects changeFlags = proxies.Select(p => p.Item2.ChangeFlags)
Expand Down Expand Up @@ -244,6 +255,10 @@ await Task.WhenAll(_stages.SelectMany(s => s.NodeTasks))

//Debug.WriteLine($"Total nodes: {total_nodes}, reused: {reused}, refresh failed: {refresh_failed}");

#if NDMF_DEBUG
System.Diagnostics.Debug.WriteLine($"Pipeline {_generation} is ready");
#endif

foreach (var stage in _stages)
{
foreach (var node in stage.NodeTasks)
Expand Down
6 changes: 3 additions & 3 deletions UnitTests~/RQ-EditorTests/PublishedValueTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public class PublishedValueTest
public void BasicObserve()
{
PublishedValue<string> val = new PublishedValue<string>("foo");
ComputeContext ctx = new ComputeContext();
ComputeContext ctx = new ComputeContext("");

string observed = ctx.Observe(val);
Assert.AreEqual("foo", observed);
Expand All @@ -25,7 +25,7 @@ public void BasicObserve()
public void ObserveWithExtract()
{
PublishedValue<string> val = new PublishedValue<string>("foo");
ComputeContext ctx = new ComputeContext();
ComputeContext ctx = new ComputeContext("");

int len = ctx.Observe(val, s => s.Length);
Assert.AreEqual(3, len);
Expand All @@ -43,7 +43,7 @@ public void ObserveWithExtract()
public void ObserveWithExtractAndEquals()
{
PublishedValue<string> val = new PublishedValue<string>("foo");
ComputeContext ctx = new ComputeContext();
ComputeContext ctx = new ComputeContext("");

string observed = ctx.Observe(val, a => a, (a, b) => a.ToLowerInvariant().Equals(b.ToLowerInvariant()));
Assert.AreEqual("foo", observed);
Expand Down
Loading

0 comments on commit 389e6b9

Please sign in to comment.