From d8615643dd9b0ec242c943da22c88372e0fb8b02 Mon Sep 17 00:00:00 2001 From: Karnah Date: Thu, 21 Jun 2018 12:47:38 +0500 Subject: [PATCH 01/24] Fixed NullReferenceException for ImageBush with null Source --- .../Media/DrawingContextImpl.cs | 2 +- .../Media/ImageBrushTests.cs | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/Windows/Avalonia.Direct2D1/Media/DrawingContextImpl.cs b/src/Windows/Avalonia.Direct2D1/Media/DrawingContextImpl.cs index bdbbdab2b9..479db51be4 100644 --- a/src/Windows/Avalonia.Direct2D1/Media/DrawingContextImpl.cs +++ b/src/Windows/Avalonia.Direct2D1/Media/DrawingContextImpl.cs @@ -394,7 +394,7 @@ namespace Avalonia.Direct2D1.Media { return new RadialGradientBrushImpl(radialGradientBrush, _renderTarget, destinationSize); } - else if (imageBrush != null) + else if (imageBrush?.Source != null) { return new ImageBrushImpl( imageBrush, diff --git a/tests/Avalonia.RenderTests/Media/ImageBrushTests.cs b/tests/Avalonia.RenderTests/Media/ImageBrushTests.cs index 0107002274..d1f51a1a0c 100644 --- a/tests/Avalonia.RenderTests/Media/ImageBrushTests.cs +++ b/tests/Avalonia.RenderTests/Media/ImageBrushTests.cs @@ -32,6 +32,23 @@ namespace Avalonia.Direct2D1.RenderTests.Media get { return System.IO.Path.Combine(OutputPath, "github_icon_small.png"); } } + [Fact] + public async Task ImageBrush_NullSource() + { + Decorator target = new Decorator + { + Width = 200, + Height = 200, + Child = new Rectangle + { + Margin = new Thickness(8), + Fill = new ImageBrush() + } + }; + + await RenderToFile(target); + } + [Fact] public async Task ImageBrush_Tile_Fill() { From 48dd3b94d1ad0d1ea8dbc2c4e6c859196862c821 Mon Sep 17 00:00:00 2001 From: Karnah Date: Tue, 26 Jun 2018 11:28:45 +0500 Subject: [PATCH 02/24] Using transparent brush when image source is null --- src/Skia/Avalonia.Skia/DrawingContextImpl.cs | 4 ++++ .../Avalonia.RenderTests/Media/ImageBrushTests.cs | 1 + .../ImageBrush/ImageBrush_NullSource.expected.png | Bin 0 -> 250 bytes .../ImageBrush/ImageBrush_NullSource.expected.png | Bin 0 -> 250 bytes 4 files changed, 5 insertions(+) create mode 100644 tests/TestFiles/Direct2D1/Media/ImageBrush/ImageBrush_NullSource.expected.png create mode 100644 tests/TestFiles/Skia/Media/ImageBrush/ImageBrush_NullSource.expected.png diff --git a/src/Skia/Avalonia.Skia/DrawingContextImpl.cs b/src/Skia/Avalonia.Skia/DrawingContextImpl.cs index b7ce6eedc4..d2c4bd0aa1 100644 --- a/src/Skia/Avalonia.Skia/DrawingContextImpl.cs +++ b/src/Skia/Avalonia.Skia/DrawingContextImpl.cs @@ -491,6 +491,10 @@ namespace Avalonia.Skia { ConfigureTileBrush(ref paintWrapper, targetSize, tileBrush, tileBrushImage); } + else + { + paint.Color = new SKColor(255, 255, 255, 0); + } return paintWrapper; } diff --git a/tests/Avalonia.RenderTests/Media/ImageBrushTests.cs b/tests/Avalonia.RenderTests/Media/ImageBrushTests.cs index d1f51a1a0c..3a41585d04 100644 --- a/tests/Avalonia.RenderTests/Media/ImageBrushTests.cs +++ b/tests/Avalonia.RenderTests/Media/ImageBrushTests.cs @@ -47,6 +47,7 @@ namespace Avalonia.Direct2D1.RenderTests.Media }; await RenderToFile(target); + CompareImages(); } [Fact] diff --git a/tests/TestFiles/Direct2D1/Media/ImageBrush/ImageBrush_NullSource.expected.png b/tests/TestFiles/Direct2D1/Media/ImageBrush/ImageBrush_NullSource.expected.png new file mode 100644 index 0000000000000000000000000000000000000000..4c9409ee63e2d96335b03121247027d66340b190 GIT binary patch literal 250 zcmeAS@N?(olHy`uVBq!ia0vp^CqS5k4M?tyST_$yu@pObhHwBu4M$1`kiXH>#WAE} r&f9~Gj6hzK!vFPo9u6SpAO$s&h0homeHw~bKo0bD^>bP0l+XkKkl+`x literal 0 HcmV?d00001 diff --git a/tests/TestFiles/Skia/Media/ImageBrush/ImageBrush_NullSource.expected.png b/tests/TestFiles/Skia/Media/ImageBrush/ImageBrush_NullSource.expected.png new file mode 100644 index 0000000000000000000000000000000000000000..4c9409ee63e2d96335b03121247027d66340b190 GIT binary patch literal 250 zcmeAS@N?(olHy`uVBq!ia0vp^CqS5k4M?tyST_$yu@pObhHwBu4M$1`kiXH>#WAE} r&f9~Gj6hzK!vFPo9u6SpAO$s&h0homeHw~bKo0bD^>bP0l+XkKkl+`x literal 0 HcmV?d00001 From 6d4694b471a7f2a38520d7ac8540c5cb4df78b28 Mon Sep 17 00:00:00 2001 From: CommonGuy Date: Mon, 2 Jul 2018 09:00:32 +0200 Subject: [PATCH 03/24] Add checks for null in Windows SystemDialogImpl --- src/Windows/Avalonia.Win32/SystemDialogImpl.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Windows/Avalonia.Win32/SystemDialogImpl.cs b/src/Windows/Avalonia.Win32/SystemDialogImpl.cs index d555e93a88..8a5617789b 100644 --- a/src/Windows/Avalonia.Win32/SystemDialogImpl.cs +++ b/src/Windows/Avalonia.Win32/SystemDialogImpl.cs @@ -37,13 +37,16 @@ namespace Avalonia.Win32 var defaultExtension = (dialog as SaveFileDialog)?.DefaultExtension ?? ""; frm.SetDefaultExtension(defaultExtension); frm.SetFileName(dialog.InitialFileName ?? ""); - frm.SetTitle(dialog.Title); + frm.SetTitle(dialog.Title ?? ""); var filters = new List(); - foreach (var filter in dialog.Filters) + if (dialog.Filters != null) { - var extMask = string.Join(";", filter.Extensions.Select(e => "*." + e)); - filters.Add(new UnmanagedMethods.COMDLG_FILTERSPEC { pszName = filter.Name, pszSpec = extMask }); + foreach (var filter in dialog.Filters) + { + var extMask = string.Join(";", filter.Extensions.Select(e => "*." + e)); + filters.Add(new UnmanagedMethods.COMDLG_FILTERSPEC { pszName = filter.Name, pszSpec = extMask }); + } } if (filters.Count == 0) filters.Add(new UnmanagedMethods.COMDLG_FILTERSPEC { pszName = "All files", pszSpec = "*.*" }); From 54a919a7284894f6ed99e28097ea97d45721ce7f Mon Sep 17 00:00:00 2001 From: CommonGuy Date: Mon, 2 Jul 2018 09:16:45 +0200 Subject: [PATCH 04/24] Ensure that selected items in Windows FileDialog are filesystem items --- src/Windows/Avalonia.Win32/SystemDialogImpl.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Windows/Avalonia.Win32/SystemDialogImpl.cs b/src/Windows/Avalonia.Win32/SystemDialogImpl.cs index 8a5617789b..020e6aa0ea 100644 --- a/src/Windows/Avalonia.Win32/SystemDialogImpl.cs +++ b/src/Windows/Avalonia.Win32/SystemDialogImpl.cs @@ -13,6 +13,9 @@ namespace Avalonia.Win32 class SystemDialogImpl : ISystemDialogImpl { + private const UnmanagedMethods.FOS DefaultDialogOptions = UnmanagedMethods.FOS.FOS_PICKFOLDERS | UnmanagedMethods.FOS.FOS_FORCEFILESYSTEM | UnmanagedMethods.FOS.FOS_NOVALIDATE | + UnmanagedMethods.FOS.FOS_NOTESTFILECREATE | UnmanagedMethods.FOS.FOS_DONTADDTORECENT; + public unsafe Task ShowFileDialogAsync(FileDialog dialog, IWindowImpl parent) { var hWnd = parent?.Handle?.Handle ?? IntPtr.Zero; @@ -29,7 +32,7 @@ namespace Avalonia.Win32 uint options; frm.GetOptions(out options); - options |= (uint)(UnmanagedMethods.FOS.FOS_NOVALIDATE | UnmanagedMethods.FOS.FOS_NOTESTFILECREATE | UnmanagedMethods.FOS.FOS_DONTADDTORECENT); + options |= (uint)(DefaultDialogOptions); if (openDialog?.AllowMultiple == true) options |= (uint)UnmanagedMethods.FOS.FOS_ALLOWMULTISELECT; frm.SetOptions(options); @@ -109,7 +112,7 @@ namespace Avalonia.Win32 var frm = (UnmanagedMethods.IFileDialog)unk; uint options; frm.GetOptions(out options); - options |= (uint)(UnmanagedMethods.FOS.FOS_PICKFOLDERS | UnmanagedMethods.FOS.FOS_FORCEFILESYSTEM | UnmanagedMethods.FOS.FOS_NOVALIDATE | UnmanagedMethods.FOS.FOS_NOTESTFILECREATE | UnmanagedMethods.FOS.FOS_DONTADDTORECENT); + options |= (uint)(UnmanagedMethods.FOS.FOS_PICKFOLDERS | DefaultDialogOptions); frm.SetOptions(options); if (dialog.InitialDirectory != null) From 8ab89b1c3581eccd041fe652e2b87c1cb1bcb5bd Mon Sep 17 00:00:00 2001 From: CommonGuy Date: Mon, 2 Jul 2018 17:24:57 +0200 Subject: [PATCH 05/24] FOS_PICKFOLDERS should not be a default options for Windows SystemDialogs --- src/Windows/Avalonia.Win32/SystemDialogImpl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Windows/Avalonia.Win32/SystemDialogImpl.cs b/src/Windows/Avalonia.Win32/SystemDialogImpl.cs index 020e6aa0ea..dec5e3a544 100644 --- a/src/Windows/Avalonia.Win32/SystemDialogImpl.cs +++ b/src/Windows/Avalonia.Win32/SystemDialogImpl.cs @@ -13,7 +13,7 @@ namespace Avalonia.Win32 class SystemDialogImpl : ISystemDialogImpl { - private const UnmanagedMethods.FOS DefaultDialogOptions = UnmanagedMethods.FOS.FOS_PICKFOLDERS | UnmanagedMethods.FOS.FOS_FORCEFILESYSTEM | UnmanagedMethods.FOS.FOS_NOVALIDATE | + private const UnmanagedMethods.FOS DefaultDialogOptions = UnmanagedMethods.FOS.FOS_FORCEFILESYSTEM | UnmanagedMethods.FOS.FOS_NOVALIDATE | UnmanagedMethods.FOS.FOS_NOTESTFILECREATE | UnmanagedMethods.FOS.FOS_DONTADDTORECENT; public unsafe Task ShowFileDialogAsync(FileDialog dialog, IWindowImpl parent) From f11f5bc154aeea7e4ac6a3f35254ba3e54623588 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 3 Jul 2018 15:19:16 -0500 Subject: [PATCH 06/24] Get rid of static deferredSetter used for PriorityValues. Use a single DeferredSetter for both styled and direct properties. --- src/Avalonia.Base/AvaloniaObject.cs | 20 +++++++++----------- src/Avalonia.Base/IPriorityValueOwner.cs | 3 +++ src/Avalonia.Base/PriorityValue.cs | 8 +++----- src/Avalonia.Base/ValueStore.cs | 20 +++++++++++++++----- 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 761c0618da..9fd2acbe5f 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -22,7 +22,7 @@ namespace Avalonia /// /// This class is analogous to DependencyObject in WPF. /// - public class AvaloniaObject : IAvaloniaObject, IAvaloniaObjectDebug, INotifyPropertyChanged, IPriorityValueOwner + public class AvaloniaObject : IAvaloniaObject, IAvaloniaObjectDebug, INotifyPropertyChanged { /// /// The parent object that inherited values are inherited from. @@ -414,9 +414,8 @@ namespace Avalonia VerifyAccess(); _values?.Revalidate(property); } - - /// - void IPriorityValueOwner.Changed(AvaloniaProperty property, int priority, object oldValue, object newValue) + + internal void PriorityValueChanged(AvaloniaProperty property, int priority, object oldValue, object newValue) { oldValue = (oldValue == AvaloniaProperty.UnsetValue) ? GetDefaultValue(property) : @@ -439,9 +438,8 @@ namespace Avalonia (BindingPriority)priority); } } - - /// - void IPriorityValueOwner.BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification) + + internal void BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification) { UpdateDataValidation(property, notification); } @@ -566,15 +564,15 @@ namespace Avalonia T value) { Contract.Requires(setterCallback != null); - return DirectPropertyDeferredSetter.SetAndNotify( + return _values.Setter.SetAndNotify( property, ref field, - (object val, ref T backing, Action notify) => + ((object value, int) update, ref T backing, Action notify) => { - setterCallback((T)val, ref backing, notify); + setterCallback((T)update.value, ref backing, notify); return true; }, - value); + (value, 0)); } /// diff --git a/src/Avalonia.Base/IPriorityValueOwner.cs b/src/Avalonia.Base/IPriorityValueOwner.cs index 5f63f6ef91..3a931d6b35 100644 --- a/src/Avalonia.Base/IPriorityValueOwner.cs +++ b/src/Avalonia.Base/IPriorityValueOwner.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using Avalonia.Data; +using Avalonia.Utilities; namespace Avalonia { @@ -31,5 +32,7 @@ namespace Avalonia /// Ensures that the current thread is the UI thread. /// void VerifyAccess(); + + DeferredSetter Setter { get; } } } diff --git a/src/Avalonia.Base/PriorityValue.cs b/src/Avalonia.Base/PriorityValue.cs index c474f9098e..bc09a41e5f 100644 --- a/src/Avalonia.Base/PriorityValue.cs +++ b/src/Avalonia.Base/PriorityValue.cs @@ -21,7 +21,7 @@ namespace Avalonia /// priority binding that doesn't return . Where there /// are multiple bindings registered with the same priority, the most recently added binding /// has a higher priority. Each time the value changes, the - /// method on the + /// method on the /// owner object is fired with the old and new values. /// internal class PriorityValue @@ -30,7 +30,6 @@ namespace Avalonia private readonly SingleOrDictionary _levels = new SingleOrDictionary(); private readonly Func _validate; - private static readonly DeferredSetter delayedSetter = new DeferredSetter(); private (object value, int priority) _value; /// @@ -243,7 +242,7 @@ namespace Avalonia /// The priority level that the value came from. private void UpdateValue(object value, int priority) { - delayedSetter.SetAndNotify(this, + Owner.Setter.SetAndNotify(Property, ref _value, UpdateCore, (value, priority)); @@ -256,14 +255,13 @@ namespace Avalonia { var val = update.value; var notification = val as BindingNotification; - object castValue; if (notification != null) { val = (notification.HasValue) ? notification.Value : null; } - if (TypeUtilities.TryConvertImplicit(_valueType, val, out castValue)) + if (TypeUtilities.TryConvertImplicit(_valueType, val, out object castValue)) { var old = backing.value; diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 8283edab80..5e34e8a43f 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using Avalonia.Data; +using Avalonia.Utilities; namespace Avalonia { @@ -91,12 +92,12 @@ namespace Avalonia public void BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification) { - ((IPriorityValueOwner)_owner).BindingNotificationReceived(property, notification); + _owner.BindingNotificationReceived(property, notification); } public void Changed(AvaloniaProperty property, int priority, object oldValue, object newValue) { - ((IPriorityValueOwner)_owner).Changed(property, priority, oldValue, newValue); + _owner.PriorityValueChanged(property, priority, oldValue, newValue); } public IDictionary GetSetValues() => throw new NotImplementedException(); @@ -148,13 +149,11 @@ namespace Avalonia validate2 = v => validate(_owner, v); } - PriorityValue result = new PriorityValue( + return new PriorityValue( this, property, property.PropertyType, validate2); - - return result; } private object Validate(AvaloniaProperty property, object value) @@ -168,5 +167,16 @@ namespace Avalonia return value; } + + private DeferredSetter _defferedSetter; + + public DeferredSetter Setter + { + get + { + return _defferedSetter ?? + (_defferedSetter = new DeferredSetter()); + } + } } } From 00f8dfabc050d5e65071062e5bc18ebd4e3c73bf Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 3 Jul 2018 16:09:23 -0500 Subject: [PATCH 07/24] Use SingleOrQueue instead of Queue in DeferredSetter. Make sure tests pass. --- src/Avalonia.Base/AvaloniaObject.cs | 49 +++++------------ src/Avalonia.Base/IPriorityValueOwner.cs | 2 +- src/Avalonia.Base/PriorityValue.cs | 9 +++- src/Avalonia.Base/Utilities/DeferredSetter.cs | 9 ++-- src/Avalonia.Base/Utilities/SingleOrQueue.cs | 54 +++++++++++++++++++ src/Avalonia.Base/ValueStore.cs | 8 +-- .../PriorityValueTests.cs | 52 ++++++++++-------- .../Utilities/SingleOrQueueTests.cs | 50 +++++++++++++++++ 8 files changed, 165 insertions(+), 68 deletions(-) create mode 100644 src/Avalonia.Base/Utilities/SingleOrQueue.cs create mode 100644 tests/Avalonia.Base.UnitTests/Utilities/SingleOrQueueTests.cs diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 9fd2acbe5f..db4e52ee0f 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -45,21 +45,8 @@ namespace Avalonia /// private EventHandler _propertyChanged; - private DeferredSetter _directDeferredSetter; private ValueStore _values; - - /// - /// Delayed setter helper for direct properties. Used to fix #855. - /// - private DeferredSetter DirectPropertyDeferredSetter - { - get - { - return _directDeferredSetter ?? - (_directDeferredSetter = new DeferredSetter()); - } - } - + private ValueStore Values => _values ?? (_values = new ValueStore(this)); /// /// Initializes a new instance of the class. @@ -223,9 +210,9 @@ namespace Avalonia { return ((IDirectPropertyAccessor)GetRegistered(property)).GetValue(this); } - else if (_values != null) + else if (Values != null) { - var result = _values.GetValue(property); + var result = Values.GetValue(property); if (result == AvaloniaProperty.UnsetValue) { @@ -263,7 +250,7 @@ namespace Avalonia Contract.Requires(property != null); VerifyAccess(); - return _values?.IsAnimating(property) ?? false; + return Values?.IsAnimating(property) ?? false; } /// @@ -280,7 +267,7 @@ namespace Avalonia Contract.Requires(property != null); VerifyAccess(); - return _values?.IsSet(property) ?? false; + return Values?.IsSet(property) ?? false; } /// @@ -376,12 +363,7 @@ namespace Avalonia description, priority); - if (_values == null) - { - _values = new ValueStore(this); - } - - return _values.AddBinding(property, source, priority); + return Values.AddBinding(property, source, priority); } } @@ -412,7 +394,7 @@ namespace Avalonia public void Revalidate(AvaloniaProperty property) { VerifyAccess(); - _values?.Revalidate(property); + Values?.Revalidate(property); } internal void PriorityValueChanged(AvaloniaProperty property, int priority, object oldValue, object newValue) @@ -454,7 +436,7 @@ namespace Avalonia /// Gets all priority values set on the object. /// /// A collection of property/value tuples. - internal IDictionary GetSetValues() => _values?.GetSetValues(); + internal IDictionary GetSetValues() => Values?.GetSetValues(); /// /// Forces revalidation of properties when a property value changes. @@ -564,15 +546,15 @@ namespace Avalonia T value) { Contract.Requires(setterCallback != null); - return _values.Setter.SetAndNotify( + return Values.Setter.SetAndNotify( property, ref field, - ((object value, int) update, ref T backing, Action notify) => + (object update, ref T backing, Action notify) => { - setterCallback((T)update.value, ref backing, notify); + setterCallback((T)update, ref backing, notify); return true; }, - (value, 0)); + value); } /// @@ -735,13 +717,8 @@ namespace Avalonia originalValue?.GetType().FullName ?? "(null)")); } - if (_values == null) - { - _values = new ValueStore(this); - } - LogPropertySet(property, value, priority); - _values.AddValue(property, value, (int)priority); + Values.AddValue(property, value, (int)priority); } /// diff --git a/src/Avalonia.Base/IPriorityValueOwner.cs b/src/Avalonia.Base/IPriorityValueOwner.cs index 3a931d6b35..8eaefac4f2 100644 --- a/src/Avalonia.Base/IPriorityValueOwner.cs +++ b/src/Avalonia.Base/IPriorityValueOwner.cs @@ -33,6 +33,6 @@ namespace Avalonia /// void VerifyAccess(); - DeferredSetter Setter { get; } + DeferredSetter Setter { get; } } } diff --git a/src/Avalonia.Base/PriorityValue.cs b/src/Avalonia.Base/PriorityValue.cs index bc09a41e5f..03094e2236 100644 --- a/src/Avalonia.Base/PriorityValue.cs +++ b/src/Avalonia.Base/PriorityValue.cs @@ -248,6 +248,12 @@ namespace Avalonia (value, priority)); } + private bool UpdateCore( + object update, + ref (object value, int priority) backing, + Action notify) + => UpdateCore(((object, int))update, ref backing, notify); + private bool UpdateCore( (object value, int priority) update, ref (object value, int priority) backing, @@ -255,13 +261,14 @@ namespace Avalonia { var val = update.value; var notification = val as BindingNotification; + object castValue; if (notification != null) { val = (notification.HasValue) ? notification.Value : null; } - if (TypeUtilities.TryConvertImplicit(_valueType, val, out object castValue)) + if (TypeUtilities.TryConvertImplicit(_valueType, val, out castValue)) { var old = backing.value; diff --git a/src/Avalonia.Base/Utilities/DeferredSetter.cs b/src/Avalonia.Base/Utilities/DeferredSetter.cs index fdfa160134..9e3cf023c7 100644 --- a/src/Avalonia.Base/Utilities/DeferredSetter.cs +++ b/src/Avalonia.Base/Utilities/DeferredSetter.cs @@ -8,6 +8,7 @@ namespace Avalonia.Utilities { /// /// A utility class to enable deferring assignment until after property-changed notifications are sent. + /// Used to fix #855. /// /// The type of the object that represents the property. /// The type of value with which to track the delayed assignment. @@ -37,13 +38,13 @@ namespace Avalonia.Utilities { public bool Notifying { get; set; } - private Queue pendingValues; + private SingleOrQueue pendingValues; - public Queue PendingValues + public SingleOrQueue PendingValues { get { - return pendingValues ?? (pendingValues = new Queue()); + return pendingValues ?? (pendingValues = new SingleOrQueue()); } } } @@ -89,7 +90,7 @@ namespace Avalonia.Utilities /// If the property has any pending assignments. private bool HasPendingSet(TProperty property) { - return setRecords.TryGetValue(property, out var status) && status.PendingValues.Count != 0; + return setRecords.TryGetValue(property, out var status) && !status.PendingValues.Empty; } /// diff --git a/src/Avalonia.Base/Utilities/SingleOrQueue.cs b/src/Avalonia.Base/Utilities/SingleOrQueue.cs new file mode 100644 index 0000000000..f33adaff37 --- /dev/null +++ b/src/Avalonia.Base/Utilities/SingleOrQueue.cs @@ -0,0 +1,54 @@ +using System; +using System.Collections.Generic; +using System.Text; + +namespace Avalonia.Utilities +{ + public class SingleOrQueue + { + private T _head; + private Queue _tail; + + private Queue Tail => _tail ?? (_tail = new Queue()); + + private bool HasTail => _tail != null; + + public bool Empty { get; private set; } = true; + + public void Enqueue(T value) + { + if (Empty) + { + _head = value; + } + else + { + Tail.Enqueue(value); + } + + Empty = false; + } + + public T Dequeue() + { + if (Empty) + { + throw new InvalidOperationException("Cannot dequeue from an empty queue!"); + } + + var result = _head; + + if (HasTail && Tail.Count != 0) + { + _head = Tail.Dequeue(); + } + else + { + _head = default; + Empty = true; + } + + return result; + } + } +} diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 5e34e8a43f..0e404500a1 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -116,7 +116,7 @@ namespace Avalonia public bool IsAnimating(AvaloniaProperty property) { - return _values.TryGetValue(property, out var value) ? (value as PriorityValue)?.IsAnimating ?? false : false; + return _values.TryGetValue(property, out var value) && value is PriorityValue priority && priority.IsAnimating; } public bool IsSet(AvaloniaProperty property) @@ -168,14 +168,14 @@ namespace Avalonia return value; } - private DeferredSetter _defferedSetter; + private DeferredSetter _defferedSetter; - public DeferredSetter Setter + public DeferredSetter Setter { get { return _defferedSetter ?? - (_defferedSetter = new DeferredSetter()); + (_defferedSetter = new DeferredSetter()); } } } diff --git a/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs b/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs index d6650aa104..61ba9bc463 100644 --- a/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs +++ b/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs @@ -1,11 +1,12 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. +using Avalonia.Utilities; +using Moq; using System; using System.Linq; using System.Reactive.Linq; using System.Reactive.Subjects; -using Moq; using Xunit; namespace Avalonia.Base.UnitTests @@ -21,7 +22,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Initial_Value_Should_Be_UnsetValue() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); Assert.Same(AvaloniaProperty.UnsetValue, target.Value); } @@ -29,7 +30,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void First_Binding_Sets_Value() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); target.Add(Single("foo"), 0); @@ -39,7 +40,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Changing_Binding_Should_Set_Value() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var subject = new BehaviorSubject("foo"); target.Add(subject, 0); @@ -51,7 +52,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Setting_Direct_Value_Should_Override_Binding() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); target.Add(Single("foo"), 0); target.SetValue("bar", 0); @@ -62,7 +63,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Binding_Firing_Should_Override_Direct_Value() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var source = new BehaviorSubject("initial"); target.Add(source, 0); @@ -76,7 +77,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Earlier_Binding_Firing_Should_Not_Override_Later() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var nonActive = new BehaviorSubject("na"); var source = new BehaviorSubject("initial"); @@ -92,7 +93,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Binding_Completing_Should_Revert_To_Direct_Value() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var source = new BehaviorSubject("initial"); target.Add(source, 0); @@ -108,7 +109,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Binding_With_Lower_Priority_Has_Precedence() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); target.Add(Single("foo"), 1); target.Add(Single("bar"), 0); @@ -120,7 +121,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Later_Binding_With_Same_Priority_Should_Take_Precedence() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); target.Add(Single("foo"), 1); target.Add(Single("bar"), 0); @@ -133,7 +134,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Changing_Binding_With_Lower_Priority_Should_Set_Not_Value() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var subject = new BehaviorSubject("bar"); target.Add(Single("foo"), 0); @@ -146,7 +147,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void UnsetValue_Should_Fall_Back_To_Next_Binding() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var subject = new BehaviorSubject("bar"); target.Add(subject, 0); @@ -162,7 +163,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Adding_Value_Should_Call_OnNext() { - var owner = new Mock(); + var owner = GetMockOwner(); var target = new PriorityValue(owner.Object, TestProperty, typeof(string)); target.Add(Single("foo"), 0); @@ -173,7 +174,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Changing_Value_Should_Call_OnNext() { - var owner = new Mock(); + var owner = GetMockOwner(); var target = new PriorityValue(owner.Object, TestProperty, typeof(string)); var subject = new BehaviorSubject("foo"); @@ -186,7 +187,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Disposing_A_Binding_Should_Revert_To_Next_Value() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); target.Add(Single("foo"), 0); var disposable = target.Add(Single("bar"), 0); @@ -199,7 +200,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Disposing_A_Binding_Should_Remove_BindingEntry() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); target.Add(Single("foo"), 0); var disposable = target.Add(Single("bar"), 0); @@ -212,7 +213,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Completing_A_Binding_Should_Revert_To_Previous_Binding() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var source = new BehaviorSubject("bar"); target.Add(Single("foo"), 0); @@ -226,7 +227,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Completing_A_Binding_Should_Revert_To_Lower_Priority() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var source = new BehaviorSubject("bar"); target.Add(Single("foo"), 1); @@ -240,7 +241,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Completing_A_Binding_Should_Remove_BindingEntry() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var subject = new BehaviorSubject("bar"); target.Add(Single("foo"), 0); @@ -254,7 +255,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Direct_Value_Should_Be_Coerced() { - var target = new PriorityValue(null, TestProperty, typeof(int), x => Math.Min((int)x, 10)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(int), x => Math.Min((int)x, 10)); target.SetValue(5, 0); Assert.Equal(5, target.Value); @@ -265,7 +266,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void Bound_Value_Should_Be_Coerced() { - var target = new PriorityValue(null, TestProperty, typeof(int), x => Math.Min((int)x, 10)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(int), x => Math.Min((int)x, 10)); var source = new Subject(); target.Add(source, 0); @@ -279,7 +280,7 @@ namespace Avalonia.Base.UnitTests public void Revalidate_Should_ReCoerce_Value() { var max = 10; - var target = new PriorityValue(null, TestProperty, typeof(int), x => Math.Min((int)x, max)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(int), x => Math.Min((int)x, max)); var source = new Subject(); target.Add(source, 0); @@ -302,5 +303,12 @@ namespace Avalonia.Base.UnitTests { return Observable.Never().StartWith(value); } + + private static Mock GetMockOwner() + { + var owner = new Mock(); + owner.SetupGet(o => o.Setter).Returns(new DeferredSetter()); + return owner; + } } } diff --git a/tests/Avalonia.Base.UnitTests/Utilities/SingleOrQueueTests.cs b/tests/Avalonia.Base.UnitTests/Utilities/SingleOrQueueTests.cs new file mode 100644 index 0000000000..f1ab265bc9 --- /dev/null +++ b/tests/Avalonia.Base.UnitTests/Utilities/SingleOrQueueTests.cs @@ -0,0 +1,50 @@ +using Avalonia.Utilities; +using System; +using System.Collections.Generic; +using System.Text; +using Xunit; + +namespace Avalonia.Base.UnitTests.Utilities +{ + public class SingleOrQueueTests + { + [Fact] + public void New_SingleOrQueue_Is_Empty() + { + Assert.True(new SingleOrQueue().Empty); + } + + [Fact] + public void Dequeue_Throws_When_Empty() + { + var queue = new SingleOrQueue(); + + Assert.Throws(() => queue.Dequeue()); + } + + [Fact] + public void Enqueue_Adds_Element() + { + var queue = new SingleOrQueue(); + + queue.Enqueue(1); + + Assert.False(queue.Empty); + + Assert.Equal(1, queue.Dequeue()); + } + + [Fact] + public void Multiple_Elements_Dequeued_In_Correct_Order() + { + var queue = new SingleOrQueue(); + + queue.Enqueue(1); + queue.Enqueue(2); + queue.Enqueue(3); + Assert.Equal(1, queue.Dequeue()); + Assert.Equal(2, queue.Dequeue()); + Assert.Equal(3, queue.Dequeue()); + } + } +} From 1ad7f27cf9a559ac4b51aa6d7cd8ffb6a769c5cb Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 3 Jul 2018 17:01:51 -0500 Subject: [PATCH 08/24] Make DeferredSetter tied to AvaloniaProperty and use a regular Dictionary. Remove SettingStatus entries for bindings from the dictionary for bindings that don't create a queue of pending values. --- src/Avalonia.Base/IPriorityValueOwner.cs | 2 +- src/Avalonia.Base/Utilities/DeferredSetter.cs | 58 ++++++++++++++----- src/Avalonia.Base/Utilities/SingleOrQueue.cs | 2 +- src/Avalonia.Base/ValueStore.cs | 6 +- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/src/Avalonia.Base/IPriorityValueOwner.cs b/src/Avalonia.Base/IPriorityValueOwner.cs index 8eaefac4f2..8cbf212381 100644 --- a/src/Avalonia.Base/IPriorityValueOwner.cs +++ b/src/Avalonia.Base/IPriorityValueOwner.cs @@ -33,6 +33,6 @@ namespace Avalonia /// void VerifyAccess(); - DeferredSetter Setter { get; } + DeferredSetter Setter { get; } } } diff --git a/src/Avalonia.Base/Utilities/DeferredSetter.cs b/src/Avalonia.Base/Utilities/DeferredSetter.cs index 9e3cf023c7..15c85338c7 100644 --- a/src/Avalonia.Base/Utilities/DeferredSetter.cs +++ b/src/Avalonia.Base/Utilities/DeferredSetter.cs @@ -10,10 +10,8 @@ namespace Avalonia.Utilities /// A utility class to enable deferring assignment until after property-changed notifications are sent. /// Used to fix #855. /// - /// The type of the object that represents the property. /// The type of value with which to track the delayed assignment. - class DeferredSetter - where TProperty: class + class DeferredSetter { private struct NotifyDisposable : IDisposable { @@ -47,20 +45,37 @@ namespace Avalonia.Utilities return pendingValues ?? (pendingValues = new SingleOrQueue()); } } + + public bool IsSimpleSet => pendingValues?.HasTail != true; } - private readonly ConditionalWeakTable setRecords = new ConditionalWeakTable(); + private Dictionary _setRecords; + private Dictionary SetRecords + => _setRecords ?? (_setRecords = new Dictionary()); + + private SettingStatus GetOrCreateStatus(AvaloniaProperty property) + { + if (!SetRecords.TryGetValue(property, out var status)) + { + status = new SettingStatus(); + SetRecords.Add(property, status); + } + + return status; + } /// /// Mark the property as currently notifying. /// /// The property to mark as notifying. /// Returns a disposable that when disposed, marks the property as done notifying. - private NotifyDisposable MarkNotifying(TProperty property) + private NotifyDisposable MarkNotifying(AvaloniaProperty property) { Contract.Requires(!IsNotifying(property)); - - return new NotifyDisposable(setRecords.GetOrCreateValue(property)); + + SettingStatus status = GetOrCreateStatus(property); + + return new NotifyDisposable(status); } /// @@ -68,19 +83,19 @@ namespace Avalonia.Utilities /// /// The property. /// If the property is currently notifying listeners. - private bool IsNotifying(TProperty property) - => setRecords.TryGetValue(property, out var value) && value.Notifying; + private bool IsNotifying(AvaloniaProperty property) + => SetRecords.TryGetValue(property, out var value) && value.Notifying; /// /// Add a pending assignment for the property. /// /// The property. /// The value to assign. - private void AddPendingSet(TProperty property, TSetRecord value) + private void AddPendingSet(AvaloniaProperty property, TSetRecord value) { Contract.Requires(IsNotifying(property)); - setRecords.GetOrCreateValue(property).PendingValues.Enqueue(value); + GetOrCreateStatus(property).PendingValues.Enqueue(value); } /// @@ -88,9 +103,9 @@ namespace Avalonia.Utilities /// /// The property to check. /// If the property has any pending assignments. - private bool HasPendingSet(TProperty property) + private bool HasPendingSet(AvaloniaProperty property) { - return setRecords.TryGetValue(property, out var status) && !status.PendingValues.Empty; + return SetRecords.TryGetValue(property, out var status) && !status.PendingValues.Empty; } /// @@ -98,9 +113,17 @@ namespace Avalonia.Utilities /// /// The property to check. /// The first pending assignment for the property. - private TSetRecord GetFirstPendingSet(TProperty property) + private TSetRecord GetFirstPendingSet(AvaloniaProperty property) + { + return GetOrCreateStatus(property).PendingValues.Dequeue(); + } + + private void CleanupSetStatus(AvaloniaProperty property) { - return setRecords.GetOrCreateValue(property).PendingValues.Dequeue(); + if (SetRecords.TryGetValue(property, out var status) && status.IsSimpleSet) + { + SetRecords.Remove(property); + } } public delegate bool SetterDelegate(TSetRecord record, ref TValue backing, Action notifyCallback); @@ -116,7 +139,7 @@ namespace Avalonia.Utilities /// /// The value to try to set. public bool SetAndNotify( - TProperty property, + AvaloniaProperty property, ref TValue backing, SetterDelegate setterCallback, TSetRecord value) @@ -145,6 +168,9 @@ namespace Avalonia.Utilities } }); } + + CleanupSetStatus(property); + return updated; } else if(!object.Equals(value, backing)) diff --git a/src/Avalonia.Base/Utilities/SingleOrQueue.cs b/src/Avalonia.Base/Utilities/SingleOrQueue.cs index f33adaff37..37eada3d96 100644 --- a/src/Avalonia.Base/Utilities/SingleOrQueue.cs +++ b/src/Avalonia.Base/Utilities/SingleOrQueue.cs @@ -11,7 +11,7 @@ namespace Avalonia.Utilities private Queue Tail => _tail ?? (_tail = new Queue()); - private bool HasTail => _tail != null; + public bool HasTail => _tail != null; public bool Empty { get; private set; } = true; diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 0e404500a1..c8e2124182 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -168,14 +168,14 @@ namespace Avalonia return value; } - private DeferredSetter _defferedSetter; + private DeferredSetter _defferedSetter; - public DeferredSetter Setter + public DeferredSetter Setter { get { return _defferedSetter ?? - (_defferedSetter = new DeferredSetter()); + (_defferedSetter = new DeferredSetter()); } } } From e2020f30120d72f57cfcaa6ef05ddd38db77de9c Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 3 Jul 2018 17:06:40 -0500 Subject: [PATCH 09/24] Fix mock setup. --- tests/Avalonia.Base.UnitTests/PriorityValueTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs b/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs index 61ba9bc463..2f1b7862a7 100644 --- a/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs +++ b/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs @@ -307,7 +307,7 @@ namespace Avalonia.Base.UnitTests private static Mock GetMockOwner() { var owner = new Mock(); - owner.SetupGet(o => o.Setter).Returns(new DeferredSetter()); + owner.SetupGet(o => o.Setter).Returns(new DeferredSetter()); return owner; } } From 95ac57d648b0d48c7e20c4da43ac10e72e6f345a Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 6 Jul 2018 14:27:15 -0500 Subject: [PATCH 10/24] Add some SingleOrQueue documentation. --- src/Avalonia.Base/Utilities/SingleOrQueue.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Avalonia.Base/Utilities/SingleOrQueue.cs b/src/Avalonia.Base/Utilities/SingleOrQueue.cs index 37eada3d96..04c1279148 100644 --- a/src/Avalonia.Base/Utilities/SingleOrQueue.cs +++ b/src/Avalonia.Base/Utilities/SingleOrQueue.cs @@ -4,6 +4,10 @@ using System.Text; namespace Avalonia.Utilities { + /// + /// FIFO Queue optimized for holding zero or one items. + /// + /// The type of items held in the queue. public class SingleOrQueue { private T _head; @@ -11,6 +15,9 @@ namespace Avalonia.Utilities private Queue Tail => _tail ?? (_tail = new Queue()); + /// + /// True if this queue has at some point had more than one element. + /// public bool HasTail => _tail != null; public bool Empty { get; private set; } = true; From 45f37dfd1eb7f8af1392447a9b0ad77da82fc964 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 6 Jul 2018 15:11:44 -0500 Subject: [PATCH 11/24] Remove cleanup since it was slowing down ControlCatalog. --- src/Avalonia.Base/Utilities/DeferredSetter.cs | 12 ------------ src/Avalonia.Base/Utilities/SingleOrQueue.cs | 5 +---- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/src/Avalonia.Base/Utilities/DeferredSetter.cs b/src/Avalonia.Base/Utilities/DeferredSetter.cs index 15c85338c7..ae6f599005 100644 --- a/src/Avalonia.Base/Utilities/DeferredSetter.cs +++ b/src/Avalonia.Base/Utilities/DeferredSetter.cs @@ -45,8 +45,6 @@ namespace Avalonia.Utilities return pendingValues ?? (pendingValues = new SingleOrQueue()); } } - - public bool IsSimpleSet => pendingValues?.HasTail != true; } private Dictionary _setRecords; @@ -118,14 +116,6 @@ namespace Avalonia.Utilities return GetOrCreateStatus(property).PendingValues.Dequeue(); } - private void CleanupSetStatus(AvaloniaProperty property) - { - if (SetRecords.TryGetValue(property, out var status) && status.IsSimpleSet) - { - SetRecords.Remove(property); - } - } - public delegate bool SetterDelegate(TSetRecord record, ref TValue backing, Action notifyCallback); /// @@ -169,8 +159,6 @@ namespace Avalonia.Utilities }); } - CleanupSetStatus(property); - return updated; } else if(!object.Equals(value, backing)) diff --git a/src/Avalonia.Base/Utilities/SingleOrQueue.cs b/src/Avalonia.Base/Utilities/SingleOrQueue.cs index 04c1279148..4a66b72a56 100644 --- a/src/Avalonia.Base/Utilities/SingleOrQueue.cs +++ b/src/Avalonia.Base/Utilities/SingleOrQueue.cs @@ -15,10 +15,7 @@ namespace Avalonia.Utilities private Queue Tail => _tail ?? (_tail = new Queue()); - /// - /// True if this queue has at some point had more than one element. - /// - public bool HasTail => _tail != null; + private bool HasTail => _tail != null; public bool Empty { get; private set; } = true; From e1ddeee321bcf0eb051da29973c769712adb025b Mon Sep 17 00:00:00 2001 From: Luis Silva Date: Sun, 15 Jul 2018 00:00:01 +0700 Subject: [PATCH 12/24] Created new test Created new test to determine if StackPanels with gaps add gaps for invisible children. --- .../StackPanelTests.cs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/StackPanelTests.cs b/tests/Avalonia.Controls.UnitTests/StackPanelTests.cs index ba80cb779a..dca2c5df35 100644 --- a/tests/Avalonia.Controls.UnitTests/StackPanelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/StackPanelTests.cs @@ -146,5 +146,44 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(new Rect(20, 0, 30, 120), target.Children[1].Bounds); Assert.Equal(new Rect(50, 0, 50, 120), target.Children[2].Bounds); } + + [Theory] + [InlineData(Orientation.Horizontal)] + [InlineData(Orientation.Vertical)] + public void Gap_Not_Added_For_Invisible_Children(Orientation orientation) + { + var targetThreeChildrenOneInvisble = new StackPanel + { + Gap = 40, + Orientation = orientation, + Children = + { + new StackPanel { Width = 10, Height= 10, IsVisible = false }, + new StackPanel { Width = 10, Height= 10 }, + new StackPanel { Width = 10, Height= 10 }, + } + }; + var targetTwoChildrenNoneInvisible = new StackPanel + { + Gap = 40, + Orientation = orientation, + Children = + { + new StackPanel { Width = 10, Height= 10 }, + new StackPanel { Width = 10, Height= 10 } + } + }; + + targetThreeChildrenOneInvisble.Measure(Size.Infinity); + targetThreeChildrenOneInvisble.Arrange(new Rect(targetThreeChildrenOneInvisble.DesiredSize)); + + targetTwoChildrenNoneInvisible.Measure(Size.Infinity); + targetTwoChildrenNoneInvisible.Arrange(new Rect(targetTwoChildrenNoneInvisible.DesiredSize)); + + Size sizeWithTwoChildren = targetTwoChildrenNoneInvisible.Bounds.Size; + Size sizeWithThreeChildren = targetThreeChildrenOneInvisble.Bounds.Size; + + Assert.Equal(sizeWithTwoChildren, sizeWithThreeChildren); + } } } From 7039b32a79c41415ca52296245417f920f8b3d5f Mon Sep 17 00:00:00 2001 From: Luis Silva Date: Sun, 15 Jul 2018 00:05:07 +0700 Subject: [PATCH 13/24] add/sub gap behaviour based on visibility Only adding and subtracting the gap value if child is visible. Arranged height/width adds and subs gap only if at least one child is visible. --- src/Avalonia.Controls/StackPanel.cs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/Avalonia.Controls/StackPanel.cs b/src/Avalonia.Controls/StackPanel.cs index a6fe35d668..d404e6913b 100644 --- a/src/Avalonia.Controls/StackPanel.cs +++ b/src/Avalonia.Controls/StackPanel.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; +using System.Linq; using Avalonia.Input; namespace Avalonia.Controls @@ -152,6 +153,7 @@ namespace Avalonia.Controls double measuredWidth = 0; double measuredHeight = 0; double gap = Gap; + bool hasVisibleChild = Children.Any(c => c.IsVisible); foreach (Control child in Children) { @@ -160,23 +162,23 @@ namespace Avalonia.Controls if (Orientation == Orientation.Vertical) { - measuredHeight += size.Height + gap; + measuredHeight += size.Height + (child.IsVisible ? gap : 0); measuredWidth = Math.Max(measuredWidth, size.Width); } else { - measuredWidth += size.Width + gap; + measuredWidth += size.Width + (child.IsVisible ? gap : 0); measuredHeight = Math.Max(measuredHeight, size.Height); } } if (Orientation == Orientation.Vertical) { - measuredHeight -= gap; + measuredHeight -= (hasVisibleChild ? gap : 0); } else { - measuredWidth -= gap; + measuredWidth -= (hasVisibleChild ? gap : 0); } return new Size(measuredWidth, measuredHeight); @@ -193,6 +195,7 @@ namespace Avalonia.Controls double arrangedWidth = finalSize.Width; double arrangedHeight = finalSize.Height; double gap = Gap; + bool hasVisibleChild = Children.Any(c => c.IsVisible); if (Orientation == Orientation.Vertical) { @@ -214,25 +217,25 @@ namespace Avalonia.Controls Rect childFinal = new Rect(0, arrangedHeight, width, childHeight); ArrangeChild(child, childFinal, finalSize, orientation); arrangedWidth = Math.Max(arrangedWidth, childWidth); - arrangedHeight += childHeight + gap; + arrangedHeight += childHeight + (child.IsVisible ? gap : 0); } else { double height = Math.Max(childHeight, arrangedHeight); Rect childFinal = new Rect(arrangedWidth, 0, childWidth, height); ArrangeChild(child, childFinal, finalSize, orientation); - arrangedWidth += childWidth + gap; + arrangedWidth += childWidth + (child.IsVisible ? gap : 0); arrangedHeight = Math.Max(arrangedHeight, childHeight); } } if (orientation == Orientation.Vertical) { - arrangedHeight = Math.Max(arrangedHeight - gap, finalSize.Height); + arrangedHeight = Math.Max(arrangedHeight - (hasVisibleChild ? gap : 0), finalSize.Height); } else { - arrangedWidth = Math.Max(arrangedWidth - gap, finalSize.Width); + arrangedWidth = Math.Max(arrangedWidth - (hasVisibleChild ? gap : 0), finalSize.Width); } return new Size(arrangedWidth, arrangedHeight); From 09892554c49e8ad74d8aa7ba72f9b563dd53aaa9 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Tue, 17 Jul 2018 19:43:31 +0100 Subject: [PATCH 14/24] dont clear textbox selection when right clicking. --- src/Avalonia.Controls/TextBox.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/TextBox.cs b/src/Avalonia.Controls/TextBox.cs index 2ea9319194..5b09fbfb51 100644 --- a/src/Avalonia.Controls/TextBox.cs +++ b/src/Avalonia.Controls/TextBox.cs @@ -557,7 +557,7 @@ namespace Avalonia.Controls var index = CaretIndex = _presenter.GetCaretIndex(point); var text = Text; - if (text != null) + if (text != null && e.MouseButton == MouseButton.Left) { switch (e.ClickCount) { From 7f945b3950e46b4a6da477d9d49fc1e9215cd970 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 18 Jul 2018 15:11:33 -0500 Subject: [PATCH 15/24] PR Feedback. --- src/Avalonia.Base/AvaloniaObject.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index db4e52ee0f..4580ef6c58 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -210,7 +210,7 @@ namespace Avalonia { return ((IDirectPropertyAccessor)GetRegistered(property)).GetValue(this); } - else if (Values != null) + else if (_values != null) { var result = Values.GetValue(property); @@ -250,7 +250,7 @@ namespace Avalonia Contract.Requires(property != null); VerifyAccess(); - return Values?.IsAnimating(property) ?? false; + return _values?.IsAnimating(property) ?? false; } /// @@ -267,7 +267,7 @@ namespace Avalonia Contract.Requires(property != null); VerifyAccess(); - return Values?.IsSet(property) ?? false; + return _values?.IsSet(property) ?? false; } /// @@ -394,7 +394,7 @@ namespace Avalonia public void Revalidate(AvaloniaProperty property) { VerifyAccess(); - Values?.Revalidate(property); + _values?.Revalidate(property); } internal void PriorityValueChanged(AvaloniaProperty property, int priority, object oldValue, object newValue) From 70544b44abfc1875e13f4b589cb9bfb8618aec92 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 18 Jul 2018 23:28:14 +0200 Subject: [PATCH 16/24] Allow BindingMode.Default in Setter bindings. Fixes #1754. --- src/Avalonia.Styling/Styling/Setter.cs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/Avalonia.Styling/Styling/Setter.cs b/src/Avalonia.Styling/Styling/Setter.cs index 1a78e0f4d7..31b685f6b1 100644 --- a/src/Avalonia.Styling/Styling/Setter.cs +++ b/src/Avalonia.Styling/Styling/Setter.cs @@ -158,18 +158,11 @@ namespace Avalonia.Styling var activated = new ActivatedObservable(activator, sourceInstance.Observable, description); return InstancedBinding.OneWay(activated, BindingPriority.StyleTrigger); } - case BindingMode.OneWayToSource: - { - var activated = new ActivatedSubject(activator, sourceInstance.Subject, description); - return InstancedBinding.OneWayToSource(activated, BindingPriority.StyleTrigger); - } - case BindingMode.TwoWay: + default: { var activated = new ActivatedSubject(activator, sourceInstance.Subject, description); - return InstancedBinding.TwoWay(activated, BindingPriority.StyleTrigger); + return new InstancedBinding(activated, sourceInstance.Mode, BindingPriority.StyleTrigger); } - default: - throw new NotSupportedException("Unsupported BindingMode."); } } From 2666161d9432f89e05d3cdda05174a5af8b35909 Mon Sep 17 00:00:00 2001 From: Nelson Carrillo Date: Wed, 18 Jul 2018 23:18:56 -0400 Subject: [PATCH 17/24] Cache GridLayouts from the last Measure --- src/Avalonia.Controls/Grid.cs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Controls/Grid.cs b/src/Avalonia.Controls/Grid.cs index 54fcefeb3f..5f194bdd71 100644 --- a/src/Avalonia.Controls/Grid.cs +++ b/src/Avalonia.Controls/Grid.cs @@ -194,6 +194,16 @@ namespace Avalonia.Controls /// private GridLayout.MeasureResult _rowMeasureCache; + /// + /// Gets the row layout as of the last measure. + /// + private GridLayout _rowLayoutCache; + + /// + /// Gets the column layout as of the last measure. + /// + private GridLayout _columnLayoutCache; + /// /// Measures the grid. /// @@ -253,6 +263,9 @@ namespace Avalonia.Controls // Cache the measure result and return the desired size. _columnMeasureCache = columnResult; _rowMeasureCache = rowResult; + _rowLayoutCache = rowLayout; + _columnLayoutCache = columnLayout; + return new Size(columnResult.DesiredLength, rowResult.DesiredLength); // Measure each child only once. @@ -299,13 +312,11 @@ namespace Avalonia.Controls // arrow back to any statements and re-run them without any side-effect. var (safeColumns, safeRows) = GetSafeColumnRows(); - var columnLayout = new GridLayout(ColumnDefinitions); - var rowLayout = new GridLayout(RowDefinitions); - + var columnLayout = _columnLayoutCache; + var rowLayout = _rowLayoutCache; // Calculate for arrange result. var columnResult = columnLayout.Arrange(finalSize.Width, _columnMeasureCache); var rowResult = rowLayout.Arrange(finalSize.Height, _rowMeasureCache); - // Arrange the children. foreach (var child in Children.OfType()) { @@ -315,7 +326,6 @@ namespace Avalonia.Controls var y = Enumerable.Range(0, row).Sum(r => rowResult.LengthList[r]); var width = Enumerable.Range(column, columnSpan).Sum(c => columnResult.LengthList[c]); var height = Enumerable.Range(row, rowSpan).Sum(r => rowResult.LengthList[r]); - child.Arrange(new Rect(x, y, width, height)); } From b651cd3a1bee4c0da033c76ea3624036d27c3aa1 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 19 Jul 2018 12:05:10 +0100 Subject: [PATCH 18/24] fix carousel-presenter when items are removed. --- src/Avalonia.Controls/Presenters/CarouselPresenter.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs index 1d5a187a73..f0f86b7fb4 100644 --- a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs +++ b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs @@ -115,6 +115,11 @@ namespace Avalonia.Controls.Presenters var containers = generator.RemoveRange(e.OldStartingIndex, e.OldItems.Count); Panel.Children.RemoveAll(containers.Select(x => x.ContainerControl)); + if(SelectedIndex > containers.Count()) + { + SelectedIndex = containers.Count(); + } + #pragma warning disable 4014 MoveToPage(-1, SelectedIndex); #pragma warning restore 4014 @@ -130,7 +135,7 @@ namespace Avalonia.Controls.Presenters #pragma warning disable 4014 var newIndex = SelectedIndex; - + if(SelectedIndex < 0) { if(Items != null && Items.Count() > 0) @@ -251,4 +256,4 @@ namespace Avalonia.Controls.Presenters } } } -} \ No newline at end of file +} From 67d60827304ca9700a46dda0f5fc37ab0e54a3fe Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 19 Jul 2018 12:13:43 +0100 Subject: [PATCH 19/24] sync selected index with items count when removing items from carousel. --- src/Avalonia.Controls/Presenters/CarouselPresenter.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs index f0f86b7fb4..7591117dca 100644 --- a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs +++ b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs @@ -115,9 +115,9 @@ namespace Avalonia.Controls.Presenters var containers = generator.RemoveRange(e.OldStartingIndex, e.OldItems.Count); Panel.Children.RemoveAll(containers.Select(x => x.ContainerControl)); - if(SelectedIndex > containers.Count()) + if(SelectedIndex >= Items.Count()) { - SelectedIndex = containers.Count(); + SelectedIndex = Items.Count() - 1; } #pragma warning disable 4014 From ee55b4fa3dab06c4e8dcae4f46866d99d4ecb466 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 19 Jul 2018 12:51:48 +0100 Subject: [PATCH 20/24] bug is in selecting items control. add failing unit test. --- .../Presenters/CarouselPresenter.cs | 5 ---- .../Primitives/SelectingItemsControlTests.cs | 28 +++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs index 7591117dca..f77f249c46 100644 --- a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs +++ b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs @@ -115,11 +115,6 @@ namespace Avalonia.Controls.Presenters var containers = generator.RemoveRange(e.OldStartingIndex, e.OldItems.Count); Panel.Children.RemoveAll(containers.Select(x => x.ContainerControl)); - if(SelectedIndex >= Items.Count()) - { - SelectedIndex = Items.Count() - 1; - } - #pragma warning disable 4014 MoveToPage(-1, SelectedIndex); #pragma warning restore 4014 diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs index c052b81309..c7a3465ac4 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs @@ -149,6 +149,34 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal(1, target.SelectedIndex); } + [Fact] + public void SelectedIndex_Item_Is_Updated_As_Items_Removed_When_Last_Item_Is_Selected() + { + var items = new ObservableCollection + { + "Foo", + "Bar", + "FooBar" + }; + + var target = new SelectingItemsControl + { + Items = items, + Template = Template(), + }; + + target.ApplyTemplate(); + target.SelectedItem = items[2]; + + Assert.Equal(items[2], target.SelectedItem); + Assert.Equal(2, target.SelectedIndex); + + items.RemoveAt(0); + + Assert.Equal(items[1], target.SelectedItem); + Assert.Equal(1, target.SelectedIndex); + } + [Fact] public void Setting_SelectedItem_To_Not_Present_Item_Should_Clear_Selection() { From ae4356ef343aa3ebfaae8460296f3b1e412b5b41 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 19 Jul 2018 12:52:45 +0100 Subject: [PATCH 21/24] add fix for selecting items control. --- src/Avalonia.Controls/Primitives/SelectingItemsControl.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index a7b8981583..354c740871 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -355,6 +355,12 @@ namespace Avalonia.Controls.Primitives case NotifyCollectionChangedAction.Replace: var selectedIndex = SelectedIndex; + var items = Items?.Cast(); + if (selectedIndex >= items.Count()) + { + selectedIndex = SelectedIndex = items.Count() - 1; + } + if (selectedIndex >= e.OldStartingIndex && selectedIndex < e.OldStartingIndex + e.OldItems.Count) { From fe1620a25dd4788cb00f780a2d4e966b004c455e Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 19 Jul 2018 12:54:14 +0100 Subject: [PATCH 22/24] whitespace. --- .../Presenters/CarouselPresenter.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs index f77f249c46..42910cdc48 100644 --- a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs +++ b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs @@ -39,7 +39,7 @@ namespace Avalonia.Controls.Presenters Carousel.PageTransitionProperty.AddOwner(); private int _selectedIndex = -1; - // private Task _current; + // private Task _current; private Task _currentTransition; private int _queuedTransitionIndex = -1; @@ -130,10 +130,10 @@ namespace Avalonia.Controls.Presenters #pragma warning disable 4014 var newIndex = SelectedIndex; - - if(SelectedIndex < 0) + + if (SelectedIndex < 0) { - if(Items != null && Items.Count() > 0) + if (Items != null && Items.Count() > 0) { newIndex = 0; } @@ -142,11 +142,11 @@ namespace Avalonia.Controls.Presenters newIndex = -1; } } - + MoveToPage(-1, newIndex); #pragma warning restore 4014 } - break; + break; } } @@ -226,7 +226,7 @@ namespace Avalonia.Controls.Presenters int fromIndex = (int)e.OldValue; int toIndex = (int)e.NewValue; - for (;;) + for (; ; ) { _currentTransition = MoveToPage(fromIndex, toIndex); await _currentTransition; From 684c65c2606fd0ecc0eb6dd30adf620c92301bf8 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 19 Jul 2018 12:56:12 +0100 Subject: [PATCH 23/24] revert file. --- .../Presenters/CarouselPresenter.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs index 42910cdc48..1d5a187a73 100644 --- a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs +++ b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs @@ -39,7 +39,7 @@ namespace Avalonia.Controls.Presenters Carousel.PageTransitionProperty.AddOwner(); private int _selectedIndex = -1; - // private Task _current; + // private Task _current; private Task _currentTransition; private int _queuedTransitionIndex = -1; @@ -131,9 +131,9 @@ namespace Avalonia.Controls.Presenters #pragma warning disable 4014 var newIndex = SelectedIndex; - if (SelectedIndex < 0) + if(SelectedIndex < 0) { - if (Items != null && Items.Count() > 0) + if(Items != null && Items.Count() > 0) { newIndex = 0; } @@ -142,11 +142,11 @@ namespace Avalonia.Controls.Presenters newIndex = -1; } } - + MoveToPage(-1, newIndex); #pragma warning restore 4014 } - break; + break; } } @@ -226,7 +226,7 @@ namespace Avalonia.Controls.Presenters int fromIndex = (int)e.OldValue; int toIndex = (int)e.NewValue; - for (; ; ) + for (;;) { _currentTransition = MoveToPage(fromIndex, toIndex); await _currentTransition; @@ -251,4 +251,4 @@ namespace Avalonia.Controls.Presenters } } } -} +} \ No newline at end of file From 6eb80bd55c856cd22653f4c6b28c7c05ea0e470c Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 19 Jul 2018 13:04:18 +0100 Subject: [PATCH 24/24] fix broken logic. --- .../Primitives/SelectingItemsControl.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 354c740871..eb3fbde8f2 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -355,18 +355,12 @@ namespace Avalonia.Controls.Primitives case NotifyCollectionChangedAction.Replace: var selectedIndex = SelectedIndex; - var items = Items?.Cast(); - if (selectedIndex >= items.Count()) - { - selectedIndex = SelectedIndex = items.Count() - 1; - } - if (selectedIndex >= e.OldStartingIndex && selectedIndex < e.OldStartingIndex + e.OldItems.Count) { if (!AlwaysSelected) { - SelectedIndex = -1; + selectedIndex = SelectedIndex = -1; } else { @@ -374,6 +368,11 @@ namespace Avalonia.Controls.Primitives } } + var items = Items?.Cast(); + if (selectedIndex >= items.Count()) + { + selectedIndex = SelectedIndex = items.Count() - 1; + } break; case NotifyCollectionChangedAction.Reset: