From 4f049cf1558618bafebbd85188b254baf76eba41 Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Mon, 19 Nov 2018 15:39:36 +0300 Subject: [PATCH 1/7] Allow GC to collect reused framebuffer, that's better than AccessViolationException from render thread --- src/Windows/Avalonia.Win32/FramebufferManager.cs | 7 +------ src/Windows/Avalonia.Win32/WindowImpl.cs | 3 +-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Windows/Avalonia.Win32/FramebufferManager.cs b/src/Windows/Avalonia.Win32/FramebufferManager.cs index c910703181..87c5a1bb02 100644 --- a/src/Windows/Avalonia.Win32/FramebufferManager.cs +++ b/src/Windows/Avalonia.Win32/FramebufferManager.cs @@ -5,7 +5,7 @@ using Avalonia.Win32.Interop; namespace Avalonia.Win32 { - class FramebufferManager : IFramebufferPlatformSurface, IDisposable + class FramebufferManager : IFramebufferPlatformSurface { private readonly IntPtr _hwnd; private WindowFramebuffer _fb; @@ -29,10 +29,5 @@ namespace Avalonia.Win32 } return _fb; } - - public void Dispose() - { - _fb?.Deallocate(); - } } } diff --git a/src/Windows/Avalonia.Win32/WindowImpl.cs b/src/Windows/Avalonia.Win32/WindowImpl.cs index 4c08e985cd..18f0696cd8 100644 --- a/src/Windows/Avalonia.Win32/WindowImpl.cs +++ b/src/Windows/Avalonia.Win32/WindowImpl.cs @@ -13,6 +13,7 @@ using Avalonia.Input.Raw; using Avalonia.OpenGL; using Avalonia.Platform; using Avalonia.Rendering; +using Avalonia.Threading; using Avalonia.Win32.Input; using Avalonia.Win32.Interop; using static Avalonia.Win32.Interop.UnmanagedMethods; @@ -234,8 +235,6 @@ namespace Avalonia.Win32 public void Dispose() { - _framebuffer?.Dispose(); - _framebuffer = null; if (_hwnd != IntPtr.Zero) { UnmanagedMethods.DestroyWindow(_hwnd); From d1e304741019b4796435994b8b27151b36e502d8 Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Mon, 19 Nov 2018 15:40:46 +0300 Subject: [PATCH 2/7] Don't be noisy about just-in-case Dispose call on disposed object on finalizer thread --- src/Shared/PlatformSupport/StandardRuntimePlatform.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Shared/PlatformSupport/StandardRuntimePlatform.cs b/src/Shared/PlatformSupport/StandardRuntimePlatform.cs index 186c55d9eb..8a5a725594 100644 --- a/src/Shared/PlatformSupport/StandardRuntimePlatform.cs +++ b/src/Shared/PlatformSupport/StandardRuntimePlatform.cs @@ -89,9 +89,11 @@ namespace Avalonia.Shared.PlatformSupport #if DEBUG if (Thread.CurrentThread.ManagedThreadId == GCThread?.ManagedThreadId) { - Console.Error.WriteLine("Native blob disposal from finalizer thread\nBacktrace: " - + Environment.StackTrace - + "\n\nBlob created by " + _backtrace); + lock(_lock) + if (!IsDisposed) + Console.Error.WriteLine("Native blob disposal from finalizer thread\nBacktrace: " + + Environment.StackTrace + + "\n\nBlob created by " + _backtrace); } #endif DoDispose(); From 31b6a0d16ca4efbc8cad1b06556e83a17b5db6f2 Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Mon, 19 Nov 2018 16:02:40 +0300 Subject: [PATCH 3/7] Wait for scene to be consumed before producing a new one. Fixes TONS of variour rendering artifacts --- .../Rendering/DeferredRenderer.cs | 40 +++++++++++++++---- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs index a268eff78a..a45ea62cd2 100644 --- a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs @@ -37,6 +37,7 @@ namespace Avalonia.Rendering private DisplayDirtyRects _dirtyRectsDisplay = new DisplayDirtyRects(); private IRef _currentDraw; private readonly IDeferredRendererLock _lock; + private readonly object _sceneLock = new object(); /// /// Initializes a new instance of the class. @@ -172,7 +173,8 @@ namespace Avalonia.Rendering } } - bool IRenderLoopTask.NeedsUpdate => _dirty == null || _dirty.Count > 0; + bool NeedsUpdate => _dirty == null || _dirty.Count > 0; + bool IRenderLoopTask.NeedsUpdate => NeedsUpdate; void IRenderLoopTask.Update(TimeSpan time) => UpdateScene(); @@ -197,19 +199,23 @@ namespace Avalonia.Rendering internal void UnitTestUpdateScene() => UpdateScene(); - internal void UnitTestRender() => Render(_scene.Item, false); + internal void UnitTestRender() => Render(_scene.Item, false, false); private void Render(bool forceComposite) { using (var l = _lock.TryLock()) if (l != null) - using (var scene = _scene?.Clone()) + { + bool reRun = false; + do { - Render(scene?.Item, forceComposite); - } + using (var scene = _scene?.Clone()) + reRun = Render(scene?.Item, forceComposite, reRun); + } while (reRun); + } } - - private void Render(Scene scene, bool forceComposite) + + private bool Render(Scene scene, bool forceComposite, bool reUpdating) { bool renderOverlay = DrawDirtyRects || DrawFps; bool composite = false; @@ -242,7 +248,18 @@ namespace Avalonia.Rendering SaveDebugFrames(scene.Generation); } - _lastSceneId = scene.Generation; + lock (_sceneLock) + _lastSceneId = scene.Generation; + + // We have consumed the previously available scene, but there might be some dirty + // rects since the last update. *If* we are on UI thread, we can force immediate scene + // rebuild before rendering anything on-screen + // By returning true we indicate that this method should be called again + if (!reUpdating && Dispatcher.UIThread.CheckAccess() && NeedsUpdate) + { + UpdateScene(); + return true; + } composite = true; } @@ -268,6 +285,8 @@ namespace Avalonia.Rendering RenderTarget?.Dispose(); RenderTarget = null; } + + return false; } private void Render(IDrawingContextImpl context, VisualNode node, IVisual layer, Rect clipBounds) @@ -405,6 +424,11 @@ namespace Avalonia.Rendering private void UpdateScene() { Dispatcher.UIThread.VerifyAccess(); + lock (_sceneLock) + { + if (_scene?.Item.Generation > _lastSceneId) + return; + } if (_root.IsVisible) { var sceneRef = RefCountable.Create(_scene?.Item.CloneScene() ?? new Scene(_root)); From 02200eb4d3d2e3ef38c0390212214f08c52b01b3 Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Mon, 19 Nov 2018 16:12:47 +0300 Subject: [PATCH 4/7] Use proper locks for _scene manipulation --- .../Rendering/DeferredRenderer.cs | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs index a45ea62cd2..cbf472426f 100644 --- a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs @@ -119,8 +119,13 @@ namespace Avalonia.Rendering /// public void Dispose() { - var scene = Interlocked.Exchange(ref _scene, null); - scene?.Dispose(); + lock (_sceneLock) + { + var scene = _scene; + _scene = null; + scene?.Dispose(); + } + Stop(); Layers.Clear(); @@ -135,7 +140,8 @@ namespace Avalonia.Rendering // When unit testing the renderLoop may be null, so update the scene manually. UpdateScene(); } - + //It's safe to access _scene here without a lock since + //it's only changed from UI thread which we are currently on return _scene?.Item.HitTest(p, root, filter) ?? Enumerable.Empty(); } @@ -209,7 +215,10 @@ namespace Avalonia.Rendering bool reRun = false; do { - using (var scene = _scene?.Clone()) + IRef scene; + lock (_sceneLock) + scene = _scene?.Clone(); + using (scene) reRun = Render(scene?.Item, forceComposite, reRun); } while (reRun); } @@ -447,15 +456,23 @@ namespace Avalonia.Rendering } } - var oldScene = Interlocked.Exchange(ref _scene, sceneRef); - oldScene?.Dispose(); + lock (_sceneLock) + { + var oldScene = _scene; + _scene = sceneRef; + oldScene?.Dispose(); + } _dirty.Clear(); } else { - var oldScene = Interlocked.Exchange(ref _scene, null); - oldScene?.Dispose(); + lock (_sceneLock) + { + var oldScene = _scene; + _scene = null; + oldScene?.Dispose(); + } } } From 36915d6c9453260105de31807d75984cc772fe38 Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Mon, 19 Nov 2018 16:57:34 +0300 Subject: [PATCH 5/7] Reworked rendering --- .../Rendering/DeferredRenderer.cs | 146 +++++++++--------- 1 file changed, 76 insertions(+), 70 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs index cbf472426f..d2852786c6 100644 --- a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs @@ -205,99 +205,105 @@ namespace Avalonia.Rendering internal void UnitTestUpdateScene() => UpdateScene(); - internal void UnitTestRender() => Render(_scene.Item, false, false); + internal void UnitTestRender() => Render(false); private void Render(bool forceComposite) { using (var l = _lock.TryLock()) - if (l != null) - { - bool reRun = false; - do - { - IRef scene; - lock (_sceneLock) - scene = _scene?.Clone(); - using (scene) - reRun = Render(scene?.Item, forceComposite, reRun); - } while (reRun); - } - } - - private bool Render(Scene scene, bool forceComposite, bool reUpdating) - { - bool renderOverlay = DrawDirtyRects || DrawFps; - bool composite = false; - - if (RenderTarget == null) { - RenderTarget = ((IRenderRoot)_root).CreateRenderTarget(); - } - - if (renderOverlay) - { - _dirtyRectsDisplay.Tick(); - } + if (l == null) + return; - try - { - if (scene != null && scene.Size != Size.Empty) + IDrawingContextImpl context = null; + try { - IDrawingContextImpl context = null; - - if (scene.Generation != _lastSceneId) + try { - context = RenderTarget.CreateDrawingContext(this); - Layers.Update(scene, context); - - RenderToLayers(scene); - - if (DebugFramesPath != null) + IDrawingContextImpl GetContext() { - SaveDebugFrames(scene.Generation); - } + if (context != null) + return context; + if (RenderTarget == null) + RenderTarget = ((IRenderRoot)_root).CreateRenderTarget(); + return context = RenderTarget.CreateDrawingContext(this); - lock (_sceneLock) - _lastSceneId = scene.Generation; + } - // We have consumed the previously available scene, but there might be some dirty - // rects since the last update. *If* we are on UI thread, we can force immediate scene - // rebuild before rendering anything on-screen - // By returning true we indicate that this method should be called again - if (!reUpdating && Dispatcher.UIThread.CheckAccess() && NeedsUpdate) + var (scene, updated) = UpdateRenderLayersAndConsumeSceneIfNeeded(GetContext); + using (scene) { - UpdateScene(); - return true; + var overlay = DrawDirtyRects || DrawFps; + if (DrawDirtyRects) + _dirtyRectsDisplay.Tick(); + if (overlay) + RenderOverlay(scene.Item, GetContext()); + if (updated || forceComposite || overlay) + RenderComposite(scene.Item, GetContext()); } - - composite = true; } - - if (renderOverlay) + finally { - context = context ?? RenderTarget.CreateDrawingContext(this); - RenderOverlay(scene, context); - RenderComposite(scene, context); + context?.Dispose(); } - else if (composite || forceComposite) + } + catch (RenderTargetCorruptedException ex) + { + Logging.Logger.Information("Renderer", this, "Render target was corrupted. Exception: {0}", ex); + RenderTarget?.Dispose(); + RenderTarget = null; + } + } + } + + private (IRef scene, bool updated) UpdateRenderLayersAndConsumeSceneIfNeeded(Func contextFactory, + bool recursiveCall = false) + { + IRef sceneRef; + lock (_sceneLock) + sceneRef = _scene?.Clone(); + if (sceneRef == null) + return (null, false); + using (sceneRef) + { + var scene = sceneRef.Item; + if (scene.Generation != _lastSceneId) + { + var context = contextFactory(); + Layers.Update(scene, context); + + RenderToLayers(scene); + + if (DebugFramesPath != null) { - context = context ?? RenderTarget.CreateDrawingContext(this); - RenderComposite(scene, context); + SaveDebugFrames(scene.Generation); } - context?.Dispose(); + lock (_sceneLock) + _lastSceneId = scene.Generation; + + + // We have consumed the previously available scene, but there might be some dirty + // rects since the last update. *If* we are on UI thread, we can force immediate scene + // rebuild before rendering anything on-screen + // We are calling the same method recursively here + if (!recursiveCall && Dispatcher.UIThread.CheckAccess() && NeedsUpdate) + { + UpdateScene(); + var (rs, _) = UpdateRenderLayersAndConsumeSceneIfNeeded(contextFactory, true); + return (rs, true); + } + + // Indicate that we have updated the layers + return (sceneRef.Clone(), true); } + + // Just return scene, layers weren't updated + return (sceneRef.Clone(), false); } - catch (RenderTargetCorruptedException ex) - { - Logging.Logger.Information("Renderer", this, "Render target was corrupted. Exception: {0}", ex); - RenderTarget?.Dispose(); - RenderTarget = null; - } - - return false; + } + private void Render(IDrawingContextImpl context, VisualNode node, IVisual layer, Rect clipBounds) { if (layer == null || node.LayerRoot == layer) From 2be6390683240a38c10442d8b54f49aaf78f52b1 Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Mon, 19 Nov 2018 17:38:22 +0300 Subject: [PATCH 6/7] Fixed tests --- .../Rendering/DeferredRendererTests_HitTesting.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests_HitTesting.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests_HitTesting.cs index a53809a029..ab5f5f37f7 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests_HitTesting.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests_HitTesting.cs @@ -405,7 +405,8 @@ namespace Avalonia.Visuals.UnitTests.Rendering root.Renderer = new DeferredRenderer(root, null); root.Measure(Size.Infinity); root.Arrange(new Rect(container.DesiredSize)); - + + root.Renderer.Paint(Rect.Empty); var result = root.Renderer.HitTest(new Point(50, 150), root, null).First(); Assert.Equal(item1, result); @@ -421,6 +422,7 @@ namespace Avalonia.Visuals.UnitTests.Rendering container.InvalidateArrange(); container.Arrange(new Rect(container.DesiredSize)); + root.Renderer.Paint(Rect.Empty); result = root.Renderer.HitTest(new Point(50, 150), root, null).First(); Assert.Equal(item2, result); From 5e698a4ab20545ca0409630188bdc1491570ab72 Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Mon, 19 Nov 2018 18:13:39 +0300 Subject: [PATCH 7/7] Fixed other tests --- src/Avalonia.Visuals/Rendering/DeferredRenderer.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs index d2852786c6..b48efaa34e 100644 --- a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs @@ -85,6 +85,7 @@ namespace Avalonia.Rendering RenderTarget = renderTarget; _sceneBuilder = sceneBuilder ?? new SceneBuilder(); Layers = new RenderLayers(); + _lock = new ManagedDeferredRendererLock(); } ///