From c9de8c42b0f7c0e60c10cf05fae5824d0c1a7b2e Mon Sep 17 00:00:00 2001 From: bd_ Date: Mon, 17 Jun 2024 07:57:37 +0900 Subject: [PATCH] Fixing some issues with preview system invalidations not working (#265) * 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. --- Editor/ChangeStream/ChangeNotifier.cs | 60 +++++++++++++++++++ Editor/ChangeStream/ChangeNotifier.cs.meta | 3 + Editor/ChangeStream/ShadowGameObject.cs | 9 ++- .../PreviewSystem/Rendering/NodeController.cs | 15 ++++- .../PreviewSystem/Rendering/ProxyPipeline.cs | 17 +++++- 5 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 Editor/ChangeStream/ChangeNotifier.cs create mode 100644 Editor/ChangeStream/ChangeNotifier.cs.meta diff --git a/Editor/ChangeStream/ChangeNotifier.cs b/Editor/ChangeStream/ChangeNotifier.cs new file mode 100644 index 00000000..c58274cb --- /dev/null +++ b/Editor/ChangeStream/ChangeNotifier.cs @@ -0,0 +1,60 @@ +#region + +using JetBrains.Annotations; +using UnityEditor; +using UnityEngine; + +#endregion + +namespace nadena.dev.ndmf.rq.unity.editor +{ + public static class ChangeNotifier + { + /// + /// Notifies the reactive query and NDMF preview system of a change in an object that isn't tracked by the normal + /// unity ObjectchangeEventStream system. + /// + /// + public static void NotifyObjectUpdate(Object obj) + { + if (obj != null) ObjectWatcher.Instance.Hierarchy.InvalidateTree(obj.GetInstanceID()); + } + + /// + /// Notifies the reactive query and NDMF preview system of a change in an object that isn't tracked by the normal + /// unity ObjectchangeEventStream system. + /// + /// + 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); + } + } + } + } + } +} \ No newline at end of file diff --git a/Editor/ChangeStream/ChangeNotifier.cs.meta b/Editor/ChangeStream/ChangeNotifier.cs.meta new file mode 100644 index 00000000..beda9f8b --- /dev/null +++ b/Editor/ChangeStream/ChangeNotifier.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: cb5e5f4868a3451c897a5a979e6ac0b5 +timeCreated: 1718249390 \ No newline at end of file diff --git a/Editor/ChangeStream/ShadowGameObject.cs b/Editor/ChangeStream/ShadowGameObject.cs index 88fb6aee..2d28408e 100644 --- a/Editor/ChangeStream/ShadowGameObject.cs +++ b/Editor/ChangeStream/ShadowGameObject.cs @@ -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; @@ -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) diff --git a/Editor/PreviewSystem/Rendering/NodeController.cs b/Editor/PreviewSystem/Rendering/NodeController.cs index 65305212..5f12e466 100644 --- a/Editor/PreviewSystem/Rendering/NodeController.cs +++ b/Editor/PreviewSystem/Rendering/NodeController.cs @@ -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) { @@ -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(); } @@ -68,7 +77,7 @@ public static async Task Create( context ); - return new NodeController(node, proxies, new RefCount()); + return new NodeController(node, proxies, new RefCount(), invalidater.Task, context); } public async Task Refresh( @@ -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; } diff --git a/Editor/PreviewSystem/Rendering/ProxyPipeline.cs b/Editor/PreviewSystem/Rendering/ProxyPipeline.cs index 274e2134..dcbed018 100644 --- a/Editor/PreviewSystem/Rendering/ProxyPipeline.cs +++ b/Editor/PreviewSystem/Rendering/ProxyPipeline.cs @@ -1,5 +1,6 @@ #region +using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; @@ -66,15 +67,22 @@ internal class ProxyPipeline internal ImmutableDictionary ProxyToOriginalObject = ImmutableDictionary.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; @@ -83,6 +91,8 @@ internal void Invalidate() public ProxyPipeline(IEnumerable filters, ProxyPipeline priorPipeline = null) { + InvalidateAction = Invalidate; + using (new SyncContextScope(ReactiveQueryScheduler.SynchronizationContext)) { _buildTask = Task.Factory.StartNew( @@ -98,8 +108,9 @@ public ProxyPipeline(IEnumerable filters, ProxyPipeline priorPipe private async Task Build(IEnumerable filters, ProxyPipeline priorPipeline) { var context = new ComputeContext(() => "ProxyPipeline construction"); - context.Invalidate = Invalidate; + context.Invalidate = InvalidateAction; context.OnInvalidate = _invalidater.Task; + _ctx = context; // prevent GC List filterList = filters.ToList(); List priorStages = priorPipeline?._stages; @@ -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()); } } }