From 26b2771864834a9db1bc00852850d145999ca742 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 1 Dec 2017 18:58:20 +0100 Subject: [PATCH 1/6] Added failing test for #1297 --- .../Primitives/TrackTests.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/TrackTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/TrackTests.cs index f07a5f5095..5396a43f3a 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/TrackTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/TrackTests.cs @@ -140,5 +140,31 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Same(thumb.Parent, target); Assert.Equal(new[] { thumb }, ((ILogical)target).LogicalChildren); } + + [Fact] + public void Should_Not_Pass_Invalid_Arrange_Rect() + { + var thumb = new Thumb { Width = 100.873106060606 }; + var increaseButton = new Button { Width = 10 }; + var decreaseButton = new Button { Width = 10 }; + + var target = new Track + { + Height = 12, + Thumb = thumb, + IncreaseButton = increaseButton, + DecreaseButton = decreaseButton, + Orientation = Orientation.Horizontal, + Minimum = 0, + Maximum = 287, + Value = 287, + ViewportSize = 241, + }; + + target.Measure(Size.Infinity); + + // #1297 was occuring here. + target.Arrange(new Rect(0, 0, 221, 12)); + } } } From 91edffd8d20ce93239c33f592793d17643593ba1 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 1 Dec 2017 18:59:12 +0100 Subject: [PATCH 2/6] Fixed #1297 Handle rounding errors when calculating increase button arrange rect. --- src/Avalonia.Controls/Primitives/Track.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Primitives/Track.cs b/src/Avalonia.Controls/Primitives/Track.cs index 8c577665d5..3a16e51851 100644 --- a/src/Avalonia.Controls/Primitives/Track.cs +++ b/src/Avalonia.Controls/Primitives/Track.cs @@ -154,7 +154,11 @@ namespace Avalonia.Controls.Primitives if (increaseButton != null) { - increaseButton.Arrange(new Rect(firstWidth + thumbWidth, 0, remaining - firstWidth, finalSize.Height)); + increaseButton.Arrange(new Rect( + firstWidth + thumbWidth, + 0, + Math.Max(0, remaining - firstWidth), + Math.Max(0, finalSize.Height))); } } else From 4676ac5fb2c87df6244307b2991ab0b8b43f4b7c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 8 Dec 2017 00:57:03 +0100 Subject: [PATCH 3/6] Don't use ObserveOn in Bindings. Instead of using `Observable.ObserveOn` in bindings, interface with `Dispatcher.UIThread` to schedule binding notifications on the UI thread. This saves a significant amount of memory. --- src/Avalonia.Base/AvaloniaObject.cs | 45 +++++++++++------- src/Avalonia.Base/PriorityBindingEntry.cs | 46 +++++++++++++------ .../AvaloniaObjectTests_Direct.cs | 27 +++++++++++ 3 files changed, 88 insertions(+), 30 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 8b93cf2fb1..c76b43f04f 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -325,7 +325,6 @@ namespace Avalonia var description = GetDescription(source); var scheduler = AvaloniaLocator.Current.GetService() ?? ImmediateScheduler.Instance; - source = source.ObserveOn(scheduler); if (property.IsDirect) { @@ -674,29 +673,41 @@ namespace Avalonia /// The value. private void SetDirectValue(AvaloniaProperty property, object value) { - var notification = value as BindingNotification; - - if (notification != null) + void Set() { - notification.LogIfError(this, property); - value = notification.Value; - } + var notification = value as BindingNotification; - if (notification == null || notification.ErrorType == BindingErrorType.Error || notification.HasValue) - { - var metadata = (IDirectPropertyMetadata)property.GetMetadata(GetType()); - var accessor = (IDirectPropertyAccessor)GetRegistered(property); - var finalValue = value == AvaloniaProperty.UnsetValue ? - metadata.UnsetValue : value; + if (notification != null) + { + notification.LogIfError(this, property); + value = notification.Value; + } + + if (notification == null || notification.ErrorType == BindingErrorType.Error || notification.HasValue) + { + var metadata = (IDirectPropertyMetadata)property.GetMetadata(GetType()); + var accessor = (IDirectPropertyAccessor)GetRegistered(property); + var finalValue = value == AvaloniaProperty.UnsetValue ? + metadata.UnsetValue : value; + + LogPropertySet(property, value, BindingPriority.LocalValue); - LogPropertySet(property, value, BindingPriority.LocalValue); + accessor.SetValue(this, finalValue); + } - accessor.SetValue(this, finalValue); + if (notification != null) + { + UpdateDataValidation(property, notification); + } } - if (notification != null) + if (Dispatcher.UIThread.CheckAccess()) + { + Set(); + } + else { - UpdateDataValidation(property, notification); + Dispatcher.UIThread.InvokeAsync(Set); } } diff --git a/src/Avalonia.Base/PriorityBindingEntry.cs b/src/Avalonia.Base/PriorityBindingEntry.cs index 63f582a34c..0e293e4fc6 100644 --- a/src/Avalonia.Base/PriorityBindingEntry.cs +++ b/src/Avalonia.Base/PriorityBindingEntry.cs @@ -3,6 +3,7 @@ using System; using Avalonia.Data; +using Avalonia.Threading; namespace Avalonia { @@ -92,33 +93,52 @@ namespace Avalonia private void ValueChanged(object value) { - _owner.Owner.Owner?.VerifyAccess(); + void Signal() + { + _owner.Owner.Owner?.VerifyAccess(); - var notification = value as BindingNotification; + var notification = value as BindingNotification; - if (notification != null) - { - if (notification.HasValue || notification.ErrorType == BindingErrorType.Error) + if (notification != null) { - Value = notification.Value; - _owner.Changed(this); + if (notification.HasValue || notification.ErrorType == BindingErrorType.Error) + { + Value = notification.Value; + _owner.Changed(this); + } + + if (notification.ErrorType != BindingErrorType.None) + { + _owner.Error(this, notification); + } } - - if (notification.ErrorType != BindingErrorType.None) + else { - _owner.Error(this, notification); + Value = value; + _owner.Changed(this); } } + + if (Dispatcher.UIThread.CheckAccess()) + { + Signal(); + } else { - Value = value; - _owner.Changed(this); + Dispatcher.UIThread.InvokeAsync(Signal); } } private void Completed() { - _owner.Completed(this); + if (Dispatcher.UIThread.CheckAccess()) + { + _owner.Completed(this); + } + else + { + Dispatcher.UIThread.InvokeAsync(() => _owner.Completed(this)); + } } } } diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs index e9cb2bf450..b874a1cbc6 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs @@ -4,10 +4,15 @@ using System; using System.Collections.Generic; using System.Reactive.Subjects; +using System.Threading; +using System.Threading.Tasks; using Avalonia; using Avalonia.Data; using Avalonia.Logging; +using Avalonia.Platform; +using Avalonia.Threading; using Avalonia.UnitTests; +using Moq; using Xunit; namespace Avalonia.Base.UnitTests @@ -411,6 +416,28 @@ namespace Avalonia.Base.UnitTests Assert.True(called); } + [Fact] + public async Task Bind_Executes_On_UIThread() + { + var target = new Class1(); + var source = new Subject(); + var currentThreadId = Thread.CurrentThread.ManagedThreadId; + + var threadingInterfaceMock = new Mock(); + threadingInterfaceMock.SetupGet(mock => mock.CurrentThreadIsLoopThread) + .Returns(() => Thread.CurrentThread.ManagedThreadId == currentThreadId); + + var services = new TestServices( + threadingInterface: threadingInterfaceMock.Object); + + using (UnitTestApplication.Start(services)) + { + target.Bind(Class1.FooProperty, source); + + await Task.Run(() => source.OnNext("foobar")); + } + } + [Fact] public void AddOwner_Should_Inherit_DefaultBindingMode() { From 7405cd79520a4bd19877b8f3f3f033fb49c98e00 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 9 Dec 2017 12:34:14 +0100 Subject: [PATCH 4/6] Match formatting of other block. --- src/Avalonia.Controls/Primitives/Track.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Primitives/Track.cs b/src/Avalonia.Controls/Primitives/Track.cs index 3a16e51851..3f50e11a48 100644 --- a/src/Avalonia.Controls/Primitives/Track.cs +++ b/src/Avalonia.Controls/Primitives/Track.cs @@ -189,7 +189,11 @@ namespace Avalonia.Controls.Primitives if (increaseButton != null) { - increaseButton.Arrange(new Rect(0, firstHeight + thumbHeight, finalSize.Width, Math.Max(remaining - firstHeight, 0))); + increaseButton.Arrange(new Rect( + 0, + firstHeight + thumbHeight, + finalSize.Width, + Math.Max(remaining - firstHeight, 0))); } } From e49e682f9ced0e0b0887350fefbf73d509cde811 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 10 Dec 2017 20:09:20 +0100 Subject: [PATCH 5/6] This Max shouldn't be needed. --- src/Avalonia.Controls/Primitives/Track.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Primitives/Track.cs b/src/Avalonia.Controls/Primitives/Track.cs index 3f50e11a48..648fe5f4b0 100644 --- a/src/Avalonia.Controls/Primitives/Track.cs +++ b/src/Avalonia.Controls/Primitives/Track.cs @@ -158,7 +158,7 @@ namespace Avalonia.Controls.Primitives firstWidth + thumbWidth, 0, Math.Max(0, remaining - firstWidth), - Math.Max(0, finalSize.Height))); + finalSize.Height)); } } else From bf2ca5da720f7783be58dc5403d89f46e83d47f9 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 10 Dec 2017 20:18:51 +0100 Subject: [PATCH 6/6] Removed unneeded code. --- src/Avalonia.Base/AvaloniaObject.cs | 2 -- src/Avalonia.Base/PriorityBindingEntry.cs | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index e547af633e..17e6ea8f0f 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -339,8 +339,6 @@ namespace Avalonia var description = GetDescription(source); - var scheduler = AvaloniaLocator.Current.GetService() ?? ImmediateScheduler.Instance; - if (property.IsDirect) { if (property.IsReadOnly) diff --git a/src/Avalonia.Base/PriorityBindingEntry.cs b/src/Avalonia.Base/PriorityBindingEntry.cs index 0e293e4fc6..b44b845f25 100644 --- a/src/Avalonia.Base/PriorityBindingEntry.cs +++ b/src/Avalonia.Base/PriorityBindingEntry.cs @@ -95,8 +95,6 @@ namespace Avalonia { void Signal() { - _owner.Owner.Owner?.VerifyAccess(); - var notification = value as BindingNotification; if (notification != null)