From c311474d5b5a507e73b96c8000d96480c5f8088f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 28 Sep 2018 17:28:13 +0200 Subject: [PATCH 1/6] Don't await updates in render loop. The render loop should not be waiting for an update to occur on the UI thread before rendering a frame. Instead of awaiting the call, simply call `IDispatcher.Post` to fire-and-forget. Placed a guard around the update to make sure multiple updates don't get queued if an update doesn't complete in a single frame. Also don't call `IRenderLoopTask.Update` unless `IRenderLoopTask.NeedsUpdate == true`. Fixes #1920 --- src/Avalonia.Visuals/Rendering/RenderLoop.cs | 31 +++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/RenderLoop.cs b/src/Avalonia.Visuals/Rendering/RenderLoop.cs index d0d5b2250d..427c91658b 100644 --- a/src/Avalonia.Visuals/Rendering/RenderLoop.cs +++ b/src/Avalonia.Visuals/Rendering/RenderLoop.cs @@ -19,7 +19,8 @@ namespace Avalonia.Rendering private readonly IDispatcher _dispatcher; private List _items = new List(); private IRenderTimer _timer; - private int inTick; + private int _inTick; + private int _inUpdate; /// /// Initializes a new instance of the class. @@ -84,21 +85,35 @@ namespace Avalonia.Rendering } } - private async void TimerTick(TimeSpan time) + private void TimerTick(TimeSpan time) { - if (Interlocked.CompareExchange(ref inTick, 1, 0) == 0) + if (Interlocked.CompareExchange(ref _inTick, 1, 0) == 0) { try { - if (_items.Any(item => item.NeedsUpdate)) + if (_items.Any(item => item.NeedsUpdate) && + Interlocked.CompareExchange(ref _inUpdate, 1, 0) == 0) { - await _dispatcher.InvokeAsync(() => + System.Diagnostics.Debug.WriteLine("Posted update"); + _dispatcher.Post(() => { foreach (var i in _items) { - i.Update(time); + if (i.NeedsUpdate) + { + try + { + i.Update(time); + } + catch (Exception ex) + { + Logger.Error(LogArea.Visual, this, "Exception in render update: {Error}", ex); + } + } } - }, DispatcherPriority.Render).ConfigureAwait(false); + + Interlocked.Exchange(ref _inUpdate, 0); + }, DispatcherPriority.Render); } foreach (var i in _items) @@ -112,7 +127,7 @@ namespace Avalonia.Rendering } finally { - Interlocked.Exchange(ref inTick, 0); + Interlocked.Exchange(ref _inTick, 0); } } } From 51f85cfac9a375912b8efd7bbf483b01d069d8d4 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 28 Sep 2018 20:10:44 +0200 Subject: [PATCH 2/6] Removed debug code. --- src/Avalonia.Visuals/Rendering/RenderLoop.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Avalonia.Visuals/Rendering/RenderLoop.cs b/src/Avalonia.Visuals/Rendering/RenderLoop.cs index 427c91658b..e10e1c4006 100644 --- a/src/Avalonia.Visuals/Rendering/RenderLoop.cs +++ b/src/Avalonia.Visuals/Rendering/RenderLoop.cs @@ -94,7 +94,6 @@ namespace Avalonia.Rendering if (_items.Any(item => item.NeedsUpdate) && Interlocked.CompareExchange(ref _inUpdate, 1, 0) == 0) { - System.Diagnostics.Debug.WriteLine("Posted update"); _dispatcher.Post(() => { foreach (var i in _items) From 62a5b66228fbf5e08f0b63e3cd888f9c7e971165 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 28 Sep 2018 20:11:35 +0200 Subject: [PATCH 3/6] Fix failing tests. --- .../Rendering/RenderLoopTests.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/RenderLoopTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/RenderLoopTests.cs index 16c2d3ee18..e9aefc46e6 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/RenderLoopTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/RenderLoopTests.cs @@ -19,14 +19,13 @@ namespace Avalonia.Visuals.UnitTests.Rendering bool inDispatcher = false; dispatcher.Setup( - d => d.InvokeAsync(It.IsAny(), DispatcherPriority.Render)) + d => d.Post(It.IsAny(), DispatcherPriority.Render)) .Callback((Action a, DispatcherPriority _) => { inDispatcher = true; a(); inDispatcher = false; - }) - .Returns(Task.CompletedTask); + }); var timer = new Mock(); @@ -71,14 +70,13 @@ namespace Avalonia.Visuals.UnitTests.Rendering var dispatcher = new Mock(); bool inDispatcher = false; dispatcher.Setup( - d => d.InvokeAsync(It.IsAny(), DispatcherPriority.Render)) + d => d.Post(It.IsAny(), DispatcherPriority.Render)) .Callback((Action a, DispatcherPriority _) => { inDispatcher = true; a(); inDispatcher = false; - }) - .Returns(Task.CompletedTask); + }); var timer = new Mock(); var loop = new RenderLoop(timer.Object, dispatcher.Object); @@ -100,9 +98,8 @@ namespace Avalonia.Visuals.UnitTests.Rendering { var dispatcher = new Mock(); dispatcher.Setup( - d => d.InvokeAsync(It.IsAny(), DispatcherPriority.Render)) - .Callback((Action a, DispatcherPriority _) => a()) - .Returns(Task.CompletedTask); + d => d.Post(It.IsAny(), DispatcherPriority.Render)) + .Callback((Action a, DispatcherPriority _) => a()); var timer = new Mock(); var loop = new RenderLoop(timer.Object, dispatcher.Object); From 4645ec1c4515f193237bbba0a0056c9d5c16ea75 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 28 Sep 2018 20:12:34 +0200 Subject: [PATCH 4/6] Use indexer for iterating items. --- src/Avalonia.Visuals/Rendering/RenderLoop.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/RenderLoop.cs b/src/Avalonia.Visuals/Rendering/RenderLoop.cs index e10e1c4006..df32d9eec1 100644 --- a/src/Avalonia.Visuals/Rendering/RenderLoop.cs +++ b/src/Avalonia.Visuals/Rendering/RenderLoop.cs @@ -96,13 +96,15 @@ namespace Avalonia.Rendering { _dispatcher.Post(() => { - foreach (var i in _items) + for (var i = 0; i < _items.Count; ++i) { - if (i.NeedsUpdate) + var item = _items[i]; + + if (item.NeedsUpdate) { try { - i.Update(time); + item.Update(time); } catch (Exception ex) { From 1df826281bbd61df064bcb334e5f968c2fbedb71 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 29 Sep 2018 16:26:32 +0200 Subject: [PATCH 5/6] Added failing test for #1932. --- .../Primitives/SelectingItemsControlTests.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs index 14e1b15ebc..bbe1d85acb 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs @@ -707,6 +707,26 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.True(target.SelectedIndex == 1); } + [Fact] + public void Binding_With_DelayedBinding_And_Initialization_Where_DataContext_Is_Root_Works() + { + // Test for #1932. + var root = new RootWithItems(); + + root.BeginInit(); + root.DataContext = root; + + var target = new ListBox(); + target.BeginInit(); + root.Child = target; + + DelayedBinding.Add(target, ItemsControl.ItemsProperty, new Binding(nameof(RootWithItems.Items))); + DelayedBinding.Add(target, ListBox.SelectedItemProperty, new Binding(nameof(RootWithItems.Selected))); + target.EndInit(); + root.EndInit(); + + Assert.Equal("b", target.SelectedItem); + } private FuncControlTemplate Template() { @@ -745,5 +765,11 @@ namespace Avalonia.Controls.UnitTests.Primitives public IList Items { get; set; } public Item SelectedItem { get; set; } } + + private class RootWithItems : TestRoot + { + public List Items { get; set; } = new List() { "a", "b", "c", "d", "e" }; + public string Selected { get; set; } = "b"; + } } } From d0b77ecca191fe630f01e8af8eb2945f9f89ce55 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 29 Sep 2018 16:30:21 +0200 Subject: [PATCH 6/6] Update before calling base.EndInit. Fixes #1932. --- src/Avalonia.Controls/Primitives/SelectingItemsControl.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index d7db04f369..f8440aac47 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -289,12 +289,12 @@ namespace Avalonia.Controls.Primitives /// public override void EndInit() { - base.EndInit(); - if (--_updateCount == 0) { UpdateFinished(); } + + base.EndInit(); } ///