Skip to content

Commit

Permalink
Fixing some issues with preview system invalidations not working (#265)
Browse files Browse the repository at this point in the history
* feat: add support for explicit change notification

* fix: issues with preview system invalidations not chaining properly

Note that the GC fixes are a temporary hack - this API is too
error-prone and needs revisiting.
  • Loading branch information
bdunderscore authored Jun 16, 2024
1 parent 4192eca commit c9de8c4
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 6 deletions.
60 changes: 60 additions & 0 deletions Editor/ChangeStream/ChangeNotifier.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#region

using JetBrains.Annotations;
using UnityEditor;
using UnityEngine;

#endregion

namespace nadena.dev.ndmf.rq.unity.editor
{
public static class ChangeNotifier
{
/// <summary>
/// Notifies the reactive query and NDMF preview system of a change in an object that isn't tracked by the normal
/// unity ObjectchangeEventStream system.
/// </summary>
/// <param name="obj"></param>
public static void NotifyObjectUpdate(Object obj)
{
if (obj != null) ObjectWatcher.Instance.Hierarchy.InvalidateTree(obj.GetInstanceID());
}

/// <summary>
/// Notifies the reactive query and NDMF preview system of a change in an object that isn't tracked by the normal
/// unity ObjectchangeEventStream system.
/// </summary>
/// <param name="obj"></param>
public static void NotifyObjectUpdate(int instanceId)
{
ObjectWatcher.Instance.Hierarchy.InvalidateTree(instanceId);
}

private class Processor : AssetPostprocessor
{
[UsedImplicitly]
static void OnPostprocessAllAssets(
// ReSharper disable once Unity.IncorrectMethodSignature
string[] importedAssets,
// ReSharper disable once UnusedParameter.Local
string[] deletedAssets,
// ReSharper disable once UnusedParameter.Local
string[] movedAssets,
// ReSharper disable once UnusedParameter.Local
string[] movedFromAssetPaths,
// ReSharper disable once UnusedParameter.Local
bool didDomainReload
)
{
foreach (var asset in importedAssets)
{
var subassets = AssetDatabase.LoadAllAssetsAtPath(asset);
foreach (var subasset in subassets)
{
NotifyObjectUpdate(subasset);
}
}
}
}
}
}
3 changes: 3 additions & 0 deletions Editor/ChangeStream/ChangeNotifier.cs.meta

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion Editor/ChangeStream/ShadowGameObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,9 @@ public void InvalidateTree(int instanceId)
{
if (_gameObjects.TryGetValue(instanceId, out var shadow))
{
_gameObjects.Remove(instanceId);
shadow._listeners.Fire(HierarchyEvent.ForceInvalidate);
FireParentComponentChangeNotifications(shadow.Parent);
_gameObjects.Remove(instanceId);

var parentGameObject = shadow.Parent?.GameObject;

Expand All @@ -367,6 +367,13 @@ public void InvalidateTree(int instanceId)
ActivateShadowObject(gameObject);
}
}

if (_otherObjects.TryGetValue(instanceId, out var otherObj))
{
Debug.Log("Invalidate OtherObject (" + instanceId + " " + otherObj.Object.GetType() + ")");
_otherObjects.Remove(instanceId);
otherObj._listeners.Fire(HierarchyEvent.ForceInvalidate);
}
}

public void FireGameObjectCreate(int instanceId)
Expand Down
15 changes: 12 additions & 3 deletions Editor/PreviewSystem/Rendering/NodeController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ private class RefCount
private readonly IRenderFilterNode _node;
private readonly List<(Renderer, ProxyObjectController)> _proxies;
private readonly RefCount _refCount;

// ReSharper disable once NotAccessedField.Local
private readonly ComputeContext _context; // prevents GC
internal ulong WhatChanged = IRenderFilterNode.Everything;
internal Task OnInvalidate;

internal ProxyObjectController GetProxyFor(Renderer r)
{
Expand All @@ -32,12 +36,17 @@ internal ProxyObjectController GetProxyFor(Renderer r)
private NodeController(
IRenderFilterNode node,
List<(Renderer, ProxyObjectController)> proxies,
RefCount refCount
RefCount refCount,
Task invalidated,
ComputeContext context
)
{
_node = node;
_proxies = proxies;
_refCount = refCount;
_context = context;

OnInvalidate = invalidated;

OnFrame();
}
Expand Down Expand Up @@ -68,7 +77,7 @@ public static async Task<NodeController> Create(
context
);

return new NodeController(node, proxies, new RefCount());
return new NodeController(node, proxies, new RefCount(), invalidater.Task, context);
}

public async Task<NodeController> Refresh(
Expand Down Expand Up @@ -103,7 +112,7 @@ ulong changes
refCount = new RefCount();
}

var controller = new NodeController(node, proxies, refCount);
var controller = new NodeController(node, proxies, refCount, invalidater.Task, context);
controller.WhatChanged = changes | node.WhatChanged;
return controller;
}
Expand Down
17 changes: 15 additions & 2 deletions Editor/PreviewSystem/Rendering/ProxyPipeline.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#region

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
Expand Down Expand Up @@ -66,15 +67,22 @@ internal class ProxyPipeline

internal ImmutableDictionary<GameObject, GameObject> ProxyToOriginalObject =
ImmutableDictionary<GameObject, GameObject>.Empty;

// ReSharper disable once NotAccessedField.Local
// needed to prevent GC of the ComputeContext
private ComputeContext _ctx;

internal void Invalidate()
{
Debug.Log("=== pipeline invalidate ===");
using (new SyncContextScope(ReactiveQueryScheduler.SynchronizationContext))
{
_invalidater.TrySetResult(null);
}
}

private readonly Action InvalidateAction;

public bool IsReady => _buildTask.IsCompletedSuccessfully;
public bool IsFailed => _buildTask.IsFaulted;

Expand All @@ -83,6 +91,8 @@ internal void Invalidate()

public ProxyPipeline(IEnumerable<IRenderFilter> filters, ProxyPipeline priorPipeline = null)
{
InvalidateAction = Invalidate;

using (new SyncContextScope(ReactiveQueryScheduler.SynchronizationContext))
{
_buildTask = Task.Factory.StartNew(
Expand All @@ -98,8 +108,9 @@ public ProxyPipeline(IEnumerable<IRenderFilter> filters, ProxyPipeline priorPipe
private async Task Build(IEnumerable<IRenderFilter> filters, ProxyPipeline priorPipeline)
{
var context = new ComputeContext(() => "ProxyPipeline construction");
context.Invalidate = Invalidate;
context.Invalidate = InvalidateAction;
context.OnInvalidate = _invalidater.Task;
_ctx = context; // prevent GC

List<IRenderFilter> filterList = filters.ToList();
List<StageDescriptor> priorStages = priorPipeline?._stages;
Expand Down Expand Up @@ -169,7 +180,9 @@ await Task.WhenAll(_stages.SelectMany(s => s.NodeTasks))
{
foreach (var node in stage.NodeTasks)
{
_nodes.Add(await node);
var resolvedNode = await node;
_nodes.Add(resolvedNode);
_ = resolvedNode.OnInvalidate.ContinueWith(_ => Invalidate());
}
}
}
Expand Down

0 comments on commit c9de8c4

Please sign in to comment.