Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: flickering selection outlines #454

Merged
merged 1 commit into from
Oct 15, 2024
Merged

fix: flickering selection outlines #454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

### Fixed
- [#454] Scene view selection outlines flicker in some cases
- [#450] Improved performance when a large number of object change events are generated (e.g. when exiting animation
mode)
- [#441] Fixed an issue where the preview object pickable status could get out of sync with the original
Expand Down
148 changes: 89 additions & 59 deletions Editor/PreviewSystem/Rendering/ProxyObjectCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,75 @@ public static bool IsProxyObject(GameObject obj)

return _proxyObjectInstanceIds.Contains(obj.GetInstanceID());
}


public interface IProxyHandle : IDisposable
{
public Renderer PrimaryProxy { get; }
public Renderer GetSetupProxy();
public void ReturnSetupProxy(Renderer proxy);
}

private class ProxyHandleImpl : IProxyHandle
{
private readonly ProxyObjectCache _cache;
private readonly Renderer _key;
private readonly Func<Renderer> _createFunc;
private readonly RendererState _state;

public ProxyHandleImpl(ProxyObjectCache cache, Renderer key, Func<Renderer> createFunc, RendererState state)
{
_cache = cache;
_key = key;
_createFunc = createFunc;
_state = state;
}

public Renderer PrimaryProxy => _state.PrimaryProxy;

public Renderer GetSetupProxy()
{
if (_state.InactiveSetupProxy != null)
{
var proxy = _state.InactiveSetupProxy;
proxy.enabled = true;
_state.InactiveSetupProxy = null;
return proxy;
}

return _createFunc();
}

public void ReturnSetupProxy(Renderer proxy)
{
if (_state.InactiveSetupProxy != null || _state.ActivePrimaryCount == 0)
{
DestroyProxy(proxy);
}
else
{
proxy.enabled = false;
_state.InactiveSetupProxy = proxy;
}
}

public void Dispose()
{
if (--_state.ActivePrimaryCount == 0)
{
_cache.MaybeDisposeProxy(_key);
}
}
}

private class RendererState
{
public Renderer InactiveProxy;
public int ActiveProxyCount = 0;
public Renderer PrimaryProxy;
public int ActivePrimaryCount;

public Renderer InactiveSetupProxy;
}

private readonly Dictionary<Renderer, RendererState> _renderers = new();
private bool _cleanupPending = false;
private bool _isRegistered = false;

private bool IsRegistered
Expand All @@ -37,10 +97,12 @@ private bool IsRegistered
if (value)
{
EditorApplication.playModeStateChanged += OnPlayModeStateChanged;
EditorApplication.update += Cleanup;
}
else
{
EditorApplication.playModeStateChanged -= OnPlayModeStateChanged;
EditorApplication.update -= Cleanup;
}

_isRegistered = value;
Expand All @@ -53,69 +115,26 @@ private void OnPlayModeStateChanged(PlayModeStateChange obj)
Dispose();
}

public Renderer GetOrCreate(Renderer original, Func<Renderer> create)
public IProxyHandle GetHandle(Renderer original, Func<Renderer> create)
{
IsRegistered = true;

if (!_renderers.TryGetValue(original, out var state))
{
state = new RendererState();
state.PrimaryProxy = create();
_renderers.Add(original, state);
}

Renderer proxy;
if (state.InactiveProxy != null)
{
proxy = state.InactiveProxy;
state.InactiveProxy = null;
state.ActiveProxyCount++;
proxy.enabled = true;
return proxy;
}

proxy = create();
if (proxy == null)
{
return null;
}

state.ActiveProxyCount++;
_proxyObjectInstanceIds.Add(proxy.gameObject.GetInstanceID());

return proxy;
}

public void ReturnProxy(Renderer original, Renderer proxy)
{
IsRegistered = true;

if (!_renderers.TryGetValue(original, out var state))
{
DestroyProxy(proxy);
return;
}

if (!_cleanupPending)
{
EditorApplication.delayCall += Cleanup;
_cleanupPending = true;
}
state.ActivePrimaryCount++;

state.ActiveProxyCount--;
if (state.ActiveProxyCount > 0 && state.InactiveProxy == null)
return new ProxyHandleImpl(this, original, () =>
{
state.InactiveProxy = proxy;
proxy.enabled = false;
return;
}

DestroyProxy(proxy);
var newProxy = create();
_proxyObjectInstanceIds.Add(newProxy.gameObject.GetInstanceID());

if (state.ActiveProxyCount == 0)
{
DestroyProxy(state.InactiveProxy);
_renderers.Remove(original);
}
return newProxy;
}, state);
}

private static void DestroyProxy(Renderer proxy)
Expand All @@ -127,13 +146,23 @@ private static void DestroyProxy(Renderer proxy)
Object.DestroyImmediate(gameObject);
}


private void MaybeDisposeProxy(Renderer key)
{
if (_renderers.TryGetValue(key, out var state) && state.ActivePrimaryCount == 0)
{
DestroyProxy(state.PrimaryProxy);
DestroyProxy(state.InactiveSetupProxy);
_renderers.Remove(key);
}
}

private void Cleanup()
{
_cleanupPending = false;

foreach (var entry in _renderers.Where(kv => kv.Key == null).ToList())
{
DestroyProxy(entry.Value.InactiveProxy);
DestroyProxy(entry.Value.InactiveSetupProxy);
DestroyProxy(entry.Value.PrimaryProxy);
_renderers.Remove(entry.Key);
}
}
Expand All @@ -142,7 +171,8 @@ public void Dispose()
{
foreach (var entry in _renderers)
{
DestroyProxy(entry.Value.InactiveProxy);
DestroyProxy(entry.Value.InactiveSetupProxy);
DestroyProxy(entry.Value.PrimaryProxy);
}
_renderers.Clear();
IsRegistered = false;
Expand Down
57 changes: 36 additions & 21 deletions Editor/PreviewSystem/Rendering/ProxyObjectController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

using System;
using System.Linq;
using System.Threading.Tasks;
using UnityEditor;
using UnityEngine;
using UnityEngine.Profiling;
Expand All @@ -17,9 +16,11 @@ internal class ProxyObjectController : IDisposable
{
private readonly ProxyObjectCache _cache;
private readonly Renderer _originalRenderer;
private Renderer _replacementRenderer;
internal Renderer Renderer => _replacementRenderer;
public bool IsValid => _originalRenderer != null && _replacementRenderer != null;
private bool _setupComplete;
private Renderer _setupProxy, _primaryProxy;
private ProxyObjectCache.IProxyHandle _proxyHandle;

internal Renderer Renderer => _setupComplete ? _primaryProxy : _setupProxy;

internal RenderAspects ChangeFlags;

Expand Down Expand Up @@ -132,9 +133,9 @@ private void SetupRendererMonitoring(Renderer r)

internal bool OnPreFrame()
{
if (_replacementRenderer == null || _originalRenderer == null)
if (Renderer == null || _originalRenderer == null)
{
if (_replacementRenderer == null)
if (Renderer == null)
{
Debug.LogWarning("Proxy object was destroyed improperly! Resetting pipeline...");
}
Expand All @@ -145,7 +146,7 @@ internal bool OnPreFrame()

try
{
var target = _replacementRenderer;
var target = Renderer;
var original = _originalRenderer;

if (VisibilityMonitor.Sequence != _lastVisibilityCheck)
Expand Down Expand Up @@ -176,7 +177,7 @@ internal bool OnPreFrame()
{
smr = smr_;

var replacementSMR = (SkinnedMeshRenderer)_replacementRenderer;
var replacementSMR = (SkinnedMeshRenderer)Renderer;
replacementSMR.sharedMesh = smr_.sharedMesh;
replacementSMR.bones = smr_.bones;

Expand All @@ -186,12 +187,12 @@ internal bool OnPreFrame()
else
{
var originalFilter = _originalRenderer.GetComponent<MeshFilter>();
var filter = _replacementRenderer.GetComponent<MeshFilter>();
var filter = Renderer.GetComponent<MeshFilter>();
filter.sharedMesh = originalFilter.sharedMesh;

var shadowBone = ShadowBoneManager.Instance.GetBone(_originalRenderer.transform).proxy;

var rendererTransform = _replacementRenderer.transform;
var rendererTransform = Renderer.transform;
if (shadowBone != rendererTransform.parent)
{
rendererTransform.SetParent(shadowBone, false);
Expand All @@ -202,7 +203,7 @@ internal bool OnPreFrame()

target.enabled = original.enabled && original.gameObject.activeInHierarchy;

_replacementRenderer.sharedMaterials = _originalRenderer.sharedMaterials;
Renderer.sharedMaterials = _originalRenderer.sharedMaterials;


target.localBounds = original.localBounds;
Expand Down Expand Up @@ -239,27 +240,27 @@ internal bool OnPreFrame()

internal void FinishPreFrame(bool isSceneViewCamera)
{
if (_replacementRenderer != null)
if (Renderer != null)
{
var shouldEnable = _replacementRenderer.enabled & !(isSceneViewCamera && _visibilityOffOriginal);
var shouldEnable = Renderer.enabled & !(isSceneViewCamera && _visibilityOffOriginal);
Mesh currentSharedMesh = null;

if (_replacementRenderer is SkinnedMeshRenderer smr)
if (Renderer is SkinnedMeshRenderer smr)
currentSharedMesh = smr.sharedMesh;
else if (_replacementRenderer is MeshRenderer mr)
else if (Renderer is MeshRenderer mr)
currentSharedMesh = mr.GetComponent<MeshFilter>().sharedMesh;

if (currentSharedMesh != _initialSharedMesh) _replacementRenderer.enabled = false;
if (currentSharedMesh != _initialSharedMesh) Renderer.enabled = false;

_replacementRenderer.enabled = shouldEnable;
Renderer.enabled = shouldEnable;
}
}

private void CreateReplacementObject()
{
if (_originalRenderer == null) return;

_replacementRenderer = _cache.GetOrCreate(_originalRenderer, () =>
_proxyHandle = _cache.GetHandle(_originalRenderer, () =>
{
var replacementGameObject = new GameObject("Proxy renderer for " + _originalRenderer.gameObject.name);
replacementGameObject.hideFlags = HideFlags.DontSave;
Expand Down Expand Up @@ -292,15 +293,29 @@ private void CreateReplacementObject()

return renderer;
});

_primaryProxy = _proxyHandle.PrimaryProxy;
_setupProxy = _proxyHandle.GetSetupProxy();
}

public void FinishSetup()
{
if (_setupComplete) return;

_setupComplete = true;
_proxyHandle.ReturnSetupProxy(_setupProxy);
_setupProxy = null;
}

public void Dispose()
{
if (_replacementRenderer != null)
if (_setupProxy != null)
{
_cache.ReturnProxy(_originalRenderer, _replacementRenderer);
_replacementRenderer = null;
_proxyHandle.ReturnSetupProxy(_setupProxy);
}

_setupProxy = null;
_proxyHandle.Dispose();
}
}
}
13 changes: 9 additions & 4 deletions Editor/PreviewSystem/Rendering/ProxyPipeline.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,6 @@ private async Task Build(ProxyObjectCache proxyCache, IEnumerable<IRenderFilter>
// we render for real).
_proxies.Add(r, proxy);

OriginalToProxyRenderer = OriginalToProxyRenderer.Add(r, proxy.Renderer);
OriginalToProxyObject = OriginalToProxyObject.Add(r.gameObject, proxy.Renderer.gameObject);
ProxyToOriginalObject = ProxyToOriginalObject.Add(proxy.Renderer.gameObject, r.gameObject);

var registry = new ObjectRegistry(null);
((IObjectRegistry)registry).RegisterReplacedObject(r, proxy.Renderer);

Expand Down Expand Up @@ -303,6 +299,15 @@ await Task.WhenAll(_stages.SelectMany(s => s.NodeTasks))

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

foreach (var (r, proxy) in _proxies)
{
proxy.FinishSetup();

OriginalToProxyRenderer = OriginalToProxyRenderer.Add(r, proxy.Renderer);
OriginalToProxyObject = OriginalToProxyObject.Add(r.gameObject, proxy.Renderer.gameObject);
ProxyToOriginalObject = ProxyToOriginalObject.Add(proxy.Renderer.gameObject, r.gameObject);
}

#if NDMF_DEBUG
Debug.Log($"Pipeline {_generation} is ready");
#endif
Expand Down
Loading