From 65470fe19f464074654671cabcb7f7108f6c2f1f Mon Sep 17 00:00:00 2001 From: bd_ Date: Sun, 22 Sep 2024 14:21:39 -0700 Subject: [PATCH] fix: exceptions sometimes thrown when accessing R/O meshes from preview callbacks Fixes: #409 --- CHANGELOG.md | 3 + Editor/ChangeStream/PropertyMonitor.cs | 16 +- .../PreviewSystem/Rendering/ProxyPipeline.cs | 19 +- Editor/PreviewSystem/Task.meta | 3 + Editor/PreviewSystem/Task/NDMFSyncContext.cs | 171 ++++++++++++++++++ .../Task/NDMFSyncContext.cs.meta | 3 + Editor/PreviewSystem/TaskThrottle.cs | 17 ++ 7 files changed, 218 insertions(+), 14 deletions(-) create mode 100644 Editor/PreviewSystem/Task.meta create mode 100644 Editor/PreviewSystem/Task/NDMFSyncContext.cs create mode 100644 Editor/PreviewSystem/Task/NDMFSyncContext.cs.meta diff --git a/CHANGELOG.md b/CHANGELOG.md index 5433d1b..716944e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- [#410] Added `NDMFSyncContext` API + ### Fixed - [#408] Improved performance of `BuildContext.Serialize` +- [#410] Sometimes R/O meshes cannot be accessed from preview context ### Changed - [#408] Unserialized assets will be serialized after the Transforming phase completes (before e.g. VRCFury runs) diff --git a/Editor/ChangeStream/PropertyMonitor.cs b/Editor/ChangeStream/PropertyMonitor.cs index e3753bb..0303cc1 100644 --- a/Editor/ChangeStream/PropertyMonitor.cs +++ b/Editor/ChangeStream/PropertyMonitor.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using nadena.dev.ndmf.preview; using UnityEditor; using UnityEngine; using UnityEngine.Profiling; @@ -19,12 +20,15 @@ internal class PropertyMonitor internal void MaybeStartRefreshTimer() { - _activeRefreshTask = Task.Factory.StartNew( - CheckAllObjectsLoop, - CancellationToken.None, - TaskCreationOptions.None, - TaskScheduler.FromCurrentSynchronizationContext() - ); + using (var scope = NDMFSyncContext.Scope()) + { + _activeRefreshTask = Task.Factory.StartNew( + CheckAllObjectsLoop, + CancellationToken.None, + TaskCreationOptions.None, + TaskScheduler.FromCurrentSynchronizationContext() + ); + } } public enum PropertyMonitorEvent diff --git a/Editor/PreviewSystem/Rendering/ProxyPipeline.cs b/Editor/PreviewSystem/Rendering/ProxyPipeline.cs index 5fdc998..bea57d1 100644 --- a/Editor/PreviewSystem/Rendering/ProxyPipeline.cs +++ b/Editor/PreviewSystem/Rendering/ProxyPipeline.cs @@ -94,14 +94,17 @@ public ProxyPipeline(ProxyObjectCache proxyCache, IEnumerable fil { _generation = (priorPipeline?._generation ?? 0) + 1; InvalidateAction = Invalidate; - - _buildTask = Task.Factory.StartNew( - _ => Build(proxyCache, filters, priorPipeline), - null, - CancellationToken.None, - 0, - TaskScheduler.FromCurrentSynchronizationContext() - ).Unwrap(); + + using (var scope = NDMFSyncContext.Scope()) + { + _buildTask = Task.Factory.StartNew( + _ => Build(proxyCache, filters, priorPipeline), + null, + CancellationToken.None, + 0, + TaskScheduler.FromCurrentSynchronizationContext() + ).Unwrap(); + } } private async Task Build(ProxyObjectCache proxyCache, IEnumerable filters, diff --git a/Editor/PreviewSystem/Task.meta b/Editor/PreviewSystem/Task.meta new file mode 100644 index 0000000..74a89a3 --- /dev/null +++ b/Editor/PreviewSystem/Task.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: d23259d492e24d5386daaf2808b52ea5 +timeCreated: 1727038374 \ No newline at end of file diff --git a/Editor/PreviewSystem/Task/NDMFSyncContext.cs b/Editor/PreviewSystem/Task/NDMFSyncContext.cs new file mode 100644 index 0000000..b8755e2 --- /dev/null +++ b/Editor/PreviewSystem/Task/NDMFSyncContext.cs @@ -0,0 +1,171 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using UnityEditor; +using UnityEngine; + +namespace nadena.dev.ndmf.preview +{ + /// + /// The default unity synchronization context runs in the context of the Player Loop, which blocks access to R/O + /// mesh data. As such, NDMF provides a synchronization context which runs in the context of + /// `EditorApplication.delayCall` instead. + /// + public static class NDMFSyncContext + { + public static SynchronizationContext Context = new Impl(); + + /// + /// Switches to the NDMF synchronization context, and returns an IDisposable which will restore the prior + /// synchronization context. + /// + /// + public static IDisposable Scope() + { + return new SyncContextScope(); + } + + private class SyncContextScope : IDisposable + { + private readonly SynchronizationContext _prior = SynchronizationContext.Current; + + public SyncContextScope() + { + SynchronizationContext.SetSynchronizationContext(Context); + } + + public void Dispose() + { + SynchronizationContext.SetSynchronizationContext(_prior); + } + } + + private class Impl : SynchronizationContext + { + private readonly object _lock = new(); + private readonly EditorApplication.CallbackFunction _turnDelegate; + private int unityMainThreadId = -1; + private readonly List asyncQueue = new(); + private readonly List localQueue = new(); + private bool isRegistered, isTurning; + + internal Impl() + { + _turnDelegate = Turn; + } + + // invoked under _lock + private void RegisterCallback() + { + if (isRegistered) return; + isRegistered = true; + EditorApplication.delayCall += _turnDelegate; + } + + public void Turn() + { + lock (_lock) + { + unityMainThreadId = Thread.CurrentThread.ManagedThreadId; + localQueue.AddRange(asyncQueue); + asyncQueue.Clear(); + isRegistered = false; + isTurning = true; + } + + while (localQueue.Count > 0 && !TaskThrottle.ShouldThrottle) + { + foreach (var ev in localQueue) + { + ev.Run(); + } + + localQueue.Clear(); + + if (!TaskThrottle.ShouldThrottle) + { + lock (_lock) + { + localQueue.AddRange(asyncQueue); + asyncQueue.Clear(); + } + } + } + + lock (_lock) + { + if (localQueue.Count > 0) + { + RegisterCallback(); + } + + isTurning = false; + } + } + + public override void Post(SendOrPostCallback d, object state) + { + lock (_lock) + { + asyncQueue.Add(new WorkRequest { callback = d, state = state }); + RegisterCallback(); + } + } + + public override void Send(SendOrPostCallback d, object state) + { + ManualResetEvent wait = null; + var runLocally = false; + lock (_lock) + { + runLocally = unityMainThreadId == Thread.CurrentThread.ManagedThreadId && isTurning; + if (!runLocally) + { + wait = new ManualResetEvent(false); + asyncQueue.Add(new WorkRequest { callback = d, state = state, waitHandle = wait }); + RegisterCallback(); + } + } + + if (runLocally) + { + try + { + d(state); + } + catch (Exception e) + { + Debug.LogException(e); + } + } + else + { + wait.WaitOne(); + } + } + } + + private class WorkRequest + { + public SendOrPostCallback callback; + public object state; + public ManualResetEvent waitHandle; + + public void Run() + { + try + { + callback(state); + } + catch (Exception e) + { + Debug.LogException(e); + } + finally + { + waitHandle?.Set(); + } + } + } + } +} \ No newline at end of file diff --git a/Editor/PreviewSystem/Task/NDMFSyncContext.cs.meta b/Editor/PreviewSystem/Task/NDMFSyncContext.cs.meta new file mode 100644 index 0000000..8317c47 --- /dev/null +++ b/Editor/PreviewSystem/Task/NDMFSyncContext.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: dafa188733324ba9bd3e419a4bc89b7d +timeCreated: 1727038384 \ No newline at end of file diff --git a/Editor/PreviewSystem/TaskThrottle.cs b/Editor/PreviewSystem/TaskThrottle.cs index 5bb3122..e365603 100644 --- a/Editor/PreviewSystem/TaskThrottle.cs +++ b/Editor/PreviewSystem/TaskThrottle.cs @@ -47,6 +47,23 @@ private static void Init() } private static int index; + + public static bool ShouldThrottle + { + get + { + lock (_taskTime) + { + if (!_taskTime.IsRunning) + { + _taskTime.Start(); + return false; + } + + return _taskTime.ElapsedMilliseconds > TASK_TIME_LIMIT_MS; + } + } + } public static async ValueTask MaybeThrottle() {