From 7c87909c367c1aeeda2a8a4caef48da085fb9a15 Mon Sep 17 00:00:00 2001 From: Julien Ignace Date: Fri, 18 Dec 2020 14:17:52 +0100 Subject: [PATCH 1/3] Fixed access to invalid Contexts references after exiting playmode. --- .../Editor/LookDev/CameraController.cs | 25 ++++++++++++++----- .../Editor/LookDev/Compositor.cs | 17 +++++++++---- .../Editor/LookDev/DisplayWindow.cs | 19 ++++++++------ .../Editor/LookDev/LookDev.cs | 19 +++++++++++--- .../Editor/LookDev/Stage.cs | 8 +++--- 5 files changed, 61 insertions(+), 27 deletions(-) diff --git a/com.unity.render-pipelines.core/Editor/LookDev/CameraController.cs b/com.unity.render-pipelines.core/Editor/LookDev/CameraController.cs index 26b67c5f1a6..e3cdb467635 100644 --- a/com.unity.render-pipelines.core/Editor/LookDev/CameraController.cs +++ b/com.unity.render-pipelines.core/Editor/LookDev/CameraController.cs @@ -129,13 +129,18 @@ virtual protected bool isDragging } } - public CameraController(CameraState cameraState, DisplayWindow window, Action focused) + public CameraController(DisplayWindow window, Action focused) { - m_CameraState = cameraState; + m_CameraState = null; m_Window = window; m_Focused = focused; } + public void UpdateCameraState(Context context, ViewIndex index) + { + m_CameraState = context.GetViewContent(index).camera; + } + private void ResetCameraControl() { isDragging = false; @@ -458,16 +463,24 @@ class SwitchableCameraController : CameraController bool switchedDrag = false; bool switchedWheel = false; - public SwitchableCameraController(CameraState cameraStateFirstView, CameraState cameraStateSecondView, DisplayWindow window, Action focused) - : base(cameraStateFirstView, window, null) + public SwitchableCameraController(DisplayWindow window, Action focused) + : base(window, null) { - m_FirstView = cameraStateFirstView; - m_SecondView = cameraStateSecondView; + m_FirstView = null; + m_SecondView = null; m_CurrentViewIndex = ViewIndex.First; m_Focused = () => focused?.Invoke(m_CurrentViewIndex); } + public void UpdateCameraState(Context context) + { + m_FirstView = context.GetViewContent(ViewIndex.First).camera; + m_SecondView = context.GetViewContent(ViewIndex.Second).camera; + + m_CameraState = m_CurrentViewIndex == ViewIndex.First ? m_FirstView : m_SecondView; + } + void SwitchTo(ViewIndex index) { CameraState stateToSwitch; diff --git a/com.unity.render-pipelines.core/Editor/LookDev/Compositor.cs b/com.unity.render-pipelines.core/Editor/LookDev/Compositor.cs index bc2ce086b00..88af4ea21cf 100644 --- a/com.unity.render-pipelines.core/Editor/LookDev/Compositor.cs +++ b/com.unity.render-pipelines.core/Editor/LookDev/Compositor.cs @@ -150,17 +150,16 @@ public bool pixelPerfect public Compositer( IViewDisplayer displayer, - Context contexts, IDataProvider dataProvider, StageCache stages) { m_Displayer = displayer; - m_Contexts = contexts; + m_Contexts = null; m_RenderDataCache = new RenderingData[2] { - new RenderingData() { stage = stages[ViewIndex.First], updater = contexts.GetViewContent(ViewIndex.First).camera }, - new RenderingData() { stage = stages[ViewIndex.Second], updater = contexts.GetViewContent(ViewIndex.Second).camera } + new RenderingData() { stage = stages[ViewIndex.First] }, + new RenderingData() { stage = stages[ViewIndex.Second] } }; m_Displayer.OnRenderDocAcquisitionTriggered += RenderDocAcquisitionRequested; @@ -197,6 +196,12 @@ public void Dispose() public void Render() { + // This can happen when entering/leaving playmode. + if (LookDev.dataProvider == null) + return; + + m_Contexts = LookDev.currentContext; + //TODO: make integration EditorWindow agnostic! if (UnityEditorInternal.RenderDoc.IsLoaded() && UnityEditorInternal.RenderDoc.IsSupported() && m_RenderDocAcquisitionRequested) UnityEditorInternal.RenderDoc.BeginCaptureRenderDoc(m_Displayer as EditorWindow); @@ -236,11 +241,13 @@ void AcquireDataForView(ViewIndex index, Rect viewport) m_RenderTextures.UpdateSize(renderingData.viewPort, index, m_Renderer.pixelPerfect, renderingData.stage.camera); - int debugMode = m_Contexts.GetViewContent(index).debug.viewMode; + int debugMode = view.debug.viewMode; if (debugMode != -1) LookDev.dataProvider.UpdateDebugMode(debugMode); renderingData.output = m_RenderTextures[index, ShadowCompositionPass.MainView]; + renderingData.updater = view.camera; + m_Renderer.BeginRendering(renderingData, LookDev.dataProvider); m_Renderer.Acquire(renderingData); diff --git a/com.unity.render-pipelines.core/Editor/LookDev/DisplayWindow.cs b/com.unity.render-pipelines.core/Editor/LookDev/DisplayWindow.cs index 3f482e01847..743825b172d 100644 --- a/com.unity.render-pipelines.core/Editor/LookDev/DisplayWindow.cs +++ b/com.unity.render-pipelines.core/Editor/LookDev/DisplayWindow.cs @@ -217,6 +217,9 @@ event Action IViewDisplayer.OnUpdateRequested StyleSheet styleSheet = null; StyleSheet styleSheetLight = null; + SwitchableCameraController m_FirstOrCompositeManipulator; + CameraController m_SecondManipulator; + void ReloadStyleSheets() { if (styleSheet == null || styleSheet.Equals(null)) @@ -395,9 +398,7 @@ void CreateViews() m_Views[(int)ViewIndex.Second] = new Image() { name = Style.k_SecondViewName, image = Texture2D.blackTexture }; m_ViewContainer.Add(m_Views[(int)ViewIndex.Second]); - var firstOrCompositeManipulator = new SwitchableCameraController( - LookDev.currentContext.GetViewContent(ViewIndex.First).camera, - LookDev.currentContext.GetViewContent(ViewIndex.Second).camera, + m_FirstOrCompositeManipulator = new SwitchableCameraController( this, index => { @@ -406,8 +407,7 @@ void CreateViews() if (sidePanel == SidePanel.Environment && environment != null && LookDev.currentContext.environmentLibrary != null) m_EnvironmentList.selectedIndex = LookDev.currentContext.environmentLibrary.IndexOf(environment); }); - var secondManipulator = new CameraController( - LookDev.currentContext.GetViewContent(ViewIndex.Second).camera, + m_SecondManipulator = new CameraController( this, () => { @@ -416,10 +416,10 @@ void CreateViews() if (sidePanel == SidePanel.Environment && environment != null && LookDev.currentContext.environmentLibrary != null) m_EnvironmentList.selectedIndex = LookDev.currentContext.environmentLibrary.IndexOf(environment); }); - var gizmoManipulator = new ComparisonGizmoController(LookDev.currentContext.layout.gizmoState, firstOrCompositeManipulator); + var gizmoManipulator = new ComparisonGizmoController(LookDev.currentContext.layout.gizmoState, m_FirstOrCompositeManipulator); m_Views[(int)ViewIndex.First].AddManipulator(gizmoManipulator); //must take event first to switch the firstOrCompositeManipulator - m_Views[(int)ViewIndex.First].AddManipulator(firstOrCompositeManipulator); - m_Views[(int)ViewIndex.Second].AddManipulator(secondManipulator); + m_Views[(int)ViewIndex.First].AddManipulator(m_FirstOrCompositeManipulator); + m_Views[(int)ViewIndex.Second].AddManipulator(m_SecondManipulator); m_NoObject1 = new Label(Style.k_DragAndDropObject); m_NoObject1.style.flexGrow = 1; @@ -667,6 +667,9 @@ void Update() Debug.LogError("LookDev is not supported: No SRP detected."); LookDev.Close(); } + + m_FirstOrCompositeManipulator.UpdateCameraState(LookDev.currentContext); + m_SecondManipulator.UpdateCameraState(LookDev.currentContext, ViewIndex.Second); } void OnGUI() diff --git a/com.unity.render-pipelines.core/Editor/LookDev/LookDev.cs b/com.unity.render-pipelines.core/Editor/LookDev/LookDev.cs index 0e7fb8c8285..4dcff264387 100644 --- a/com.unity.render-pipelines.core/Editor/LookDev/LookDev.cs +++ b/com.unity.render-pipelines.core/Editor/LookDev/LookDev.cs @@ -27,8 +27,21 @@ internal static IDataProvider dataProvider /// internal static Context currentContext { + //get => s_CurrentContext ?? (s_CurrentContext = LoadConfigInternal() ?? defaultContext); + //Lazy init: load it when needed instead in static even if you do not support lookdev - get => s_CurrentContext ?? (s_CurrentContext = LoadConfigInternal() ?? defaultContext); + get + { + if (s_CurrentContext == null || s_CurrentContext.Equals(null)) + { + s_CurrentContext = LoadConfigInternal(); + if (s_CurrentContext == null) + s_CurrentContext = defaultContext; + + ReloadStage(false); + } + return s_CurrentContext; + } private set => s_CurrentContext = value; } @@ -161,9 +174,9 @@ static void WaitingSRPReloadForConfiguringRenderer(int maxAttempt, bool reloadWi static void ConfigureRenderer(bool reloadWithTemporaryID) { s_Stages?.Dispose(); //clean previous occurrence on reloading - s_Stages = new StageCache(dataProvider, currentContext); + s_Stages = new StageCache(dataProvider); s_Compositor?.Dispose(); //clean previous occurrence on reloading - s_Compositor = new Compositer(s_ViewDisplayer, currentContext, dataProvider, s_Stages); + s_Compositor = new Compositer(s_ViewDisplayer, dataProvider, s_Stages); } static void LinkViewDisplayer() diff --git a/com.unity.render-pipelines.core/Editor/LookDev/Stage.cs b/com.unity.render-pipelines.core/Editor/LookDev/Stage.cs index 08551be06b2..d67d18bb966 100644 --- a/com.unity.render-pipelines.core/Editor/LookDev/Stage.cs +++ b/com.unity.render-pipelines.core/Editor/LookDev/Stage.cs @@ -284,7 +284,6 @@ class StageCache : IDisposable const string secondStageName = "LookDevSecondView"; Stage[] m_Stages; - Context m_Contexts; IDataProvider m_CurrentDataProvider; public Stage this[ViewIndex index] @@ -292,9 +291,8 @@ public Stage this[ViewIndex index] public bool initialized { get; private set; } - public StageCache(IDataProvider dataProvider, Context contexts) + public StageCache(IDataProvider dataProvider) { - m_Contexts = contexts; m_Stages = new Stage[2] { InitStage(ViewIndex.First, dataProvider), @@ -333,7 +331,7 @@ public void UpdateSceneObjects(ViewIndex index) Stage stage = this[index]; stage.Clear(); - var viewContent = m_Contexts.GetViewContent(index); + var viewContent = LookDev.currentContext.GetViewContent(index); if (viewContent == null) { viewContent.viewedInstanceInPreview = null; @@ -347,7 +345,7 @@ public void UpdateSceneObjects(ViewIndex index) public void UpdateSceneLighting(ViewIndex index, IDataProvider provider) { Stage stage = this[index]; - Environment environment = m_Contexts.GetViewContent(index).environment; + Environment environment = LookDev.currentContext.GetViewContent(index).environment; provider.UpdateSky(stage.camera, environment == null ? default : environment.sky, stage.runtimeInterface); From 22743a9143ec6f87662a5c716e6423455456f4e2 Mon Sep 17 00:00:00 2001 From: Julien Ignace Date: Fri, 18 Dec 2020 14:24:52 +0100 Subject: [PATCH 2/3] Fixed comparison gizmo after playmode. --- .../Editor/LookDev/ComparisonGizmoController.cs | 8 ++++++-- .../Editor/LookDev/DisplayWindow.cs | 7 +++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/com.unity.render-pipelines.core/Editor/LookDev/ComparisonGizmoController.cs b/com.unity.render-pipelines.core/Editor/LookDev/ComparisonGizmoController.cs index 645fea46e53..9955fd6e923 100644 --- a/com.unity.render-pipelines.core/Editor/LookDev/ComparisonGizmoController.cs +++ b/com.unity.render-pipelines.core/Editor/LookDev/ComparisonGizmoController.cs @@ -49,12 +49,16 @@ bool isDragging } } - public ComparisonGizmoController(ComparisonGizmoState state, SwitchableCameraController switcher) + public ComparisonGizmoController(SwitchableCameraController switcher) { - m_State = state; m_Switcher = switcher; } + public void UpdateGizmoState(ComparisonGizmoState state) + { + m_State = state; + } + protected override void RegisterCallbacksOnTarget() { target.RegisterCallback(OnMouseDown); diff --git a/com.unity.render-pipelines.core/Editor/LookDev/DisplayWindow.cs b/com.unity.render-pipelines.core/Editor/LookDev/DisplayWindow.cs index 743825b172d..ee8c36705a2 100644 --- a/com.unity.render-pipelines.core/Editor/LookDev/DisplayWindow.cs +++ b/com.unity.render-pipelines.core/Editor/LookDev/DisplayWindow.cs @@ -219,6 +219,7 @@ event Action IViewDisplayer.OnUpdateRequested SwitchableCameraController m_FirstOrCompositeManipulator; CameraController m_SecondManipulator; + ComparisonGizmoController m_GizmoManipulator; void ReloadStyleSheets() { @@ -416,8 +417,8 @@ void CreateViews() if (sidePanel == SidePanel.Environment && environment != null && LookDev.currentContext.environmentLibrary != null) m_EnvironmentList.selectedIndex = LookDev.currentContext.environmentLibrary.IndexOf(environment); }); - var gizmoManipulator = new ComparisonGizmoController(LookDev.currentContext.layout.gizmoState, m_FirstOrCompositeManipulator); - m_Views[(int)ViewIndex.First].AddManipulator(gizmoManipulator); //must take event first to switch the firstOrCompositeManipulator + m_GizmoManipulator = new ComparisonGizmoController(m_FirstOrCompositeManipulator); + m_Views[(int)ViewIndex.First].AddManipulator(m_GizmoManipulator); //must take event first to switch the firstOrCompositeManipulator m_Views[(int)ViewIndex.First].AddManipulator(m_FirstOrCompositeManipulator); m_Views[(int)ViewIndex.Second].AddManipulator(m_SecondManipulator); @@ -668,8 +669,10 @@ void Update() LookDev.Close(); } + // All those states coming from the Contexts can become invalid after a domain reload so we need to update them. m_FirstOrCompositeManipulator.UpdateCameraState(LookDev.currentContext); m_SecondManipulator.UpdateCameraState(LookDev.currentContext, ViewIndex.Second); + m_GizmoManipulator.UpdateGizmoState(LookDev.currentContext.layout.gizmoState); } void OnGUI() From bd68f2fbc9749bc4945a5058716564b05e82495b Mon Sep 17 00:00:00 2001 From: Julien Ignace Date: Fri, 18 Dec 2020 15:37:13 +0100 Subject: [PATCH 3/3] Fixes from PR feedback --- .../Editor/LookDev/CameraController.cs | 3 --- com.unity.render-pipelines.core/Editor/LookDev/Compositor.cs | 1 - com.unity.render-pipelines.core/Editor/LookDev/LookDev.cs | 2 -- 3 files changed, 6 deletions(-) diff --git a/com.unity.render-pipelines.core/Editor/LookDev/CameraController.cs b/com.unity.render-pipelines.core/Editor/LookDev/CameraController.cs index e3cdb467635..a2271c88f3c 100644 --- a/com.unity.render-pipelines.core/Editor/LookDev/CameraController.cs +++ b/com.unity.render-pipelines.core/Editor/LookDev/CameraController.cs @@ -131,7 +131,6 @@ virtual protected bool isDragging public CameraController(DisplayWindow window, Action focused) { - m_CameraState = null; m_Window = window; m_Focused = focused; } @@ -466,8 +465,6 @@ class SwitchableCameraController : CameraController public SwitchableCameraController(DisplayWindow window, Action focused) : base(window, null) { - m_FirstView = null; - m_SecondView = null; m_CurrentViewIndex = ViewIndex.First; m_Focused = () => focused?.Invoke(m_CurrentViewIndex); diff --git a/com.unity.render-pipelines.core/Editor/LookDev/Compositor.cs b/com.unity.render-pipelines.core/Editor/LookDev/Compositor.cs index 88af4ea21cf..15698d250e4 100644 --- a/com.unity.render-pipelines.core/Editor/LookDev/Compositor.cs +++ b/com.unity.render-pipelines.core/Editor/LookDev/Compositor.cs @@ -154,7 +154,6 @@ public Compositer( StageCache stages) { m_Displayer = displayer; - m_Contexts = null; m_RenderDataCache = new RenderingData[2] { diff --git a/com.unity.render-pipelines.core/Editor/LookDev/LookDev.cs b/com.unity.render-pipelines.core/Editor/LookDev/LookDev.cs index 4dcff264387..756dacb06b9 100644 --- a/com.unity.render-pipelines.core/Editor/LookDev/LookDev.cs +++ b/com.unity.render-pipelines.core/Editor/LookDev/LookDev.cs @@ -27,8 +27,6 @@ internal static IDataProvider dataProvider /// internal static Context currentContext { - //get => s_CurrentContext ?? (s_CurrentContext = LoadConfigInternal() ?? defaultContext); - //Lazy init: load it when needed instead in static even if you do not support lookdev get {