From 7f8dc7766ea500a1046763e73e0640d62973660c Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 20 Jun 2018 12:46:29 -0500 Subject: [PATCH 01/42] Span-ified PathMarkupParser v1. --- build/System.Memory.props | 5 + src/Avalonia.Visuals/Avalonia.Visuals.csproj | 1 + .../Media/PathMarkupParser.cs | 106 +++++++++++++++++- 3 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 build/System.Memory.props diff --git a/build/System.Memory.props b/build/System.Memory.props new file mode 100644 index 0000000000..f3253f8882 --- /dev/null +++ b/build/System.Memory.props @@ -0,0 +1,5 @@ + + + + + diff --git a/src/Avalonia.Visuals/Avalonia.Visuals.csproj b/src/Avalonia.Visuals/Avalonia.Visuals.csproj index 0f003a4018..c34752a3ef 100644 --- a/src/Avalonia.Visuals/Avalonia.Visuals.csproj +++ b/src/Avalonia.Visuals/Avalonia.Visuals.csproj @@ -8,4 +8,5 @@ + \ No newline at end of file diff --git a/src/Avalonia.Visuals/Media/PathMarkupParser.cs b/src/Avalonia.Visuals/Media/PathMarkupParser.cs index e8824753b5..7ef07d7ec8 100644 --- a/src/Avalonia.Visuals/Media/PathMarkupParser.cs +++ b/src/Avalonia.Visuals/Media/PathMarkupParser.cs @@ -82,7 +82,7 @@ namespace Avalonia.Media /// The path data. public void Parse(string pathData) { - var tokens = ParseTokens(pathData); + var tokens = ParseTokens2(pathData); CreateGeometry(tokens); } @@ -121,6 +121,17 @@ namespace Avalonia.Media return @"(?=[" + stringBuilder + "])"; } + private static IEnumerable ParseTokens2(string s) + { + var commands = new List(); + var span = s.AsSpan(); + while (!span.IsEmpty) + { + commands.Add(CommandToken.Parse(ref span)); + } + return commands; + } + private static IEnumerable ParseTokens(string s) { return Regex.Split(s, s_separatorPattern).Where(t => !string.IsNullOrEmpty(t)).Select(CommandToken.Parse); @@ -494,6 +505,27 @@ namespace Avalonia.Media return new CommandToken(command, isRelative, arguments); } } + + public static CommandToken Parse(ref ReadOnlySpan span) + { + if (!ReadCommand(ref span, out var command, out var isRelative)) + { + throw new InvalidDataException("No path command declared."); + } + + span = span.Slice(1); + span = SkipWhitespace(span); + + var arguments = new List(); + + while (ReadArgument(ref span, out var argument)) + { + arguments.Add(argument.ToString()); + span = ReadSeparator(span); + } + + return new CommandToken(command, isRelative, arguments); + } public FillRule ReadFillRule() { @@ -592,6 +624,28 @@ namespace Avalonia.Media return new Point(origin.X + x, origin.Y + y); } + private static bool ReadCommand(ref ReadOnlySpan span, out Command command, out bool relative) + { + span = SkipWhitespace(span); + if (span.IsEmpty) + { + command = default; + relative = false; + return false; + } + + var c = span[0]; + + if (!s_commands.TryGetValue(char.ToUpperInvariant(c), out command)) + { + throw new InvalidDataException("Unexpected path command '" + c + "'."); + } + + relative = char.IsLower(c); + + return true; + } + private static bool ReadCommand(TextReader reader, ref Command command, ref bool relative) { ReadWhitespace(reader); @@ -619,6 +673,56 @@ namespace Avalonia.Media return true; } + private static bool ReadArgument(ref ReadOnlySpan remaining, out ReadOnlySpan argument) + { + if (remaining.IsEmpty) + { + argument = ReadOnlySpan.Empty; + return false; + } + + var valid = false; + int i = 0; + if (remaining[i] == '-') + { + i++; + } + for (; i < remaining.Length && char.IsNumber(remaining[i]); i++) valid = true ; + + if (i < remaining.Length && remaining[i] == '.') + { + valid = false; + i++; + } + for (; i < remaining.Length && char.IsNumber(remaining[i]); i++) valid = true ; + + if (!valid) + { + argument = ReadOnlySpan.Empty; + return false; + } + argument = remaining.Slice(0, i); + remaining = remaining.Slice(i); + return true; + } + + private static ReadOnlySpan ReadSeparator(ReadOnlySpan span) + { + span = SkipWhitespace(span); + if (!span.IsEmpty && span[0] == ',') + { + span = span.Slice(1); + } + return SkipWhitespace(span); + } + + private static ReadOnlySpan SkipWhitespace(ReadOnlySpan span) + { + int i = 0; + for (; i < span.Length && char.IsWhiteSpace(span[i]); i++) ; + return span.Slice(i); + } + private static void ReadWhitespace(TextReader reader) { int i; From dc9fb565fd35b4feed137f11cacd62b7854c7f47 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 20 Jun 2018 12:50:06 -0500 Subject: [PATCH 02/42] Remove old regex-based parser --- .../Media/PathMarkupParser.cs | 107 +----------------- 1 file changed, 5 insertions(+), 102 deletions(-) diff --git a/src/Avalonia.Visuals/Media/PathMarkupParser.cs b/src/Avalonia.Visuals/Media/PathMarkupParser.cs index 7ef07d7ec8..c71aed8b66 100644 --- a/src/Avalonia.Visuals/Media/PathMarkupParser.cs +++ b/src/Avalonia.Visuals/Media/PathMarkupParser.cs @@ -17,7 +17,6 @@ namespace Avalonia.Media /// public class PathMarkupParser : IDisposable { - private static readonly string s_separatorPattern; private static readonly Dictionary s_commands = new Dictionary { @@ -42,7 +41,6 @@ namespace Avalonia.Media static PathMarkupParser() { - s_separatorPattern = CreatesSeparatorPattern(); } /// @@ -82,7 +80,7 @@ namespace Avalonia.Media /// The path data. public void Parse(string pathData) { - var tokens = ParseTokens2(pathData); + var tokens = ParseTokens(pathData); CreateGeometry(tokens); } @@ -107,21 +105,7 @@ namespace Avalonia.Media _isDisposed = true; } - private static string CreatesSeparatorPattern() - { - var stringBuilder = new StringBuilder(); - - foreach (var command in s_commands.Keys) - { - stringBuilder.Append(command); - - stringBuilder.Append(char.ToLower(command)); - } - - return @"(?=[" + stringBuilder + "])"; - } - - private static IEnumerable ParseTokens2(string s) + private static IEnumerable ParseTokens(string s) { var commands = new List(); var span = s.AsSpan(); @@ -132,11 +116,6 @@ namespace Avalonia.Media return commands; } - private static IEnumerable ParseTokens(string s) - { - return Regex.Split(s, s_separatorPattern).Where(t => !string.IsNullOrEmpty(t)).Select(CommandToken.Parse); - } - private static Point MirrorControlPoint(Point controlPoint, Point center) { var dir = controlPoint - center; @@ -146,7 +125,7 @@ namespace Avalonia.Media private void CreateGeometry(IEnumerable commandTokens) { - _currentPoint = new Point(); + _currentPoint = new Point(); foreach (var commandToken in commandTokens) { @@ -446,15 +425,13 @@ namespace Avalonia.Media private class CommandToken { - private const string ArgumentExpression = @"-?[0-9]*\.?\d+"; - - private CommandToken(Command command, bool isRelative, IEnumerable arguments) + private CommandToken(Command command, bool isRelative, List arguments) { Command = command; IsRelative = isRelative; - Arguments = new List(arguments); + Arguments = arguments; } public Command Command { get; } @@ -477,34 +454,6 @@ namespace Avalonia.Media private int CurrentPosition { get; set; } private List Arguments { get; } - - public static CommandToken Parse(string s) - { - using (var reader = new StringReader(s)) - { - var command = Command.None; - - var isRelative = false; - - if (!ReadCommand(reader, ref command, ref isRelative)) - { - throw new InvalidDataException("No path command declared."); - } - - var commandArguments = reader.ReadToEnd(); - - var argumentMatches = Regex.Matches(commandArguments, ArgumentExpression); - - var arguments = new List(); - - foreach (Match match in argumentMatches) - { - arguments.Add(match.Value); - } - - return new CommandToken(command, isRelative, arguments); - } - } public static CommandToken Parse(ref ReadOnlySpan span) { @@ -646,33 +595,6 @@ namespace Avalonia.Media return true; } - private static bool ReadCommand(TextReader reader, ref Command command, ref bool relative) - { - ReadWhitespace(reader); - - var i = reader.Peek(); - - if (i == -1) - { - return false; - } - - var c = (char)i; - - if (!s_commands.TryGetValue(char.ToUpperInvariant(c), out var next)) - { - throw new InvalidDataException("Unexpected path command '" + c + "'."); - } - - command = next; - - relative = char.IsLower(c); - - reader.Read(); - - return true; - } - private static bool ReadArgument(ref ReadOnlySpan remaining, out ReadOnlySpan argument) { if (remaining.IsEmpty) @@ -722,25 +644,6 @@ namespace Avalonia.Media for (; i < span.Length && char.IsWhiteSpace(span[i]); i++) ; return span.Slice(i); } - - private static void ReadWhitespace(TextReader reader) - { - int i; - - while ((i = reader.Peek()) != -1) - { - var c = (char)i; - - if (char.IsWhiteSpace(c)) - { - reader.Read(); - } - else - { - break; - } - } - } } } } \ No newline at end of file From 6c6f1b48cf62048c9b94c671020558cb1b742ba9 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 20 Jun 2018 15:08:23 -0500 Subject: [PATCH 03/42] Remove unused static constructor. --- src/Avalonia.Visuals/Media/PathMarkupParser.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Avalonia.Visuals/Media/PathMarkupParser.cs b/src/Avalonia.Visuals/Media/PathMarkupParser.cs index c71aed8b66..e17aafeda3 100644 --- a/src/Avalonia.Visuals/Media/PathMarkupParser.cs +++ b/src/Avalonia.Visuals/Media/PathMarkupParser.cs @@ -39,10 +39,6 @@ namespace Avalonia.Media private bool? _isOpen; private bool _isDisposed; - static PathMarkupParser() - { - } - /// /// Initializes a new instance of the class. /// From 82802621ead1ff9440b2bb3060697388906b560d Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 20 Jun 2018 15:54:48 -0500 Subject: [PATCH 04/42] Update gitignore and add System.Memory package dependency. --- .gitignore | 8 ++++++-- packages.cake | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index a9a8fd36b4..583a2b8a2b 100644 --- a/.gitignore +++ b/.gitignore @@ -176,5 +176,9 @@ nuget Avalonia.XBuild.sln project.lock.json .idea/* -**/obj-Skia/* -**/obj-Direct2D1/* + + +################## +## BenchmarkDotNet +################## +BenchmarkDotNet.Artifacts/ diff --git a/packages.cake b/packages.cake index 7e7e722c82..79147b5f99 100644 --- a/packages.cake +++ b/packages.cake @@ -253,7 +253,7 @@ public class Packages } .Deps(new string[]{null, "netcoreapp2.0"}, "System.ValueTuple", "System.ComponentModel.TypeConverter", "System.ComponentModel.Primitives", - "System.Runtime.Serialization.Primitives", "System.Xml.XmlDocument", "System.Xml.ReaderWriter") + "System.Runtime.Serialization.Primitives", "System.Xml.XmlDocument", "System.Xml.ReaderWriter", "System.Memory") .ToArray(), Files = coreLibrariesNuSpecContent .Concat(win32CoreLibrariesNuSpecContent).Concat(net45RuntimePlatform) From 63ec705748fd13ad3fa33aad762eebd3bf315d18 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 20 Jun 2018 16:37:16 -0500 Subject: [PATCH 05/42] Remove extraneous whitespace in tests. --- tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs b/tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs index 35ec38789e..5074d306fd 100644 --- a/tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs @@ -69,7 +69,7 @@ namespace Avalonia.Visuals.UnitTests.Media using (var context = new PathGeometryContext(pathGeometry)) using (var parser = new PathMarkupParser(context)) { - parser.Parse("F 1M0,0"); + parser.Parse("F 1M0,0"); Assert.Equal(FillRule.NonZero, pathGeometry.FillRule); } From e2f54537982df5136da906478807a71e5e4fe96d Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 20 Jun 2018 16:53:00 -0500 Subject: [PATCH 06/42] Remove LINQ usage. --- .../Media/PathMarkupParser.cs | 51 ++++--------------- 1 file changed, 10 insertions(+), 41 deletions(-) diff --git a/src/Avalonia.Visuals/Media/PathMarkupParser.cs b/src/Avalonia.Visuals/Media/PathMarkupParser.cs index e17aafeda3..41cd95d319 100644 --- a/src/Avalonia.Visuals/Media/PathMarkupParser.cs +++ b/src/Avalonia.Visuals/Media/PathMarkupParser.cs @@ -5,9 +5,6 @@ using System; using System.Collections.Generic; using System.Globalization; using System.IO; -using System.Linq; -using System.Text; -using System.Text.RegularExpressions; using Avalonia.Platform; namespace Avalonia.Media @@ -70,17 +67,6 @@ namespace Avalonia.Media Close } - /// - /// Parses the specified path data and writes the result to the geometryContext of this instance. - /// - /// The path data. - public void Parse(string pathData) - { - var tokens = ParseTokens(pathData); - - CreateGeometry(tokens); - } - void IDisposable.Dispose() { Dispose(true); @@ -101,17 +87,6 @@ namespace Avalonia.Media _isDisposed = true; } - private static IEnumerable ParseTokens(string s) - { - var commands = new List(); - var span = s.AsSpan(); - while (!span.IsEmpty) - { - commands.Add(CommandToken.Parse(ref span)); - } - return commands; - } - private static Point MirrorControlPoint(Point controlPoint, Point center) { var dir = controlPoint - center; @@ -119,15 +94,21 @@ namespace Avalonia.Media return center + -dir; } - private void CreateGeometry(IEnumerable commandTokens) + /// + /// Parses the specified path data and writes the result to the geometryContext of this instance. + /// + /// The path data. + public void Parse(string pathData) { + var span = pathData.AsSpan(); _currentPoint = new Point(); - foreach (var commandToken in commandTokens) + while(!span.IsEmpty) { + var commandToken = CommandToken.Parse(ref span); try { - while (true) + do { switch (commandToken.Command) { @@ -169,14 +150,7 @@ namespace Avalonia.Media default: throw new NotSupportedException("Unsupported command"); } - - if (commandToken.HasImplicitCommands) - { - continue; - } - - break; - } + } while (commandToken.HasImplicitCommands); } catch (InvalidDataException) { @@ -235,11 +209,6 @@ namespace Avalonia.Media CreateFigure(); - if (!commandToken.HasImplicitCommands) - { - return; - } - while (commandToken.HasImplicitCommands) { AddLine(commandToken); From c31edafb553c70ea64087248fa84addac77fb1b8 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 20 Jun 2018 17:05:33 -0500 Subject: [PATCH 07/42] Clear up code logic. --- src/Avalonia.Visuals/Media/PathMarkupParser.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Visuals/Media/PathMarkupParser.cs b/src/Avalonia.Visuals/Media/PathMarkupParser.cs index 41cd95d319..b0c7ed7e95 100644 --- a/src/Avalonia.Visuals/Media/PathMarkupParser.cs +++ b/src/Avalonia.Visuals/Media/PathMarkupParser.cs @@ -213,14 +213,11 @@ namespace Avalonia.Media { AddLine(commandToken); - if (commandToken.IsRelative) + if (!commandToken.IsRelative) { - continue; + _currentPoint = currentPoint; + CreateFigure(); } - - _currentPoint = currentPoint; - - CreateFigure(); } } From 11d3e7f7e3cd26f8c92fb09bc4449b40dcac935f Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 20 Jun 2018 17:10:56 -0500 Subject: [PATCH 08/42] Make _isOpen non-nullable since the only two values that were used were true and null. --- .../Media/PathMarkupParser.cs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Avalonia.Visuals/Media/PathMarkupParser.cs b/src/Avalonia.Visuals/Media/PathMarkupParser.cs index b0c7ed7e95..8f1d8ebc71 100644 --- a/src/Avalonia.Visuals/Media/PathMarkupParser.cs +++ b/src/Avalonia.Visuals/Media/PathMarkupParser.cs @@ -33,7 +33,7 @@ namespace Avalonia.Media private IGeometryContext _geometryContext; private Point _currentPoint; private Point? _previousControlPoint; - private bool? _isOpen; + private bool _isOpen; private bool _isDisposed; /// @@ -162,7 +162,7 @@ namespace Avalonia.Media } } - if (_isOpen != null) + if (_isOpen) { _geometryContext.EndFigure(false); } @@ -170,7 +170,7 @@ namespace Avalonia.Media private void CreateFigure() { - if (_isOpen != null) + if (_isOpen) { _geometryContext.EndFigure(false); } @@ -189,14 +189,14 @@ namespace Avalonia.Media private void CloseFigure() { - if (_isOpen == true) + if (_isOpen) { _geometryContext.EndFigure(true); } _previousControlPoint = null; - _isOpen = null; + _isOpen = false; } private void AddMove(CommandToken commandToken) @@ -227,7 +227,7 @@ namespace Avalonia.Media ? commandToken.ReadRelativePoint(_currentPoint) : commandToken.ReadPoint(); - if (_isOpen == null) + if (!_isOpen) { CreateFigure(); } @@ -241,7 +241,7 @@ namespace Avalonia.Media ? new Point(_currentPoint.X + commandToken.ReadDouble(), _currentPoint.Y) : _currentPoint.WithX(commandToken.ReadDouble()); - if (_isOpen == null) + if (!_isOpen) { CreateFigure(); } @@ -255,7 +255,7 @@ namespace Avalonia.Media ? new Point(_currentPoint.X, _currentPoint.Y + commandToken.ReadDouble()) : _currentPoint.WithY(commandToken.ReadDouble()); - if (_isOpen == null) + if (!_isOpen) { CreateFigure(); } @@ -279,7 +279,7 @@ namespace Avalonia.Media ? commandToken.ReadRelativePoint(_currentPoint) : commandToken.ReadPoint(); - if (_isOpen == null) + if (!_isOpen) { CreateFigure(); } @@ -301,7 +301,7 @@ namespace Avalonia.Media ? commandToken.ReadRelativePoint(_currentPoint) : commandToken.ReadPoint(); - if (_isOpen == null) + if (!_isOpen) { CreateFigure(); } @@ -326,7 +326,7 @@ namespace Avalonia.Media _previousControlPoint = MirrorControlPoint((Point)_previousControlPoint, _currentPoint); } - if (_isOpen == null) + if (!_isOpen) { CreateFigure(); } @@ -349,7 +349,7 @@ namespace Avalonia.Media _previousControlPoint = MirrorControlPoint((Point)_previousControlPoint, _currentPoint); } - if (_isOpen == null) + if (!_isOpen) { CreateFigure(); } @@ -373,7 +373,7 @@ namespace Avalonia.Media ? commandToken.ReadRelativePoint(_currentPoint) : commandToken.ReadPoint(); - if (_isOpen == null) + if (!_isOpen) { CreateFigure(); } From 85159c069c08df23983fa0163c78bf851f46a515 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 20 Jun 2018 18:52:04 -0500 Subject: [PATCH 09/42] Span-ification v2. Remove CommandToken and just operate directly on the single span. Memory allocation is now down to less than 5KB. --- .../Media/PathMarkupParser.cs | 534 ++++++++---------- 1 file changed, 238 insertions(+), 296 deletions(-) diff --git a/src/Avalonia.Visuals/Media/PathMarkupParser.cs b/src/Avalonia.Visuals/Media/PathMarkupParser.cs index 8f1d8ebc71..384c96a7d6 100644 --- a/src/Avalonia.Visuals/Media/PathMarkupParser.cs +++ b/src/Avalonia.Visuals/Media/PathMarkupParser.cs @@ -105,61 +105,64 @@ namespace Avalonia.Media while(!span.IsEmpty) { - var commandToken = CommandToken.Parse(ref span); - try + if(!ReadCommand(ref span, out var command, out var relative)) { - do - { - switch (commandToken.Command) - { - case Command.None: - break; - case Command.FillRule: - SetFillRule(commandToken); - break; - case Command.Move: - AddMove(commandToken); - break; - case Command.Line: - AddLine(commandToken); - break; - case Command.HorizontalLine: - AddHorizontalLine(commandToken); - break; - case Command.VerticalLine: - AddVerticalLine(commandToken); - break; - case Command.CubicBezierCurve: - AddCubicBezierCurve(commandToken); - break; - case Command.QuadraticBezierCurve: - AddQuadraticBezierCurve(commandToken); - break; - case Command.SmoothCubicBezierCurve: - AddSmoothCubicBezierCurve(commandToken); - break; - case Command.SmoothQuadraticBezierCurve: - AddSmoothQuadraticBezierCurve(commandToken); - break; - case Command.Arc: - AddArc(commandToken); - break; - case Command.Close: - CloseFigure(); - break; - default: - throw new NotSupportedException("Unsupported command"); - } - } while (commandToken.HasImplicitCommands); - } - catch (InvalidDataException) - { - break; + return; } - catch (NotSupportedException) + + bool initialCommand = true; + + do { - break; - } + if (!initialCommand) + { + span = ReadSeparator(span); + } + + switch (command) + { + case Command.None: + break; + case Command.FillRule: + SetFillRule(ref span); + break; + case Command.Move: + AddMove(ref span, relative); + break; + case Command.Line: + AddLine(ref span, relative); + break; + case Command.HorizontalLine: + AddHorizontalLine(ref span, relative); + break; + case Command.VerticalLine: + AddVerticalLine(ref span, relative); + break; + case Command.CubicBezierCurve: + AddCubicBezierCurve(ref span, relative); + break; + case Command.QuadraticBezierCurve: + AddQuadraticBezierCurve(ref span, relative); + break; + case Command.SmoothCubicBezierCurve: + AddSmoothCubicBezierCurve(ref span, relative); + break; + case Command.SmoothQuadraticBezierCurve: + AddSmoothQuadraticBezierCurve(ref span, relative); + break; + case Command.Arc: + AddArc(ref span, relative); + break; + case Command.Close: + CloseFigure(); + break; + default: + throw new NotSupportedException("Unsupported command"); + } + + initialCommand = false; + } while (PeekArgument(span)); + } if (_isOpen) @@ -180,11 +183,28 @@ namespace Avalonia.Media _isOpen = true; } - private void SetFillRule(CommandToken commandToken) + private void SetFillRule(ref ReadOnlySpan span) { - var fillRule = commandToken.ReadFillRule(); + if (!ReadArgument(ref span, out var fillRule) || fillRule.Length != 1) + { + throw new InvalidDataException("Invalid fill rule."); + } + + FillRule rule; - _geometryContext.SetFillRule(fillRule); + switch (fillRule[0]) + { + case '0': + rule = FillRule.EvenOdd; + break; + case '1': + rule = FillRule.NonZero; + break; + default: + throw new InvalidDataException("Invalid fill rule"); + } + + _geometryContext.SetFillRule(rule); } private void CloseFigure() @@ -199,21 +219,22 @@ namespace Avalonia.Media _isOpen = false; } - private void AddMove(CommandToken commandToken) + private void AddMove(ref ReadOnlySpan span, bool relative) { - var currentPoint = commandToken.IsRelative - ? commandToken.ReadRelativePoint(_currentPoint) - : commandToken.ReadPoint(); + var currentPoint = relative + ? ReadRelativePoint(ref span, _currentPoint) + : ReadPoint(ref span); _currentPoint = currentPoint; CreateFigure(); - while (commandToken.HasImplicitCommands) + while (PeekArgument(span)) { - AddLine(commandToken); + span = ReadSeparator(span); + AddLine(ref span, relative); - if (!commandToken.IsRelative) + if (!relative) { _currentPoint = currentPoint; CreateFigure(); @@ -221,11 +242,11 @@ namespace Avalonia.Media } } - private void AddLine(CommandToken commandToken) + private void AddLine(ref ReadOnlySpan span, bool relative) { - _currentPoint = commandToken.IsRelative - ? commandToken.ReadRelativePoint(_currentPoint) - : commandToken.ReadPoint(); + _currentPoint = relative + ? ReadRelativePoint(ref span, _currentPoint) + : ReadPoint(ref span); if (!_isOpen) { @@ -235,11 +256,11 @@ namespace Avalonia.Media _geometryContext.LineTo(_currentPoint); } - private void AddHorizontalLine(CommandToken commandToken) + private void AddHorizontalLine(ref ReadOnlySpan span, bool relative) { - _currentPoint = commandToken.IsRelative - ? new Point(_currentPoint.X + commandToken.ReadDouble(), _currentPoint.Y) - : _currentPoint.WithX(commandToken.ReadDouble()); + _currentPoint = relative + ? new Point(_currentPoint.X + ReadDouble(ref span), _currentPoint.Y) + : _currentPoint.WithX(ReadDouble(ref span)); if (!_isOpen) { @@ -249,11 +270,11 @@ namespace Avalonia.Media _geometryContext.LineTo(_currentPoint); } - private void AddVerticalLine(CommandToken commandToken) + private void AddVerticalLine(ref ReadOnlySpan span, bool relative) { - _currentPoint = commandToken.IsRelative - ? new Point(_currentPoint.X, _currentPoint.Y + commandToken.ReadDouble()) - : _currentPoint.WithY(commandToken.ReadDouble()); + _currentPoint = relative + ? new Point(_currentPoint.X, _currentPoint.Y + ReadDouble(ref span)) + : _currentPoint.WithY(ReadDouble(ref span)); if (!_isOpen) { @@ -263,21 +284,25 @@ namespace Avalonia.Media _geometryContext.LineTo(_currentPoint); } - private void AddCubicBezierCurve(CommandToken commandToken) + private void AddCubicBezierCurve(ref ReadOnlySpan span, bool relative) { - var point1 = commandToken.IsRelative - ? commandToken.ReadRelativePoint(_currentPoint) - : commandToken.ReadPoint(); + var point1 = relative + ? ReadRelativePoint(ref span, _currentPoint) + : ReadPoint(ref span); - var point2 = commandToken.IsRelative - ? commandToken.ReadRelativePoint(_currentPoint) - : commandToken.ReadPoint(); + span = ReadSeparator(span); + + var point2 = relative + ? ReadRelativePoint(ref span, _currentPoint) + : ReadPoint(ref span); _previousControlPoint = point2; - var point3 = commandToken.IsRelative - ? commandToken.ReadRelativePoint(_currentPoint) - : commandToken.ReadPoint(); + span = ReadSeparator(span); + + var point3 = relative + ? ReadRelativePoint(ref span, _currentPoint) + : ReadPoint(ref span); if (!_isOpen) { @@ -289,17 +314,19 @@ namespace Avalonia.Media _currentPoint = point3; } - private void AddQuadraticBezierCurve(CommandToken commandToken) + private void AddQuadraticBezierCurve(ref ReadOnlySpan span, bool relative) { - var start = commandToken.IsRelative - ? commandToken.ReadRelativePoint(_currentPoint) - : commandToken.ReadPoint(); + var start = relative + ? ReadRelativePoint(ref span, _currentPoint) + : ReadPoint(ref span); _previousControlPoint = start; - var end = commandToken.IsRelative - ? commandToken.ReadRelativePoint(_currentPoint) - : commandToken.ReadPoint(); + span = ReadSeparator(span); + + var end = relative + ? ReadRelativePoint(ref span, _currentPoint) + : ReadPoint(ref span); if (!_isOpen) { @@ -311,15 +338,17 @@ namespace Avalonia.Media _currentPoint = end; } - private void AddSmoothCubicBezierCurve(CommandToken commandToken) + private void AddSmoothCubicBezierCurve(ref ReadOnlySpan span, bool relative) { - var point2 = commandToken.IsRelative - ? commandToken.ReadRelativePoint(_currentPoint) - : commandToken.ReadPoint(); + var point2 = relative + ? ReadRelativePoint(ref span, _currentPoint) + : ReadPoint(ref span); + + span = ReadSeparator(span); - var end = commandToken.IsRelative - ? commandToken.ReadRelativePoint(_currentPoint) - : commandToken.ReadPoint(); + var end = relative + ? ReadRelativePoint(ref span, _currentPoint) + : ReadPoint(ref span); if (_previousControlPoint != null) { @@ -338,11 +367,11 @@ namespace Avalonia.Media _currentPoint = end; } - private void AddSmoothQuadraticBezierCurve(CommandToken commandToken) + private void AddSmoothQuadraticBezierCurve(ref ReadOnlySpan span, bool relative) { - var end = commandToken.IsRelative - ? commandToken.ReadRelativePoint(_currentPoint) - : commandToken.ReadPoint(); + var end = relative + ? ReadRelativePoint(ref span, _currentPoint) + : ReadPoint(ref span); if (_previousControlPoint != null) { @@ -359,19 +388,25 @@ namespace Avalonia.Media _currentPoint = end; } - private void AddArc(CommandToken commandToken) + private void AddArc(ref ReadOnlySpan span, bool relative) { - var size = commandToken.ReadSize(); + var size = ReadSize(ref span); - var rotationAngle = commandToken.ReadDouble(); + span = ReadSeparator(span); - var isLargeArc = commandToken.ReadBool(); + var rotationAngle = ReadDouble(ref span); + span = ReadSeparator(span); + var isLargeArc = ReadBool(ref span); - var sweepDirection = commandToken.ReadBool() ? SweepDirection.Clockwise : SweepDirection.CounterClockwise; + span = ReadSeparator(span); + + var sweepDirection = ReadBool(ref span) ? SweepDirection.Clockwise : SweepDirection.CounterClockwise; + + span = ReadSeparator(span); - var end = commandToken.IsRelative - ? commandToken.ReadRelativePoint(_currentPoint) - : commandToken.ReadPoint(); + var end = relative + ? ReadRelativePoint(ref span, _currentPoint) + : ReadPoint(ref span); if (!_isOpen) { @@ -385,227 +420,134 @@ namespace Avalonia.Media _previousControlPoint = null; } - private class CommandToken + private static bool PeekArgument(ReadOnlySpan span) { - private CommandToken(Command command, bool isRelative, List arguments) - { - Command = command; - - IsRelative = isRelative; - - Arguments = arguments; - } - - public Command Command { get; } + span = SkipWhitespace(span); - public bool IsRelative { get; } + return !span.IsEmpty && (span[0] == ',' || span[0] == '-' || char.IsDigit(span[0])); + } - public bool HasImplicitCommands + private static bool ReadArgument(ref ReadOnlySpan remaining, out ReadOnlySpan argument) + { + remaining = SkipWhitespace(remaining); + if (remaining.IsEmpty) { - get - { - if (CurrentPosition == 0 && Arguments.Count > 0) - { - return true; - } - - return CurrentPosition < Arguments.Count - 1; - } + argument = ReadOnlySpan.Empty; + return false; } - private int CurrentPosition { get; set; } - - private List Arguments { get; } - - public static CommandToken Parse(ref ReadOnlySpan span) + var valid = false; + int i = 0; + if (remaining[i] == '-') { - if (!ReadCommand(ref span, out var command, out var isRelative)) - { - throw new InvalidDataException("No path command declared."); - } - - span = span.Slice(1); - span = SkipWhitespace(span); - - var arguments = new List(); - - while (ReadArgument(ref span, out var argument)) - { - arguments.Add(argument.ToString()); - span = ReadSeparator(span); - } - - return new CommandToken(command, isRelative, arguments); + i++; } + for (; i < remaining.Length && char.IsNumber(remaining[i]); i++) valid = true; - public FillRule ReadFillRule() + if (i < remaining.Length && remaining[i] == '.') { - if (CurrentPosition == Arguments.Count) - { - throw new InvalidDataException("Invalid fill rule"); - } - - var value = Arguments[CurrentPosition]; - - CurrentPosition++; - - switch (value) - { - case "0": - { - return FillRule.EvenOdd; - } - - case "1": - { - return FillRule.NonZero; - } - - default: - throw new InvalidDataException("Invalid fill rule"); - } + valid = false; + i++; } + for (; i < remaining.Length && char.IsNumber(remaining[i]); i++) valid = true; - public bool ReadBool() + if (!valid) { - if (CurrentPosition == Arguments.Count) - { - throw new InvalidDataException("Invalid boolean value"); - } - - var value = Arguments[CurrentPosition]; - - CurrentPosition++; - - switch (value) - { - case "1": - { - return true; - } - - case "0": - { - return false; - } - - default: - throw new InvalidDataException("Invalid boolean value"); - } + argument = ReadOnlySpan.Empty; + return false; } + argument = remaining.Slice(0, i); + remaining = remaining.Slice(i); + return true; + } - public double ReadDouble() - { - if (CurrentPosition == Arguments.Count) - { - throw new InvalidDataException("Invalid double value"); - } - - var value = Arguments[CurrentPosition]; - - CurrentPosition++; - - return double.Parse(value, CultureInfo.InvariantCulture); - } - public Size ReadSize() + private static ReadOnlySpan ReadSeparator(ReadOnlySpan span) + { + span = SkipWhitespace(span); + if (!span.IsEmpty && span[0] == ',') { - var width = ReadDouble(); - - var height = ReadDouble(); - - return new Size(width, height); + span = span.Slice(1); } + return span; + } - public Point ReadPoint() - { - var x = ReadDouble(); - - var y = ReadDouble(); - - return new Point(x, y); - } + private static ReadOnlySpan SkipWhitespace(ReadOnlySpan span) + { + int i = 0; + for (; i < span.Length && char.IsWhiteSpace(span[i]); i++) ; + return span.Slice(i); + } - public Point ReadRelativePoint(Point origin) + private bool ReadBool(ref ReadOnlySpan span) + { + if (!ReadArgument(ref span, out var boolValue) || boolValue.Length != 1) { - var x = ReadDouble(); - - var y = ReadDouble(); - - return new Point(origin.X + x, origin.Y + y); + throw new InvalidDataException("Invalid bool rule."); } - - private static bool ReadCommand(ref ReadOnlySpan span, out Command command, out bool relative) + + switch (boolValue[0]) { - span = SkipWhitespace(span); - if (span.IsEmpty) - { - command = default; - relative = false; + case '0': return false; - } - - var c = span[0]; - - if (!s_commands.TryGetValue(char.ToUpperInvariant(c), out command)) - { - throw new InvalidDataException("Unexpected path command '" + c + "'."); - } - - relative = char.IsLower(c); - - return true; + case '1': + return true; + default: + throw new InvalidDataException("Invalid bool rule"); } + } - private static bool ReadArgument(ref ReadOnlySpan remaining, out ReadOnlySpan argument) + private double ReadDouble(ref ReadOnlySpan span) + { + if (!ReadArgument(ref span, out var doubleValue)) { - if (remaining.IsEmpty) - { - argument = ReadOnlySpan.Empty; - return false; - } + throw new InvalidDataException("Invalid double value"); + } - var valid = false; - int i = 0; - if (remaining[i] == '-') - { - i++; - } - for (; i < remaining.Length && char.IsNumber(remaining[i]); i++) valid = true ; + return double.Parse(doubleValue.ToString(), CultureInfo.InvariantCulture); + } - if (i < remaining.Length && remaining[i] == '.') - { - valid = false; - i++; - } - for (; i < remaining.Length && char.IsNumber(remaining[i]); i++) valid = true ; + private Size ReadSize(ref ReadOnlySpan span) + { + var width = ReadDouble(ref span); + span = ReadSeparator(span); + var height = ReadDouble(ref span); + return new Size(width, height); + } - if (!valid) - { - argument = ReadOnlySpan.Empty; - return false; - } - argument = remaining.Slice(0, i); - remaining = remaining.Slice(i); - return true; - } + private Point ReadPoint(ref ReadOnlySpan span) + { + var x = ReadDouble(ref span); + span = ReadSeparator(span); + var y = ReadDouble(ref span); + return new Point(x, y); + } - private static ReadOnlySpan ReadSeparator(ReadOnlySpan span) + private Point ReadRelativePoint(ref ReadOnlySpan span, Point origin) + { + var x = ReadDouble(ref span); + span = ReadSeparator(span); + var y = ReadDouble(ref span); + return new Point(origin.X + x, origin.Y + y); + } + + private bool ReadCommand(ref ReadOnlySpan span, out Command command, out bool relative) + { + span = SkipWhitespace(span); + if (span.IsEmpty) { - span = SkipWhitespace(span); - if (!span.IsEmpty && span[0] == ',') - { - span = span.Slice(1); - } - return SkipWhitespace(span); + command = default; + relative = false; + return false; } - - private static ReadOnlySpan SkipWhitespace(ReadOnlySpan span) + var c = span[0]; + if (!s_commands.TryGetValue(char.ToUpperInvariant(c), out command)) { - int i = 0; - for (; i < span.Length && char.IsWhiteSpace(span[i]); i++) ; - return span.Slice(i); + throw new InvalidDataException("Unexpected path command '" + c + "'."); } + relative = char.IsLower(c); + span = span.Slice(1); + return true; } } } \ No newline at end of file From d3a0507f35c6951daa1cc06c4fe1c9f4e7664af5 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 14 Dec 2017 01:07:14 +0100 Subject: [PATCH 10/42] Implement custom observable for classes. Use a custom `ClassObserver` for observing changes to a control's classes. This saves shaves off about 15% memory use in ControlCatalog after cyling through all tabs. --- .../Styling/TypeNameAndClassSelector.cs | 70 ++++++++++++++++--- 1 file changed, 62 insertions(+), 8 deletions(-) diff --git a/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs b/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs index fb32913e7e..5d48496e8c 100644 --- a/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs +++ b/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs @@ -5,9 +5,11 @@ using System; using System.Collections.Generic; using System.Collections.Specialized; using System.Reactive; +using System.Reactive.Disposables; using System.Reactive.Linq; using System.Reflection; using System.Text; +using Avalonia.Collections; namespace Avalonia.Styling { @@ -122,14 +124,7 @@ namespace Avalonia.Styling { if (subscribe) { - var observable = Observable.FromEventPattern< - NotifyCollectionChangedEventHandler, - NotifyCollectionChangedEventArgs>( - x => control.Classes.CollectionChanged += x, - x => control.Classes.CollectionChanged -= x) - .StartWith((EventPattern)null) - .Select(_ => Matches(control.Classes)) - .DistinctUntilChanged(); + var observable = new ClassObserver(control.Classes, _classes.Value); return new SelectorMatch(observable); } else @@ -204,5 +199,64 @@ namespace Avalonia.Styling return builder.ToString(); } + + private class ClassObserver : IObservable + { + readonly IList _match; + readonly List> _observers = new List>(); + IAvaloniaReadOnlyList _classes; + + public ClassObserver(IAvaloniaReadOnlyList classes, IList match) + { + _classes = classes; + _match = match; + } + + public IDisposable Subscribe(IObserver observer) + { + if (_observers.Count == 0) + { + _classes.CollectionChanged += ClassesChanged; + } + + _observers.Add(observer); + observer.OnNext(GetResult()); + + return Disposable.Create(() => + { + _observers.Remove(observer); + if (_observers.Count == 0) + { + _classes.CollectionChanged -= ClassesChanged; + } + }); + } + + private void ClassesChanged(object sender, NotifyCollectionChangedEventArgs e) + { + if (e.Action != NotifyCollectionChangedAction.Move) + { + foreach (var observer in _observers) + { + observer.OnNext(GetResult()); + } + } + } + + private bool GetResult() + { + int remaining = _match.Count; + + foreach (var c in _classes) + { + if (_match.Contains(c)) + { + --remaining; + } + } + + return remaining == 0; + } + } } } From 6d3381dd6e8992f7dd39b4c3f439f98f531775e5 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 14 Dec 2017 13:56:45 +0100 Subject: [PATCH 11/42] Make ActivatedObservable a custom observable. --- .../Styling/ActivatedObservable.cs | 123 +++++++++++++++--- 1 file changed, 102 insertions(+), 21 deletions(-) diff --git a/src/Avalonia.Styling/Styling/ActivatedObservable.cs b/src/Avalonia.Styling/Styling/ActivatedObservable.cs index 2cd324fff4..1d9af6ed41 100644 --- a/src/Avalonia.Styling/Styling/ActivatedObservable.cs +++ b/src/Avalonia.Styling/Styling/ActivatedObservable.cs @@ -2,8 +2,8 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Reactive; -using System.Reactive.Linq; +using System.Collections.Generic; +using System.Reactive.Disposables; namespace Avalonia.Styling { @@ -17,8 +17,17 @@ namespace Avalonia.Styling /// value. When the activator produces false it will produce /// . /// - internal class ActivatedObservable : ObservableBase, IDescription + internal class ActivatedObservable : IObservable, IDescription { + private static readonly object NotSent = new object(); + private readonly Listener _listener; + private List> _observers; + private IDisposable _activatorSubscription; + private IDisposable _sourceSubscription; + private bool? _active; + private object _value; + private object _last = NotSent; + /// /// Initializes a new instance of the class. /// @@ -36,6 +45,7 @@ namespace Avalonia.Styling Activator = activator; Description = description; Source = source; + _listener = new Listener(this); } /// @@ -53,25 +63,96 @@ namespace Avalonia.Styling /// public IObservable Source { get; } - /// - /// Notifies the provider that an observer is to receive notifications. - /// - /// The observer. - /// IDisposable object used to unsubscribe from the observable sequence. - protected override IDisposable SubscribeCore(IObserver observer) + public IDisposable Subscribe(IObserver observer) + { + var subscribe = _observers == null; + + _observers = _observers ?? new List>(); + _observers.Add(observer); + + if (subscribe) + { + _sourceSubscription = Source.Subscribe(_listener); + _activatorSubscription = Activator.Subscribe(_listener); + } + + return Disposable.Create(() => + { + _observers.Remove(observer); + + if (_observers.Count == 0) + { + _activatorSubscription.Dispose(); + _sourceSubscription.Dispose(); + _activatorSubscription = null; + _sourceSubscription = null; + } + }); + } + + private void NotifyCompleted() + { + foreach (var observer in _observers) + { + observer.OnCompleted(); + } + + _observers = null; + } + + private void NotifyError(Exception error) { - Contract.Requires(observer != null); - - var sourceCompleted = Source.LastOrDefaultAsync().Select(_ => Unit.Default); - var activatorCompleted = Activator.LastOrDefaultAsync().Select(_ => Unit.Default); - var completed = sourceCompleted.Merge(activatorCompleted); - - return Activator - .CombineLatest(Source, (x, y) => new { Active = x, Value = y }) - .Select(x => x.Active ? x.Value : AvaloniaProperty.UnsetValue) - .DistinctUntilChanged() - .TakeUntil(completed) - .Subscribe(observer); + foreach (var observer in _observers) + { + observer.OnError(error); + } + + _observers = null; + } + + private void Update() + { + if (_active.HasValue) + { + var v = _active.Value ? _value : AvaloniaProperty.UnsetValue; + + if (!Equals(v, _last)) + { + foreach (var observer in _observers) + { + observer.OnNext(v); + } + + _last = v; + } + } + } + + private class Listener : IObserver, IObserver + { + private readonly ActivatedObservable _parent; + + public Listener(ActivatedObservable parent) + { + _parent = parent; + } + + void IObserver.OnCompleted() => _parent.NotifyCompleted(); + void IObserver.OnCompleted() => _parent.NotifyCompleted(); + void IObserver.OnError(Exception error) => _parent.NotifyError(error); + void IObserver.OnError(Exception error) => _parent.NotifyError(error); + + void IObserver.OnNext(bool value) + { + _parent._active = value; + _parent.Update(); + } + + void IObserver.OnNext(object value) + { + _parent._value = value; + _parent.Update(); + } } } } From de35eb9fee049179e41076de2fc441454e415959 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 14 Dec 2017 15:56:22 +0100 Subject: [PATCH 12/42] Updated ActivatedSubject. Can be expressed more cleanly given the new `ActivatedObject`. --- .../Styling/ActivatedObservable.cs | 46 +++++++++------ .../Styling/ActivatedSubject.cs | 59 ++++++++++--------- .../ActivatedSubjectTests.cs | 10 +++- 3 files changed, 66 insertions(+), 49 deletions(-) diff --git a/src/Avalonia.Styling/Styling/ActivatedObservable.cs b/src/Avalonia.Styling/Styling/ActivatedObservable.cs index 1d9af6ed41..3c58bb5f9d 100644 --- a/src/Avalonia.Styling/Styling/ActivatedObservable.cs +++ b/src/Avalonia.Styling/Styling/ActivatedObservable.cs @@ -24,8 +24,6 @@ namespace Avalonia.Styling private List> _observers; private IDisposable _activatorSubscription; private IDisposable _sourceSubscription; - private bool? _active; - private object _value; private object _last = NotSent; /// @@ -58,11 +56,21 @@ namespace Avalonia.Styling /// public string Description { get; } + /// + /// Gets a value indicating whether the observable is active. + /// + public bool? IsActive { get; private set; } + /// /// Gets an observable which produces the . /// public IObservable Source { get; } + /// + /// Gets the value that will be produced when is true. + /// + public object Value { get; private set; } + public IDisposable Subscribe(IObserver observer) { var subscribe = _observers == null; @@ -90,7 +98,7 @@ namespace Avalonia.Styling }); } - private void NotifyCompleted() + protected virtual void NotifyCompleted() { foreach (var observer in _observers) { @@ -100,7 +108,7 @@ namespace Avalonia.Styling _observers = null; } - private void NotifyError(Exception error) + protected virtual void NotifyError(Exception error) { foreach (var observer in _observers) { @@ -110,11 +118,23 @@ namespace Avalonia.Styling _observers = null; } + protected virtual void NotifyValue(object value) + { + Value = value; + Update(); + } + + protected virtual void NotifyActive(bool active) + { + IsActive = active; + Update(); + } + private void Update() { - if (_active.HasValue) + if (IsActive.HasValue) { - var v = _active.Value ? _value : AvaloniaProperty.UnsetValue; + var v = IsActive.Value ? Value : AvaloniaProperty.UnsetValue; if (!Equals(v, _last)) { @@ -141,18 +161,8 @@ namespace Avalonia.Styling void IObserver.OnCompleted() => _parent.NotifyCompleted(); void IObserver.OnError(Exception error) => _parent.NotifyError(error); void IObserver.OnError(Exception error) => _parent.NotifyError(error); - - void IObserver.OnNext(bool value) - { - _parent._active = value; - _parent.Update(); - } - - void IObserver.OnNext(object value) - { - _parent._value = value; - _parent.Update(); - } + void IObserver.OnNext(bool value) => _parent.NotifyActive(value); + void IObserver.OnNext(object value) => _parent.NotifyValue(value); } } } diff --git a/src/Avalonia.Styling/Styling/ActivatedSubject.cs b/src/Avalonia.Styling/Styling/ActivatedSubject.cs index 4c2e8dde85..bcbf30649a 100644 --- a/src/Avalonia.Styling/Styling/ActivatedSubject.cs +++ b/src/Avalonia.Styling/Styling/ActivatedSubject.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Reactive.Linq; using System.Reactive.Subjects; namespace Avalonia.Styling @@ -19,9 +18,8 @@ namespace Avalonia.Styling /// internal class ActivatedSubject : ActivatedObservable, ISubject, IDescription { - private bool? _active; private bool _completed; - private object _value; + private object _pushValue; /// /// Initializes a new instance of the class. @@ -35,7 +33,6 @@ namespace Avalonia.Styling string description) : base(activator, source, description) { - Activator.Subscribe(ActivatorChanged, ActivatorError, ActivatorCompleted); } /// @@ -46,53 +43,57 @@ namespace Avalonia.Styling get { return (ISubject)base.Source; } } - /// - /// Notifies all subscribed observers about the end of the sequence. - /// public void OnCompleted() { - if (_active.Value && !_completed) + Source.OnCompleted(); + } + + public void OnError(Exception error) + { + Source.OnError(error); + } + + public void OnNext(object value) + { + _pushValue = value; + + if (IsActive == true && !_completed) { - Source.OnCompleted(); + Source.OnNext(_pushValue); } } - /// - /// Notifies all subscribed observers with the exception. - /// - /// The exception to send to all subscribed observers. - /// is null. - public void OnError(Exception error) + protected override void NotifyCompleted() { - if (_active.Value && !_completed) + base.NotifyCompleted(); + + if (!_completed) { - Source.OnError(error); + Source.OnCompleted(); + _completed = true; } } - /// - /// Notifies all subscribed observers with the value. - /// - /// The value to send to all subscribed observers. - public void OnNext(object value) + protected override void NotifyError(Exception error) { - _value = value; + base.NotifyError(error); - if (_active.Value && !_completed) + if (!_completed) { - Source.OnNext(value); + Source.OnError(error); + _completed = true; } } - private void ActivatorChanged(bool active) + protected override void NotifyActive(bool active) { - bool first = !_active.HasValue; + bool first = !IsActive.HasValue; - _active = active; + base.NotifyActive(active); if (!first) { - Source.OnNext(active ? _value : AvaloniaProperty.UnsetValue); + Source.OnNext(active ? _pushValue : AvaloniaProperty.UnsetValue); } } diff --git a/tests/Avalonia.Styling.UnitTests/ActivatedSubjectTests.cs b/tests/Avalonia.Styling.UnitTests/ActivatedSubjectTests.cs index cc42e3f840..03f91d97a1 100644 --- a/tests/Avalonia.Styling.UnitTests/ActivatedSubjectTests.cs +++ b/tests/Avalonia.Styling.UnitTests/ActivatedSubjectTests.cs @@ -17,6 +17,7 @@ namespace Avalonia.Styling.UnitTests var source = new TestSubject(); var target = new ActivatedSubject(activator, source, string.Empty); + target.Subscribe(); target.OnNext("bar"); Assert.Equal(AvaloniaProperty.UnsetValue, source.Value); activator.OnNext(true); @@ -36,6 +37,7 @@ namespace Avalonia.Styling.UnitTests var source = new TestSubject(); var target = new ActivatedSubject(activator, source, string.Empty); + target.Subscribe(); activator.OnCompleted(); Assert.True(source.Completed); @@ -47,10 +49,14 @@ namespace Avalonia.Styling.UnitTests var activator = new BehaviorSubject(false); var source = new TestSubject(); var target = new ActivatedSubject(activator, source, string.Empty); + var targetError = default(Exception); + var error = new Exception(); - activator.OnError(new Exception()); + target.Subscribe(_ => { }, e => targetError = e); + activator.OnError(error); - Assert.NotNull(source.Error); + Assert.Same(error, source.Error); + Assert.Same(error, targetError); } private class TestSubject : ISubject From cdba9b856d93d597e8192e00236763acab4952ee Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 14 Dec 2017 18:25:57 +0100 Subject: [PATCH 13/42] Make ActivatedValue a custom Observable. And make `ActivatedObservable` inherit from it. --- .../Styling/ActivatedObservable.cs | 125 ++------------- .../Styling/ActivatedValue.cs | 147 +++++++++++++++--- 2 files changed, 139 insertions(+), 133 deletions(-) diff --git a/src/Avalonia.Styling/Styling/ActivatedObservable.cs b/src/Avalonia.Styling/Styling/ActivatedObservable.cs index 3c58bb5f9d..4933daed31 100644 --- a/src/Avalonia.Styling/Styling/ActivatedObservable.cs +++ b/src/Avalonia.Styling/Styling/ActivatedObservable.cs @@ -2,8 +2,6 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Collections.Generic; -using System.Reactive.Disposables; namespace Avalonia.Styling { @@ -17,14 +15,9 @@ namespace Avalonia.Styling /// value. When the activator produces false it will produce /// . /// - internal class ActivatedObservable : IObservable, IDescription + internal class ActivatedObservable : ActivatedValue, IDescription { - private static readonly object NotSent = new object(); - private readonly Listener _listener; - private List> _observers; - private IDisposable _activatorSubscription; private IDisposable _sourceSubscription; - private object _last = NotSent; /// /// Initializes a new instance of the class. @@ -36,133 +29,49 @@ namespace Avalonia.Styling IObservable activator, IObservable source, string description) + : base(activator, AvaloniaProperty.UnsetValue, description) { - Contract.Requires(activator != null); Contract.Requires(source != null); - Activator = activator; - Description = description; Source = source; - _listener = new Listener(this); } - /// - /// Gets the activator observable. - /// - public IObservable Activator { get; } - - /// - /// Gets a description of the binding. - /// - public string Description { get; } - - /// - /// Gets a value indicating whether the observable is active. - /// - public bool? IsActive { get; private set; } - /// /// Gets an observable which produces the . /// public IObservable Source { get; } - /// - /// Gets the value that will be produced when is true. - /// - public object Value { get; private set; } - - public IDisposable Subscribe(IObserver observer) - { - var subscribe = _observers == null; - - _observers = _observers ?? new List>(); - _observers.Add(observer); - - if (subscribe) - { - _sourceSubscription = Source.Subscribe(_listener); - _activatorSubscription = Activator.Subscribe(_listener); - } - - return Disposable.Create(() => - { - _observers.Remove(observer); - - if (_observers.Count == 0) - { - _activatorSubscription.Dispose(); - _sourceSubscription.Dispose(); - _activatorSubscription = null; - _sourceSubscription = null; - } - }); - } + protected override ActivatorListener CreateListener() => new ValueListener(this); - protected virtual void NotifyCompleted() + protected override void Deinitialize() { - foreach (var observer in _observers) - { - observer.OnCompleted(); - } - - _observers = null; + base.Deinitialize(); + _sourceSubscription.Dispose(); + _sourceSubscription = null; } - protected virtual void NotifyError(Exception error) + protected override void Initialize() { - foreach (var observer in _observers) - { - observer.OnError(error); - } - - _observers = null; + base.Initialize(); + _sourceSubscription = Source.Subscribe((ValueListener)Listener); } protected virtual void NotifyValue(object value) { Value = value; - Update(); } - protected virtual void NotifyActive(bool active) + private class ValueListener : ActivatorListener, IObserver { - IsActive = active; - Update(); - } - - private void Update() - { - if (IsActive.HasValue) - { - var v = IsActive.Value ? Value : AvaloniaProperty.UnsetValue; - - if (!Equals(v, _last)) - { - foreach (var observer in _observers) - { - observer.OnNext(v); - } - - _last = v; - } - } - } - - private class Listener : IObserver, IObserver - { - private readonly ActivatedObservable _parent; - - public Listener(ActivatedObservable parent) + public ValueListener(ActivatedObservable parent) + : base(parent) { - _parent = parent; } + protected new ActivatedObservable Parent => (ActivatedObservable)base.Parent; - void IObserver.OnCompleted() => _parent.NotifyCompleted(); - void IObserver.OnCompleted() => _parent.NotifyCompleted(); - void IObserver.OnError(Exception error) => _parent.NotifyError(error); - void IObserver.OnError(Exception error) => _parent.NotifyError(error); - void IObserver.OnNext(bool value) => _parent.NotifyActive(value); - void IObserver.OnNext(object value) => _parent.NotifyValue(value); + void IObserver.OnCompleted() => Parent.NotifyCompleted(); + void IObserver.OnError(Exception error) => Parent.NotifyError(error); + void IObserver.OnNext(object value) => Parent.NotifyValue(value); } } } diff --git a/src/Avalonia.Styling/Styling/ActivatedValue.cs b/src/Avalonia.Styling/Styling/ActivatedValue.cs index 3b9324c059..683121628c 100644 --- a/src/Avalonia.Styling/Styling/ActivatedValue.cs +++ b/src/Avalonia.Styling/Styling/ActivatedValue.cs @@ -2,8 +2,8 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Reactive; -using System.Reactive.Linq; +using System.Collections.Generic; +using System.Reactive.Disposables; namespace Avalonia.Styling { @@ -16,12 +16,13 @@ namespace Avalonia.Styling /// will produce the current value. When the activator /// produces false it will produce . /// - internal class ActivatedValue : ObservableBase, IDescription + internal class ActivatedValue : IObservable, IDescription { - /// - /// The activator. - /// - private readonly IObservable _activator; + private static readonly object NotSent = new object(); + private List> _observers = new List>(); + private IDisposable _activatorSubscription; + private object _value; + private object _last = NotSent; /// /// Initializes a new instance of the class. @@ -34,39 +35,135 @@ namespace Avalonia.Styling object value, string description) { - _activator = activator; + Contract.Requires(activator != null); + + Activator = activator; Value = value; Description = description; + Listener = CreateListener(); } /// - /// Gets the activated value. + /// Gets the activator observable. /// - public object Value - { - get; - } + public IObservable Activator { get; } /// /// Gets a description of the binding. /// - public string Description - { - get; - } + public string Description { get; } + + /// + /// Gets a value indicating whether the activator is active. + /// + public bool? IsActive { get; private set; } /// - /// Notifies the provider that an observer is to receive notifications. + /// Gets the value that will be produced when is true. /// - /// The observer. - /// IDisposable object used to unsubscribe from the observable sequence. - protected override IDisposable SubscribeCore(IObserver observer) + public object Value + { + get => _value; + protected set + { + _value = value; + PublishValue(); + } + } + + protected ActivatorListener Listener { get; } + + public virtual IDisposable Subscribe(IObserver observer) + { + _observers.Add(observer); + + if (_observers.Count == 1) + { + Initialize(); + } + + return Disposable.Create(() => + { + _observers.Remove(observer); + + if (_observers.Count == 0) + { + Deinitialize(); + } + }); + } + + protected virtual ActivatorListener CreateListener() => new ActivatorListener(this); + + protected virtual void Deinitialize() { - Contract.Requires(observer != null); + _activatorSubscription.Dispose(); + _activatorSubscription = null; + } + + protected virtual void Initialize() + { + _activatorSubscription = Activator.Subscribe(Listener); + } + + protected virtual void NotifyCompleted() + { + foreach (var observer in _observers) + { + observer.OnCompleted(); + } + + Deinitialize(); + _observers = null; + } + + protected virtual void NotifyError(Exception error) + { + foreach (var observer in _observers) + { + observer.OnError(error); + } + + Deinitialize(); + _observers = null; + } + + protected virtual void NotifyActive(bool active) + { + IsActive = active; + PublishValue(); + } + + private void PublishValue() + { + if (IsActive.HasValue) + { + var v = IsActive.Value ? Value : AvaloniaProperty.UnsetValue; + + if (!Equals(v, _last)) + { + foreach (var observer in _observers) + { + observer.OnNext(v); + } + + _last = v; + } + } + } + + protected class ActivatorListener : IObserver + { + public ActivatorListener(ActivatedValue parent) + { + Parent = parent; + } + + protected ActivatedValue Parent { get; } - return _activator - .Select(active => active ? Value : AvaloniaProperty.UnsetValue) - .Subscribe(observer); + void IObserver.OnCompleted() => Parent.NotifyCompleted(); + void IObserver.OnError(Exception error) => Parent.NotifyError(error); + void IObserver.OnNext(bool value) => Parent.NotifyActive(value); } } } From fcbde80682939636c7aa49da99018252dfa1b1c2 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 14 Dec 2017 19:35:30 +0100 Subject: [PATCH 14/42] Added a couple of tests. --- src/Avalonia.Styling/Styling/ActivatedValue.cs | 4 ++++ .../ActivatedObservableTests.cs | 7 ++++--- .../ActivatedValueTests.cs | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Styling/Styling/ActivatedValue.cs b/src/Avalonia.Styling/Styling/ActivatedValue.cs index 683121628c..a68ae1e435 100644 --- a/src/Avalonia.Styling/Styling/ActivatedValue.cs +++ b/src/Avalonia.Styling/Styling/ActivatedValue.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Reactive.Disposables; +using Avalonia.Threading; namespace Avalonia.Styling { @@ -75,6 +76,9 @@ namespace Avalonia.Styling public virtual IDisposable Subscribe(IObserver observer) { + Contract.Requires(observer != null); + Dispatcher.UIThread.VerifyAccess(); + _observers.Add(observer); if (_observers.Count == 1) diff --git a/tests/Avalonia.Styling.UnitTests/ActivatedObservableTests.cs b/tests/Avalonia.Styling.UnitTests/ActivatedObservableTests.cs index d5d1d3b01c..7773d4767a 100644 --- a/tests/Avalonia.Styling.UnitTests/ActivatedObservableTests.cs +++ b/tests/Avalonia.Styling.UnitTests/ActivatedObservableTests.cs @@ -54,15 +54,16 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void Should_Complete_When_Activator_Completes() + public void Should_Error_When_Source_Errors() { var activator = new BehaviorSubject(false); var source = new BehaviorSubject(1); var target = new ActivatedObservable(activator, source, string.Empty); + var error = new Exception(); var completed = false; - target.Subscribe(_ => { }, () => completed = true); - activator.OnCompleted(); + target.Subscribe(_ => { }, x => completed = true); + source.OnError(error); Assert.True(completed); } diff --git a/tests/Avalonia.Styling.UnitTests/ActivatedValueTests.cs b/tests/Avalonia.Styling.UnitTests/ActivatedValueTests.cs index e2cc470a1e..92a7c1bd1f 100644 --- a/tests/Avalonia.Styling.UnitTests/ActivatedValueTests.cs +++ b/tests/Avalonia.Styling.UnitTests/ActivatedValueTests.cs @@ -40,6 +40,20 @@ namespace Avalonia.Styling.UnitTests Assert.True(completed); } + [Fact] + public void Should_Error_When_Activator_Errors() + { + var activator = new BehaviorSubject(false); + var target = new ActivatedValue(activator, 1, string.Empty); + var error = new Exception(); + var completed = false; + + target.Subscribe(_ => { }, x => completed = true); + activator.OnError(error); + + Assert.True(completed); + } + [Fact] public void Should_Unsubscribe_From_Activator_When_All_Subscriptions_Disposed() { From 29eafc6be226ab7932fd5abe616b9f43ba48c4ff Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 20 Dec 2017 22:15:35 +0000 Subject: [PATCH 15/42] Added LightweightObservableBase class. And use it for `ActivatedValue`. --- .../Reactive/LightweightObservableBase.cs | 115 ++++++++++++++++++ .../Styling/ActivatedObservable.cs | 6 +- .../Styling/ActivatedSubject.cs | 36 +++--- .../Styling/ActivatedValue.cs | 76 +++--------- 4 files changed, 153 insertions(+), 80 deletions(-) create mode 100644 src/Avalonia.Base/Reactive/LightweightObservableBase.cs diff --git a/src/Avalonia.Base/Reactive/LightweightObservableBase.cs b/src/Avalonia.Base/Reactive/LightweightObservableBase.cs new file mode 100644 index 0000000000..3ae6e5916b --- /dev/null +++ b/src/Avalonia.Base/Reactive/LightweightObservableBase.cs @@ -0,0 +1,115 @@ +using System; +using System.Collections.Generic; +using System.Reactive; +using System.Reactive.Disposables; +using Avalonia.Threading; + +namespace Avalonia.Reactive +{ + /// + /// Lightweight base class for observable implementations. + /// + /// The observable type. + /// + /// is rather heavyweight in terms of allocations and memory + /// usage. This class provides a more lightweight base for some internal observable types + /// in the Avalonia framework. + /// + public abstract class LightweightObservableBase : IObservable + { + private Exception _error; + private List> _observers = new List>(); + + public IDisposable Subscribe(IObserver observer) + { + Contract.Requires(observer != null); + Dispatcher.UIThread.VerifyAccess(); + + if (_observers == null) + { + if (_error != null) + { + observer.OnError(_error); + } + else + { + observer.OnCompleted(); + } + + return Disposable.Empty; + } + + lock (_observers) + { + _observers.Add(observer); + } + + if (_observers.Count == 1) + { + Initialize(); + } + + Subscribed(observer); + + return Disposable.Create(() => + { + _observers?.Remove(observer); + + if (_observers?.Count == 0) + { + Deinitialize(); + _observers.TrimExcess(); + } + }); + } + + protected abstract void Initialize(); + protected abstract void Deinitialize(); + + protected void PublishNext(T value) + { + lock (_observers) + { + foreach (var observer in _observers) + { + observer.OnNext(value); + } + } + } + + protected void PublishCompleted() + { + lock (_observers) + { + foreach (var observer in _observers) + { + observer.OnCompleted(); + } + + _observers = null; + } + + Deinitialize(); + } + + protected void PublishError(Exception error) + { + lock (_observers) + { + foreach (var observer in _observers) + { + observer.OnError(error); + } + + _observers = null; + } + + _error = error; + Deinitialize(); + } + + protected virtual void Subscribed(IObserver observer) + { + } + } +} diff --git a/src/Avalonia.Styling/Styling/ActivatedObservable.cs b/src/Avalonia.Styling/Styling/ActivatedObservable.cs index 4933daed31..5b2774943a 100644 --- a/src/Avalonia.Styling/Styling/ActivatedObservable.cs +++ b/src/Avalonia.Styling/Styling/ActivatedObservable.cs @@ -9,7 +9,7 @@ namespace Avalonia.Styling /// An observable which is switched on or off according to an activator observable. /// /// - /// An has two inputs: an activator observable a + /// An has two inputs: an activator observable and a /// observable which produces the activated value. When the activator /// produces true, the will produce the current activated /// value. When the activator produces false it will produce @@ -69,8 +69,8 @@ namespace Avalonia.Styling } protected new ActivatedObservable Parent => (ActivatedObservable)base.Parent; - void IObserver.OnCompleted() => Parent.NotifyCompleted(); - void IObserver.OnError(Exception error) => Parent.NotifyError(error); + void IObserver.OnCompleted() => Parent.CompletedReceived(); + void IObserver.OnError(Exception error) => Parent.ErrorReceived(error); void IObserver.OnNext(object value) => Parent.NotifyValue(value); } } diff --git a/src/Avalonia.Styling/Styling/ActivatedSubject.cs b/src/Avalonia.Styling/Styling/ActivatedSubject.cs index bcbf30649a..a8446c4bfb 100644 --- a/src/Avalonia.Styling/Styling/ActivatedSubject.cs +++ b/src/Avalonia.Styling/Styling/ActivatedSubject.cs @@ -10,11 +10,9 @@ namespace Avalonia.Styling /// A subject which is switched on or off according to an activator observable. /// /// - /// An has two inputs: an activator observable and either an - /// or a observable which produces the - /// activated value. When the activator produces true, the will - /// produce the current activated value. When the activator produces false it will produce - /// . + /// An extends to + /// be an . When the object is active then values + /// received via will be passed to the source subject. /// internal class ActivatedSubject : ActivatedObservable, ISubject, IDescription { @@ -63,37 +61,37 @@ namespace Avalonia.Styling } } - protected override void NotifyCompleted() + protected override void ActiveChanged(bool active) { - base.NotifyCompleted(); + bool first = !IsActive.HasValue; - if (!_completed) + base.ActiveChanged(active); + + if (!first) { - Source.OnCompleted(); - _completed = true; + Source.OnNext(active ? _pushValue : AvaloniaProperty.UnsetValue); } } - protected override void NotifyError(Exception error) + protected override void CompletedReceived() { - base.NotifyError(error); + base.CompletedReceived(); if (!_completed) { - Source.OnError(error); + Source.OnCompleted(); _completed = true; } } - protected override void NotifyActive(bool active) + protected override void ErrorReceived(Exception error) { - bool first = !IsActive.HasValue; + base.ErrorReceived(error); - base.NotifyActive(active); - - if (!first) + if (!_completed) { - Source.OnNext(active ? _pushValue : AvaloniaProperty.UnsetValue); + Source.OnError(error); + _completed = true; } } diff --git a/src/Avalonia.Styling/Styling/ActivatedValue.cs b/src/Avalonia.Styling/Styling/ActivatedValue.cs index a68ae1e435..5c69c2c9aa 100644 --- a/src/Avalonia.Styling/Styling/ActivatedValue.cs +++ b/src/Avalonia.Styling/Styling/ActivatedValue.cs @@ -2,9 +2,7 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Collections.Generic; -using System.Reactive.Disposables; -using Avalonia.Threading; +using Avalonia.Reactive; namespace Avalonia.Styling { @@ -17,10 +15,9 @@ namespace Avalonia.Styling /// will produce the current value. When the activator /// produces false it will produce . /// - internal class ActivatedValue : IObservable, IDescription + internal class ActivatedValue : LightweightObservableBase, IDescription { private static readonly object NotSent = new object(); - private List> _observers = new List>(); private IDisposable _activatorSubscription; private object _value; private object _last = NotSent; @@ -74,68 +71,35 @@ namespace Avalonia.Styling protected ActivatorListener Listener { get; } - public virtual IDisposable Subscribe(IObserver observer) + protected virtual void ActiveChanged(bool active) { - Contract.Requires(observer != null); - Dispatcher.UIThread.VerifyAccess(); - - _observers.Add(observer); - - if (_observers.Count == 1) - { - Initialize(); - } - - return Disposable.Create(() => - { - _observers.Remove(observer); - - if (_observers.Count == 0) - { - Deinitialize(); - } - }); + IsActive = active; + PublishValue(); } + protected virtual void CompletedReceived() => PublishCompleted(); + protected virtual ActivatorListener CreateListener() => new ActivatorListener(this); - protected virtual void Deinitialize() + protected override void Deinitialize() { _activatorSubscription.Dispose(); _activatorSubscription = null; } - protected virtual void Initialize() - { - _activatorSubscription = Activator.Subscribe(Listener); - } + protected virtual void ErrorReceived(Exception error) => PublishError(error); - protected virtual void NotifyCompleted() + protected override void Initialize() { - foreach (var observer in _observers) - { - observer.OnCompleted(); - } - - Deinitialize(); - _observers = null; + _activatorSubscription = Activator.Subscribe(Listener); } - protected virtual void NotifyError(Exception error) + protected override void Subscribed(IObserver observer) { - foreach (var observer in _observers) + if (IsActive == true) { - observer.OnError(error); + observer.OnNext(Value); } - - Deinitialize(); - _observers = null; - } - - protected virtual void NotifyActive(bool active) - { - IsActive = active; - PublishValue(); } private void PublishValue() @@ -146,11 +110,7 @@ namespace Avalonia.Styling if (!Equals(v, _last)) { - foreach (var observer in _observers) - { - observer.OnNext(v); - } - + PublishNext(v); _last = v; } } @@ -165,9 +125,9 @@ namespace Avalonia.Styling protected ActivatedValue Parent { get; } - void IObserver.OnCompleted() => Parent.NotifyCompleted(); - void IObserver.OnError(Exception error) => Parent.NotifyError(error); - void IObserver.OnNext(bool value) => Parent.NotifyActive(value); + void IObserver.OnCompleted() => Parent.CompletedReceived(); + void IObserver.OnError(Exception error) => Parent.ErrorReceived(error); + void IObserver.OnNext(bool value) => Parent.ActiveChanged(value); } } } From dadc30d84ebfbb257c0a2625937efc781b525b49 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 21 Dec 2017 00:26:36 +0000 Subject: [PATCH 16/42] Use lightweight observable for ExpressionObserver. --- .../Data/Core/ExpressionObserver.cs | 105 ++++++++---------- .../Reactive/LightweightObservableBase.cs | 69 ++++++++---- .../Styling/ActivatedValue.cs | 4 +- 3 files changed, 98 insertions(+), 80 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/ExpressionObserver.cs b/src/Avalonia.Base/Data/Core/ExpressionObserver.cs index 14bc09f5b7..3a25407133 100644 --- a/src/Avalonia.Base/Data/Core/ExpressionObserver.cs +++ b/src/Avalonia.Base/Data/Core/ExpressionObserver.cs @@ -4,18 +4,19 @@ using System; using System.Collections.Generic; using System.Reactive; -using System.Reactive.Disposables; using System.Reactive.Linq; -using System.Reactive.Subjects; using Avalonia.Data; using Avalonia.Data.Core.Plugins; +using Avalonia.Reactive; namespace Avalonia.Data.Core { /// /// Observes and sets the value of an expression on an object. /// - public class ExpressionObserver : ObservableBase, IDescription + public class ExpressionObserver : LightweightObservableBase, + IDescription, + IObserver { /// /// An ordered collection of property accessor plugins that can be used to customize @@ -54,9 +55,10 @@ namespace Avalonia.Data.Core private static readonly object UninitializedValue = new object(); private readonly ExpressionNode _node; - private readonly Subject _finished; - private readonly object _root; - private IObservable _result; + private IDisposable _nodeSubscription; + private object _root; + private IDisposable _rootSubscription; + private WeakReference _value; /// /// Initializes a new instance of the class. @@ -107,7 +109,6 @@ namespace Avalonia.Data.Core Expression = expression; Description = description ?? expression; _node = Parse(expression, enableDataValidation); - _finished = new Subject(); _root = rootObservable; } @@ -135,8 +136,6 @@ namespace Avalonia.Data.Core Expression = expression; Description = description ?? expression; _node = Parse(expression, enableDataValidation); - _finished = new Subject(); - _node.Target = new WeakReference(rootGetter()); _root = update.Select(x => rootGetter()); } @@ -203,27 +202,42 @@ namespace Avalonia.Data.Core } } - /// - protected override IDisposable SubscribeCore(IObserver observer) + void IObserver.OnNext(object value) { - if (_result == null) - { - var source = (IObservable)_node; + var broken = BindingNotification.ExtractError(value) as MarkupBindingChainException; + broken?.Commit(Description); + _value = new WeakReference(value); + PublishNext(value); + } - if (_finished != null) - { - source = source.TakeUntil(_finished); - } + void IObserver.OnCompleted() + { + } - _result = Observable.Using(StartRoot, _ => source) - .Select(ToWeakReference) - .Publish(UninitializedValue) - .RefCount() - .Where(x => x != UninitializedValue) - .Select(Translate); - } + void IObserver.OnError(Exception error) + { + } + + protected override void Initialize() + { + _value = null; + _nodeSubscription = _node.Subscribe(this); + StartRoot(); + } + + protected override void Deinitialize() + { + _rootSubscription?.Dispose(); + _nodeSubscription?.Dispose(); + _rootSubscription = _nodeSubscription = null; + } - return _result.Subscribe(observer); + protected override void Subscribed(IObserver observer, bool first) + { + if (!first && _value != null && _value.TryGetTarget(out var value)) + { + observer.OnNext(value); + } } private static ExpressionNode Parse(string expression, bool enableDataValidation) @@ -238,42 +252,19 @@ namespace Avalonia.Data.Core } } - private static object ToWeakReference(object o) + private void StartRoot() { - return o is BindingNotification ? o : new WeakReference(o); - } - - private object Translate(object o) - { - if (o is WeakReference weak) + if (_root is IObservable observable) { - return weak.Target; + _rootSubscription = observable.Subscribe( + x => _node.Target = new WeakReference(x != AvaloniaProperty.UnsetValue ? x : null), + x => PublishCompleted(), + () => PublishCompleted()); } - else if (BindingNotification.ExtractError(o) is MarkupBindingChainException broken) - { - broken.Commit(Description); - } - - return o; - } - - private IDisposable StartRoot() - { - switch (_root) + else { - case IObservable observable: - return observable.Subscribe( - x => _node.Target = new WeakReference(x != AvaloniaProperty.UnsetValue ? x : null), - _ => _finished.OnNext(Unit.Default), - () => _finished.OnNext(Unit.Default)); - case WeakReference weak: - _node.Target = weak; - break; - default: - throw new AvaloniaInternalException("The ExpressionObserver._root member should only be either an observable or WeakReference."); + _node.Target = (WeakReference)_root; } - - return Disposable.Empty; } } } diff --git a/src/Avalonia.Base/Reactive/LightweightObservableBase.cs b/src/Avalonia.Base/Reactive/LightweightObservableBase.cs index 3ae6e5916b..ced2c03b01 100644 --- a/src/Avalonia.Base/Reactive/LightweightObservableBase.cs +++ b/src/Avalonia.Base/Reactive/LightweightObservableBase.cs @@ -39,26 +39,34 @@ namespace Avalonia.Reactive return Disposable.Empty; } + var first = _observers.Count == 0; + lock (_observers) { _observers.Add(observer); } - if (_observers.Count == 1) + if (first) { Initialize(); } - Subscribed(observer); + Subscribed(observer, first); return Disposable.Create(() => { - _observers?.Remove(observer); - - if (_observers?.Count == 0) + if (_observers != null) { - Deinitialize(); - _observers.TrimExcess(); + lock (_observers) + { + _observers?.Remove(observer); + + if (_observers?.Count == 0) + { + Deinitialize(); + _observers.TrimExcess(); + } + } } }); } @@ -68,9 +76,16 @@ namespace Avalonia.Reactive protected void PublishNext(T value) { - lock (_observers) + if (_observers != null) { - foreach (var observer in _observers) + IObserver[] observers; + + lock (_observers) + { + observers = _observers.ToArray(); + } + + foreach (var observer in observers) { observer.OnNext(value); } @@ -79,36 +94,48 @@ namespace Avalonia.Reactive protected void PublishCompleted() { - lock (_observers) + if (_observers != null) { - foreach (var observer in _observers) + IObserver[] observers; + + lock (_observers) + { + observers = _observers.ToArray(); + _observers = null; + } + + foreach (var observer in observers) { observer.OnCompleted(); } - _observers = null; + Deinitialize(); } - - Deinitialize(); } protected void PublishError(Exception error) { - lock (_observers) + if (_observers != null) { - foreach (var observer in _observers) + IObserver[] observers; + + lock (_observers) + { + observers = _observers.ToArray(); + _observers = null; + } + + foreach (var observer in observers) { observer.OnError(error); } - _observers = null; + _error = error; + Deinitialize(); } - - _error = error; - Deinitialize(); } - protected virtual void Subscribed(IObserver observer) + protected virtual void Subscribed(IObserver observer, bool first) { } } diff --git a/src/Avalonia.Styling/Styling/ActivatedValue.cs b/src/Avalonia.Styling/Styling/ActivatedValue.cs index 5c69c2c9aa..908d89b751 100644 --- a/src/Avalonia.Styling/Styling/ActivatedValue.cs +++ b/src/Avalonia.Styling/Styling/ActivatedValue.cs @@ -94,9 +94,9 @@ namespace Avalonia.Styling _activatorSubscription = Activator.Subscribe(Listener); } - protected override void Subscribed(IObserver observer) + protected override void Subscribed(IObserver observer, bool first) { - if (IsActive == true) + if (IsActive == true && !first) { observer.OnNext(Value); } From 0f1664a84672f49a85f26a7e65718c631d8c81c4 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 24 Dec 2017 21:05:17 +0000 Subject: [PATCH 17/42] Use custom rx for observing properties. --- src/Avalonia.Base/AvaloniaObjectExtensions.cs | 46 ++----------------- .../Reactive/AvaloniaObservable.cs | 42 ----------------- .../Reactive/AvaloniaPropertyObservable.cs | 46 +++++++++++++++++++ 3 files changed, 51 insertions(+), 83 deletions(-) delete mode 100644 src/Avalonia.Base/Reactive/AvaloniaObservable.cs create mode 100644 src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs diff --git a/src/Avalonia.Base/AvaloniaObjectExtensions.cs b/src/Avalonia.Base/AvaloniaObjectExtensions.cs index 1da2ecb942..71e6b9474f 100644 --- a/src/Avalonia.Base/AvaloniaObjectExtensions.cs +++ b/src/Avalonia.Base/AvaloniaObjectExtensions.cs @@ -41,27 +41,7 @@ namespace Avalonia Contract.Requires(o != null); Contract.Requires(property != null); - return new AvaloniaObservable( - observer => - { - EventHandler handler = (s, e) => - { - if (e.Property == property) - { - observer.OnNext(e.NewValue); - } - }; - - observer.OnNext(o.GetValue(property)); - - o.PropertyChanged += handler; - - return Disposable.Create(() => - { - o.PropertyChanged -= handler; - }); - }, - GetDescription(o, property)); + return new AvaloniaPropertyObservable(o, property); } /// @@ -79,7 +59,7 @@ namespace Avalonia Contract.Requires(o != null); Contract.Requires(property != null); - return o.GetObservable((AvaloniaProperty)property).Cast(); + return new AvaloniaPropertyObservable(o, property); } /// @@ -100,25 +80,9 @@ namespace Avalonia Contract.Requires(o != null); Contract.Requires(property != null); - return new AvaloniaObservable>( - observer => - { - EventHandler handler = (s, e) => - { - if (e.Property == property) - { - observer.OnNext(Tuple.Create((T)e.OldValue, (T)e.NewValue)); - } - }; - - o.PropertyChanged += handler; - - return Disposable.Create(() => - { - o.PropertyChanged -= handler; - }); - }, - GetDescription(o, property)); + return new AvaloniaPropertyObservable(o, property) + .Buffer(2, 1) + .Select(x => Tuple.Create(x[0], x[1])); } /// diff --git a/src/Avalonia.Base/Reactive/AvaloniaObservable.cs b/src/Avalonia.Base/Reactive/AvaloniaObservable.cs deleted file mode 100644 index b59d8e5226..0000000000 --- a/src/Avalonia.Base/Reactive/AvaloniaObservable.cs +++ /dev/null @@ -1,42 +0,0 @@ -// 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 System; -using System.Reactive; -using System.Reactive.Disposables; - -namespace Avalonia.Reactive -{ - /// - /// An with an additional description. - /// - /// The type of the elements in the sequence. - public class AvaloniaObservable : ObservableBase, IDescription - { - private readonly Func, IDisposable> _subscribe; - - /// - /// Initializes a new instance of the class. - /// - /// The subscribe function. - /// The description of the observable. - public AvaloniaObservable(Func, IDisposable> subscribe, string description) - { - Contract.Requires(subscribe != null); - - _subscribe = subscribe; - Description = description; - } - - /// - /// Gets the description of the observable. - /// - public string Description { get; } - - /// - protected override IDisposable SubscribeCore(IObserver observer) - { - return _subscribe(observer) ?? Disposable.Empty; - } - } -} \ No newline at end of file diff --git a/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs b/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs new file mode 100644 index 0000000000..c19423b623 --- /dev/null +++ b/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs @@ -0,0 +1,46 @@ +using System; + +namespace Avalonia.Reactive +{ + public class AvaloniaPropertyObservable : LightweightObservableBase, IDescription + { + private readonly IAvaloniaObject _target; + private readonly AvaloniaProperty _property; + private T _value; + + public AvaloniaPropertyObservable( + IAvaloniaObject target, + AvaloniaProperty property) + { + _target = target; + _property = property; + } + + public string Description => $"{_target.GetType().Name}.{_property.Name}"; + + protected override void Initialize() + { + _value = (T)_target.GetValue(_property); + _target.PropertyChanged += PropertyChanged; + } + + protected override void Deinitialize() + { + _target.PropertyChanged -= PropertyChanged; + } + + protected override void Subscribed(IObserver observer, bool first) + { + observer.OnNext(_value); + } + + private void PropertyChanged(object sender, AvaloniaPropertyChangedEventArgs e) + { + if (e.Property == _property) + { + _value = (T)e.NewValue; + PublishNext(_value); + } + } + } +} From 34474af8350f0312aa85f287ddace77db8a05e16 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 28 Dec 2017 11:54:42 +0000 Subject: [PATCH 18/42] Make AvaloniaPropertyObservable use weak refs. And remove `WeakPropertyChangedObservable`/`GetWeakObservable`. --- src/Avalonia.Base/AvaloniaObjectExtensions.cs | 23 ++--- .../Plugins/AvaloniaPropertyAccessorPlugin.cs | 2 +- .../Reactive/AvaloniaPropertyObservable.cs | 17 ++-- .../Reactive/WeakPropertyChangedObservable.cs | 85 ------------------- 4 files changed, 19 insertions(+), 108 deletions(-) delete mode 100644 src/Avalonia.Base/Reactive/WeakPropertyChangedObservable.cs diff --git a/src/Avalonia.Base/AvaloniaObjectExtensions.cs b/src/Avalonia.Base/AvaloniaObjectExtensions.cs index 71e6b9474f..75e56c7186 100644 --- a/src/Avalonia.Base/AvaloniaObjectExtensions.cs +++ b/src/Avalonia.Base/AvaloniaObjectExtensions.cs @@ -36,6 +36,9 @@ namespace Avalonia /// An observable which fires immediately with the current value of the property on the /// object and subsequently each time the property value changes. /// + /// + /// The subscription to is created using a weak reference. + /// public static IObservable GetObservable(this IAvaloniaObject o, AvaloniaProperty property) { Contract.Requires(o != null); @@ -54,6 +57,9 @@ namespace Avalonia /// An observable which fires immediately with the current value of the property on the /// object and subsequently each time the property value changes. /// + /// + /// The subscription to is created using a weak reference. + /// public static IObservable GetObservable(this IAvaloniaObject o, AvaloniaProperty property) { Contract.Requires(o != null); @@ -130,23 +136,6 @@ namespace Avalonia o.GetObservable(property)); } - /// - /// Gets a weak observable for a . - /// - /// The object. - /// The property. - /// An observable. - public static IObservable GetWeakObservable(this IAvaloniaObject o, AvaloniaProperty property) - { - Contract.Requires(o != null); - Contract.Requires(property != null); - - return new WeakPropertyChangedObservable( - new WeakReference(o), - property, - GetDescription(o, property)); - } - /// /// Binds a property on an to an . /// diff --git a/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs index 8cbcaa8233..48edb218dc 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs @@ -153,7 +153,7 @@ namespace Avalonia.Data.Core.Plugins protected override void SubscribeCore(IObserver observer) { - _subscription = Instance?.GetWeakObservable(_property).Subscribe(observer); + _subscription = Instance?.GetObservable(_property).Subscribe(observer); } } } diff --git a/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs b/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs index c19423b623..0e07065d6c 100644 --- a/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs +++ b/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs @@ -1,10 +1,11 @@ using System; +using Avalonia.Utilities; namespace Avalonia.Reactive { public class AvaloniaPropertyObservable : LightweightObservableBase, IDescription { - private readonly IAvaloniaObject _target; + private readonly WeakReference _target; private readonly AvaloniaProperty _property; private T _value; @@ -12,7 +13,7 @@ namespace Avalonia.Reactive IAvaloniaObject target, AvaloniaProperty property) { - _target = target; + _target = new WeakReference(target); _property = property; } @@ -20,13 +21,19 @@ namespace Avalonia.Reactive protected override void Initialize() { - _value = (T)_target.GetValue(_property); - _target.PropertyChanged += PropertyChanged; + if (_target.TryGetTarget(out var target)) + { + _value = (T)target.GetValue(_property); + target.PropertyChanged += PropertyChanged; + } } protected override void Deinitialize() { - _target.PropertyChanged -= PropertyChanged; + if (_target.TryGetTarget(out var target)) + { + target.PropertyChanged -= PropertyChanged; + } } protected override void Subscribed(IObserver observer, bool first) diff --git a/src/Avalonia.Base/Reactive/WeakPropertyChangedObservable.cs b/src/Avalonia.Base/Reactive/WeakPropertyChangedObservable.cs deleted file mode 100644 index aa5f553865..0000000000 --- a/src/Avalonia.Base/Reactive/WeakPropertyChangedObservable.cs +++ /dev/null @@ -1,85 +0,0 @@ -// 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 System; -using System.Reactive; -using System.Reactive.Disposables; -using System.Reactive.Linq; -using System.Reactive.Subjects; -using Avalonia.Utilities; - -namespace Avalonia.Reactive -{ - internal class WeakPropertyChangedObservable : ObservableBase, - IWeakSubscriber, IDescription - { - private WeakReference _sourceReference; - private readonly AvaloniaProperty _property; - private readonly Subject _changed = new Subject(); - - private int _count; - - public WeakPropertyChangedObservable( - WeakReference source, - AvaloniaProperty property, - string description) - { - _sourceReference = source; - _property = property; - Description = description; - } - - public string Description { get; } - - public void OnEvent(object sender, AvaloniaPropertyChangedEventArgs e) - { - if (e.Property == _property) - { - _changed.OnNext(e.NewValue); - } - } - - protected override IDisposable SubscribeCore(IObserver observer) - { - IAvaloniaObject instance; - - if (_sourceReference.TryGetTarget(out instance)) - { - if (_count++ == 0) - { - WeakSubscriptionManager.Subscribe( - instance, - nameof(instance.PropertyChanged), - this); - } - - observer.OnNext(instance.GetValue(_property)); - - return Observable.Using(() => Disposable.Create(DecrementCount), _ => _changed) - .Subscribe(observer); - } - else - { - _changed.OnCompleted(); - observer.OnCompleted(); - return Disposable.Empty; - } - } - - private void DecrementCount() - { - if (--_count == 0) - { - IAvaloniaObject instance; - - if (_sourceReference.TryGetTarget(out instance)) - { - WeakSubscriptionManager.Unsubscribe( - instance, - nameof(instance.PropertyChanged), - this); - } - } - } - } -} From 3150d3f10aa9b22f06b0f9ecee16dc8f60f680f1 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 28 Dec 2017 12:29:01 +0000 Subject: [PATCH 19/42] Added GetPropertyChangedObservable. Replaced `GetObservableWithHistory` with `GetPropertyChangedObservable`. --- src/Avalonia.Base/AvaloniaObjectExtensions.cs | 18 ++++---- .../AvaloniaPropertyChangedObservable.cs | 46 +++++++++++++++++++ .../Reactive/AvaloniaPropertyObservable.cs | 3 +- .../Mixins/ContentControlMixin.cs | 40 ++++++---------- 4 files changed, 69 insertions(+), 38 deletions(-) create mode 100644 src/Avalonia.Base/Reactive/AvaloniaPropertyChangedObservable.cs diff --git a/src/Avalonia.Base/AvaloniaObjectExtensions.cs b/src/Avalonia.Base/AvaloniaObjectExtensions.cs index 75e56c7186..393482cccf 100644 --- a/src/Avalonia.Base/AvaloniaObjectExtensions.cs +++ b/src/Avalonia.Base/AvaloniaObjectExtensions.cs @@ -69,26 +69,24 @@ namespace Avalonia } /// - /// Gets an observable for a . + /// Gets an observable that listens for property changed events for an + /// . /// /// The object. - /// The type of the property. /// The property. /// - /// An observable which when subscribed pushes the old and new values of the property each - /// time it is changed. Note that the observable returned from this method does not fire - /// with the current value of the property immediately. + /// An observable which when subscribed pushes the property changed event args + /// each time a event is raised + /// for the specified property. /// - public static IObservable> GetObservableWithHistory( + public static IObservable GetPropertyChangedObservable( this IAvaloniaObject o, - AvaloniaProperty property) + AvaloniaProperty property) { Contract.Requires(o != null); Contract.Requires(property != null); - return new AvaloniaPropertyObservable(o, property) - .Buffer(2, 1) - .Select(x => Tuple.Create(x[0], x[1])); + return new AvaloniaPropertyChangedObservable(o, property); } /// diff --git a/src/Avalonia.Base/Reactive/AvaloniaPropertyChangedObservable.cs b/src/Avalonia.Base/Reactive/AvaloniaPropertyChangedObservable.cs new file mode 100644 index 0000000000..5ef0d25133 --- /dev/null +++ b/src/Avalonia.Base/Reactive/AvaloniaPropertyChangedObservable.cs @@ -0,0 +1,46 @@ +using System; + +namespace Avalonia.Reactive +{ + internal class AvaloniaPropertyChangedObservable : + LightweightObservableBase, + IDescription + { + private readonly WeakReference _target; + private readonly AvaloniaProperty _property; + + public AvaloniaPropertyChangedObservable( + IAvaloniaObject target, + AvaloniaProperty property) + { + _target = new WeakReference(target); + _property = property; + } + + public string Description => $"{_target.GetType().Name}.{_property.Name}"; + + protected override void Initialize() + { + if (_target.TryGetTarget(out var target)) + { + target.PropertyChanged += PropertyChanged; + } + } + + protected override void Deinitialize() + { + if (_target.TryGetTarget(out var target)) + { + target.PropertyChanged -= PropertyChanged; + } + } + + private void PropertyChanged(object sender, AvaloniaPropertyChangedEventArgs e) + { + if (e.Property == _property) + { + PublishNext(e); + } + } + } +} diff --git a/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs b/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs index 0e07065d6c..4385ab13ef 100644 --- a/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs +++ b/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs @@ -1,9 +1,8 @@ using System; -using Avalonia.Utilities; namespace Avalonia.Reactive { - public class AvaloniaPropertyObservable : LightweightObservableBase, IDescription + internal class AvaloniaPropertyObservable : LightweightObservableBase, IDescription { private readonly WeakReference _target; private readonly AvaloniaProperty _property; diff --git a/src/Avalonia.Controls/Mixins/ContentControlMixin.cs b/src/Avalonia.Controls/Mixins/ContentControlMixin.cs index daaf3fd5cf..95193c0432 100644 --- a/src/Avalonia.Controls/Mixins/ContentControlMixin.cs +++ b/src/Avalonia.Controls/Mixins/ContentControlMixin.cs @@ -49,11 +49,9 @@ namespace Avalonia.Controls.Mixins Contract.Requires(content != null); Contract.Requires(logicalChildrenSelector != null); - EventHandler templateApplied = (s, ev) => + void TemplateApplied(object s, RoutedEventArgs ev) { - var sender = s as TControl; - - if (sender != null) + if (s is TControl sender) { var e = (TemplateAppliedEventArgs)ev; var presenter = (IControl)e.NameScope.Find(presenterName); @@ -64,12 +62,12 @@ namespace Avalonia.Controls.Mixins var logicalChildren = logicalChildrenSelector(sender); var subscription = presenter - .GetObservableWithHistory(ContentPresenter.ChildProperty) - .Subscribe(child => UpdateLogicalChild( + .GetPropertyChangedObservable(ContentPresenter.ChildProperty) + .Subscribe(c => UpdateLogicalChild( sender, - logicalChildren, - child.Item1, - child.Item2)); + logicalChildren, + c.OldValue, + c.NewValue)); UpdateLogicalChild( sender, @@ -80,18 +78,16 @@ namespace Avalonia.Controls.Mixins subscriptions.Value.Add(sender, subscription); } } - }; + } TemplatedControl.TemplateAppliedEvent.AddClassHandler( typeof(TControl), - templateApplied, + TemplateApplied, RoutingStrategies.Direct); content.Changed.Subscribe(e => { - var sender = e.Sender as TControl; - - if (sender != null) + if (e.Sender is TControl sender) { var logicalChildren = logicalChildrenSelector(sender); UpdateLogicalChild(sender, logicalChildren, e.OldValue, e.NewValue); @@ -100,9 +96,7 @@ namespace Avalonia.Controls.Mixins Control.TemplatedParentProperty.Changed.Subscribe(e => { - var sender = e.Sender as TControl; - - if (sender != null) + if (e.Sender is TControl sender) { var logicalChild = logicalChildrenSelector(sender).FirstOrDefault() as IControl; logicalChild?.SetValue(Control.TemplatedParentProperty, sender.TemplatedParent); @@ -111,13 +105,9 @@ namespace Avalonia.Controls.Mixins TemplatedControl.TemplateProperty.Changed.Subscribe(e => { - var sender = e.Sender as TControl; - - if (sender != null) + if (e.Sender is TControl sender) { - IDisposable subscription; - - if (subscriptions.Value.TryGetValue(sender, out subscription)) + if (subscriptions.Value.TryGetValue(sender, out IDisposable subscription)) { subscription.Dispose(); subscriptions.Value.Remove(sender); @@ -134,9 +124,7 @@ namespace Avalonia.Controls.Mixins { if (oldValue != newValue) { - var child = oldValue as IControl; - - if (child != null) + if (oldValue is IControl child) { logicalChildren.Remove(child); } From 360a2603c3285482f7d981c417d4b66e6cfafab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Karnok?= Date: Sat, 26 May 2018 00:36:39 +0200 Subject: [PATCH 20/42] Fix LightweightObservableBase's concurrency, save 1 allocation. --- .../Reactive/LightweightObservableBase.cs | 122 +++++++++++++----- 1 file changed, 91 insertions(+), 31 deletions(-) diff --git a/src/Avalonia.Base/Reactive/LightweightObservableBase.cs b/src/Avalonia.Base/Reactive/LightweightObservableBase.cs index ced2c03b01..a2786d63da 100644 --- a/src/Avalonia.Base/Reactive/LightweightObservableBase.cs +++ b/src/Avalonia.Base/Reactive/LightweightObservableBase.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Reactive; using System.Reactive.Disposables; +using System.Threading; using Avalonia.Threading; namespace Avalonia.Reactive @@ -25,25 +26,35 @@ namespace Avalonia.Reactive Contract.Requires(observer != null); Dispatcher.UIThread.VerifyAccess(); - if (_observers == null) + var first = false; + + for (; ; ) { - if (_error != null) - { - observer.OnError(_error); - } - else + if (Volatile.Read(ref _observers) == null) { - observer.OnCompleted(); - } + if (_error != null) + { + observer.OnError(_error); + } + else + { + observer.OnCompleted(); + } - return Disposable.Empty; - } + return Disposable.Empty; + } - var first = _observers.Count == 0; + lock (this) + { + if (_observers == null) + { + continue; + } - lock (_observers) - { - _observers.Add(observer); + first = _observers.Count == 0; + _observers.Add(observer); + break; + } } if (first) @@ -53,22 +64,57 @@ namespace Avalonia.Reactive Subscribed(observer, first); - return Disposable.Create(() => + return new RemoveObserver(this, observer); + } + + void Remove(IObserver observer) + { + if (Volatile.Read(ref _observers) != null) { - if (_observers != null) + lock (this) { - lock (_observers) + var observers = _observers; + + if (observers != null) { - _observers?.Remove(observer); + observers.Remove(observer); - if (_observers?.Count == 0) + if (observers.Count == 0) + { + observers.TrimExcess(); + } + else { - Deinitialize(); - _observers.TrimExcess(); + return; } + } else + { + return; } } - }); + + Deinitialize(); + } + } + + sealed class RemoveObserver : IDisposable + { + LightweightObservableBase _parent; + + IObserver _observer; + + public RemoveObserver(LightweightObservableBase parent, IObserver observer) + { + _parent = parent; + Volatile.Write(ref _observer, observer); + } + + public void Dispose() + { + var observer = _observer; + Interlocked.Exchange(ref _parent, null)?.Remove(observer); + _observer = null; + } } protected abstract void Initialize(); @@ -76,12 +122,16 @@ namespace Avalonia.Reactive protected void PublishNext(T value) { - if (_observers != null) + if (Volatile.Read(ref _observers) != null) { IObserver[] observers; - lock (_observers) + lock (this) { + if (_observers == null) + { + return; + } observers = _observers.ToArray(); } @@ -94,14 +144,18 @@ namespace Avalonia.Reactive protected void PublishCompleted() { - if (_observers != null) + if (Volatile.Read(ref _observers) != null) { IObserver[] observers; - lock (_observers) + lock (this) { + if (_observers == null) + { + return; + } observers = _observers.ToArray(); - _observers = null; + Volatile.Write(ref _observers, null); } foreach (var observer in observers) @@ -115,14 +169,21 @@ namespace Avalonia.Reactive protected void PublishError(Exception error) { - if (_observers != null) + if (Volatile.Read(ref _observers) != null) { + IObserver[] observers; - lock (_observers) + lock (this) { + if (_observers == null) + { + return; + } + + _error = error; observers = _observers.ToArray(); - _observers = null; + Volatile.Write(ref _observers, null); } foreach (var observer in observers) @@ -130,7 +191,6 @@ namespace Avalonia.Reactive observer.OnError(error); } - _error = error; Deinitialize(); } } From 9b0aff8cc1c0a1ed18333355459394aee75b8268 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 01:14:49 +0200 Subject: [PATCH 21/42] Use LightweightObservableBase in TypeNameAndClassSelector --- .../Styling/TypeNameAndClassSelector.cs | 32 ++++--------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs b/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs index 5d48496e8c..93d4e14f27 100644 --- a/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs +++ b/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs @@ -4,12 +4,10 @@ using System; using System.Collections.Generic; using System.Collections.Specialized; -using System.Reactive; -using System.Reactive.Disposables; -using System.Reactive.Linq; using System.Reflection; using System.Text; using Avalonia.Collections; +using Avalonia.Reactive; namespace Avalonia.Styling { @@ -200,10 +198,9 @@ namespace Avalonia.Styling return builder.ToString(); } - private class ClassObserver : IObservable + private class ClassObserver : LightweightObservableBase { readonly IList _match; - readonly List> _observers = new List>(); IAvaloniaReadOnlyList _classes; public ClassObserver(IAvaloniaReadOnlyList classes, IList match) @@ -212,34 +209,19 @@ namespace Avalonia.Styling _match = match; } - public IDisposable Subscribe(IObserver observer) - { - if (_observers.Count == 0) - { - _classes.CollectionChanged += ClassesChanged; - } + protected override void Deinitialize() => _classes.CollectionChanged -= ClassesChanged; + protected override void Initialize() => _classes.CollectionChanged += ClassesChanged; - _observers.Add(observer); + protected override void Subscribed(IObserver observer, bool first) + { observer.OnNext(GetResult()); - - return Disposable.Create(() => - { - _observers.Remove(observer); - if (_observers.Count == 0) - { - _classes.CollectionChanged -= ClassesChanged; - } - }); } private void ClassesChanged(object sender, NotifyCollectionChangedEventArgs e) { if (e.Action != NotifyCollectionChangedAction.Move) { - foreach (var observer in _observers) - { - observer.OnNext(GetResult()); - } + PublishNext(GetResult()); } } From de325ec501f2104ea2870e92dd5ea2ef792fec5d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 01:44:59 +0200 Subject: [PATCH 22/42] Use LightweightObservableBase for WeakCollectionChangedObservable --- .../NotifyCollectionChangedExtensions.cs | 59 ++++++------------- 1 file changed, 17 insertions(+), 42 deletions(-) diff --git a/src/Avalonia.Base/Collections/NotifyCollectionChangedExtensions.cs b/src/Avalonia.Base/Collections/NotifyCollectionChangedExtensions.cs index d295cb91ce..3a355bcb48 100644 --- a/src/Avalonia.Base/Collections/NotifyCollectionChangedExtensions.cs +++ b/src/Avalonia.Base/Collections/NotifyCollectionChangedExtensions.cs @@ -2,13 +2,9 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Collections; -using System.Collections.Generic; using System.Collections.Specialized; -using System.Reactive; -using System.Reactive.Disposables; using System.Reactive.Linq; -using System.Reactive.Subjects; +using Avalonia.Reactive; using Avalonia.Utilities; namespace Avalonia.Collections @@ -43,9 +39,8 @@ namespace Avalonia.Collections Contract.Requires(collection != null); Contract.Requires(handler != null); - return - collection.GetWeakCollectionChangedObservable() - .Subscribe(e => handler.Invoke(collection, e)); + return collection.GetWeakCollectionChangedObservable() + .Subscribe(e => handler(collection, e)); } /// @@ -63,18 +58,13 @@ namespace Avalonia.Collections Contract.Requires(collection != null); Contract.Requires(handler != null); - return - collection.GetWeakCollectionChangedObservable() - .Subscribe(handler); + return collection.GetWeakCollectionChangedObservable().Subscribe(handler); } - private class WeakCollectionChangedObservable : ObservableBase, + private class WeakCollectionChangedObservable : LightweightObservableBase, IWeakSubscriber { private WeakReference _sourceReference; - private readonly Subject _changed = new Subject(); - - private int _count; public WeakCollectionChangedObservable(WeakReference source) { @@ -83,43 +73,28 @@ namespace Avalonia.Collections public void OnEvent(object sender, NotifyCollectionChangedEventArgs e) { - _changed.OnNext(e); + PublishNext(e); } - protected override IDisposable SubscribeCore(IObserver observer) + protected override void Initialize() { if (_sourceReference.TryGetTarget(out INotifyCollectionChanged instance)) { - if (_count++ == 0) - { - WeakSubscriptionManager.Subscribe( - instance, - nameof(instance.CollectionChanged), - this); - } - - return Observable.Using(() => Disposable.Create(DecrementCount), _ => _changed) - .Subscribe(observer); - } - else - { - _changed.OnCompleted(); - observer.OnCompleted(); - return Disposable.Empty; + WeakSubscriptionManager.Subscribe( + instance, + nameof(instance.CollectionChanged), + this); } } - private void DecrementCount() + protected override void Deinitialize() { - if (--_count == 0) + if (_sourceReference.TryGetTarget(out INotifyCollectionChanged instance)) { - if (_sourceReference.TryGetTarget(out INotifyCollectionChanged instance)) - { - WeakSubscriptionManager.Unsubscribe( - instance, - nameof(instance.CollectionChanged), - this); - } + WeakSubscriptionManager.Unsubscribe( + instance, + nameof(instance.CollectionChanged), + this); } } } From a3dea23560c04c1d603f91af1d6fe65e0e949254 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 02:28:12 +0200 Subject: [PATCH 23/42] Use LightweightObservableBase for BindingExpression. --- .../Data/Core/BindingExpression.cs | 43 ++++++++++++++++--- .../Data/Core/BindingExpressionTests.cs | 15 +++++++ 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/BindingExpression.cs b/src/Avalonia.Base/Data/Core/BindingExpression.cs index 4b41d1568c..911c621fb7 100644 --- a/src/Avalonia.Base/Data/Core/BindingExpression.cs +++ b/src/Avalonia.Base/Data/Core/BindingExpression.cs @@ -7,6 +7,7 @@ using System.Reactive.Linq; using System.Reactive.Subjects; using Avalonia.Data.Converters; using Avalonia.Logging; +using Avalonia.Reactive; using Avalonia.Utilities; namespace Avalonia.Data.Core @@ -15,13 +16,14 @@ namespace Avalonia.Data.Core /// Binds to an expression on an object using a type value converter to convert the values /// that are send and received. /// - public class BindingExpression : ISubject, IDescription + public class BindingExpression : LightweightObservableBase, ISubject, IDescription { private readonly ExpressionObserver _inner; private readonly Type _targetType; private readonly object _fallbackValue; private readonly BindingPriority _priority; - private readonly Subject _errors = new Subject(); + InnerListener _innerListener; + WeakReference _value; /// /// Initializes a new instance of the class. @@ -139,7 +141,7 @@ namespace Avalonia.Data.Core "IValueConverter should not return non-errored BindingNotification."); } - _errors.OnNext(notification); + PublishNext(notification); if (_fallbackValue != AvaloniaProperty.UnsetValue) { @@ -170,12 +172,18 @@ namespace Avalonia.Data.Core } } - /// - public IDisposable Subscribe(IObserver observer) + protected override void Initialize() => _innerListener = new InnerListener(this); + protected override void Deinitialize() => _innerListener.Dispose(); + + protected override void Subscribed(IObserver observer, bool first) { - return _inner.Select(ConvertValue).Merge(_errors).Subscribe(observer); + if (!first && _value != null && _value.TryGetTarget(out var val) == true) + { + observer.OnNext(val); + } } + /// private object ConvertValue(object value) { var notification = value as BindingNotification; @@ -301,5 +309,28 @@ namespace Avalonia.Data.Core return a; } + + public class InnerListener : IObserver, IDisposable + { + private readonly BindingExpression _owner; + private readonly IDisposable _dispose; + + public InnerListener(BindingExpression owner) + { + _owner = owner; + _dispose = owner._inner.Subscribe(this); + } + + public void Dispose() => _dispose.Dispose(); + public void OnCompleted() => _owner.PublishCompleted(); + public void OnError(Exception error) => _owner.PublishError(error); + + public void OnNext(object value) + { + var converted = _owner.ConvertValue(value); + _owner._value = new WeakReference(converted); + _owner.PublishNext(converted); + } + } } } diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.cs b/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.cs index 6b71d28e22..81acd6a087 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.cs @@ -337,6 +337,21 @@ namespace Avalonia.Base.UnitTests.Data.Core GC.KeepAlive(data); } + [Fact] + public void Second_Subscription_Should_Fire_Immediately() + { + var data = new Class1 { StringValue = "foo" }; + var target = new BindingExpression(new ExpressionObserver(data, "StringValue"), typeof(string)); + object result = null; + + target.Subscribe(); + target.Subscribe(x => result = x); + + Assert.Equal("foo", result); + + GC.KeepAlive(data); + } + private class Class1 : NotifyingBase { private string _stringValue; From 365aa42b4ab1396561f94f148fe024f494a9c4e3 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 13:10:25 +0200 Subject: [PATCH 24/42] Use custom rx for binding update signal. --- .../Data/Core/BindingExpression.cs | 2 +- .../SingleSubscriberObservableBase.cs | 79 +++++++++++++++++++ .../Styling/StyleActivator.cs | 4 +- src/Markup/Avalonia.Markup/Data/Binding.cs | 44 ++++++++--- 4 files changed, 115 insertions(+), 14 deletions(-) create mode 100644 src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs diff --git a/src/Avalonia.Base/Data/Core/BindingExpression.cs b/src/Avalonia.Base/Data/Core/BindingExpression.cs index 911c621fb7..c4ffa839e0 100644 --- a/src/Avalonia.Base/Data/Core/BindingExpression.cs +++ b/src/Avalonia.Base/Data/Core/BindingExpression.cs @@ -14,7 +14,7 @@ namespace Avalonia.Data.Core { /// /// Binds to an expression on an object using a type value converter to convert the values - /// that are send and received. + /// that are sent and received. /// public class BindingExpression : LightweightObservableBase, ISubject, IDescription { diff --git a/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs b/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs new file mode 100644 index 0000000000..af48df177e --- /dev/null +++ b/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs @@ -0,0 +1,79 @@ +using System; +using Avalonia.Threading; + +namespace Avalonia.Reactive +{ + public abstract class SingleSubscriberObservableBase : IObservable, IDisposable + { + private Exception _error; + private IObserver _observer; + private bool _completed; + + public IDisposable Subscribe(IObserver observer) + { + Contract.Requires(observer != null); + Dispatcher.UIThread.VerifyAccess(); + + if (_observer != null) + { + throw new InvalidOperationException("The observable can only be subscribed once."); + } + + if (_error != null) + { + observer.OnError(_error); + } + else if (_completed) + { + observer.OnCompleted(); + } + else + { + _observer = observer; + Subscribed(); + } + + return this; + } + + void IDisposable.Dispose() + { + Unsubscribed(); + _observer = null; + } + + protected abstract void Unsubscribed(); + + protected void PublishNext(T value) + { + if (_observer != null) + { + _observer?.OnNext(value); + } + } + + protected void PublishCompleted() + { + if (_observer != null) + { + _observer?.OnCompleted(); + _completed = true; + Unsubscribed(); + _observer = null; + } + } + + protected void PublishError(Exception error) + { + if (_observer != null) + { + _observer?.OnError(error); + _error = error; + Unsubscribed(); + _observer = null; + } + } + + protected abstract void Subscribed(); + } +} diff --git a/src/Avalonia.Styling/Styling/StyleActivator.cs b/src/Avalonia.Styling/Styling/StyleActivator.cs index 5bdea9f42e..63945037d8 100644 --- a/src/Avalonia.Styling/Styling/StyleActivator.cs +++ b/src/Avalonia.Styling/Styling/StyleActivator.cs @@ -48,8 +48,8 @@ namespace Avalonia.Styling else { return inputs.CombineLatest() - .Select(values => values.Any(x => x)) - .DistinctUntilChanged(); + .Select(values => values.Any(x => x)) + .DistinctUntilChanged(); } } } diff --git a/src/Markup/Avalonia.Markup/Data/Binding.cs b/src/Markup/Avalonia.Markup/Data/Binding.cs index 96fc2986e8..ef5ebb9844 100644 --- a/src/Markup/Avalonia.Markup/Data/Binding.cs +++ b/src/Markup/Avalonia.Markup/Data/Binding.cs @@ -5,11 +5,10 @@ using System; using System.Linq; using System.Reactive; using System.Reactive.Linq; -using System.Reflection; -using Avalonia.Controls; using Avalonia.Data.Converters; using Avalonia.Data.Core; using Avalonia.LogicalTree; +using Avalonia.Reactive; using Avalonia.VisualTree; namespace Avalonia.Data @@ -190,13 +189,10 @@ namespace Avalonia.Data if (!targetIsDataContext) { - var update = target.GetObservable(StyledElement.DataContextProperty) - .Skip(1) - .Select(_ => Unit.Default); var result = new ExpressionObserver( () => target.GetValue(StyledElement.DataContextProperty), path, - update, + new UpdateSignal(target, StyledElement.DataContextProperty), enableDataValidation); return result; @@ -278,14 +274,10 @@ namespace Avalonia.Data { Contract.Requires(target != null); - var update = target.GetObservable(StyledElement.TemplatedParentProperty) - .Skip(1) - .Select(_ => Unit.Default); - var result = new ExpressionObserver( () => target.GetValue(StyledElement.TemplatedParentProperty), path, - update, + new UpdateSignal(target, StyledElement.TemplatedParentProperty), enableDataValidation); return result; @@ -306,5 +298,35 @@ namespace Avalonia.Data Observable.Return((object)null); }).Switch(); } + + private class UpdateSignal : SingleSubscriberObservableBase + { + private readonly IAvaloniaObject _target; + private readonly AvaloniaProperty _property; + + public UpdateSignal(IAvaloniaObject target, AvaloniaProperty property) + { + _target = target; + _property = property; + } + + protected override void Subscribed() + { + _target.PropertyChanged += PropertyChanged; + } + + protected override void Unsubscribed() + { + _target.PropertyChanged -= PropertyChanged; + } + + private void PropertyChanged(object sender, AvaloniaPropertyChangedEventArgs e) + { + if (e.Property == _property) + { + PublishNext(Unit.Default); + } + } + } } } From b488ca179a0b72aa074c24d7948fe78f380af69a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 13:32:05 +0200 Subject: [PATCH 25/42] Use custom class for direct binding subscriptions. --- src/Avalonia.Base/AvaloniaObject.cs | 53 ++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 48e72db126..baca1494be 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -10,6 +10,7 @@ using System.Reactive.Linq; using Avalonia.Data; using Avalonia.Diagnostics; using Avalonia.Logging; +using Avalonia.Reactive; using Avalonia.Threading; using Avalonia.Utilities; @@ -38,7 +39,7 @@ namespace Avalonia /// Maintains a list of direct property binding subscriptions so that the binding source /// doesn't get collected. /// - private List _directBindings; + private List _directBindings; /// /// Event handler for implementation. @@ -359,25 +360,12 @@ namespace Avalonia property, description); - IDisposable subscription = null; - if (_directBindings == null) { - _directBindings = new List(); + _directBindings = new List(); } - subscription = source - .Select(x => CastOrDefault(x, property.PropertyType)) - .Do(_ => { }, () => _directBindings.Remove(subscription)) - .Subscribe(x => SetDirectValue(property, x)); - - _directBindings.Add(subscription); - - return Disposable.Create(() => - { - subscription.Dispose(); - _directBindings.Remove(subscription); - }); + return new DirectBindingSubscription(this, property, source); } else { @@ -908,5 +896,38 @@ namespace Avalonia value, priority); } + + private class DirectBindingSubscription : IObserver, IDisposable + { + readonly AvaloniaObject _owner; + readonly AvaloniaProperty _property; + IDisposable _subscription; + + public DirectBindingSubscription( + AvaloniaObject owner, + AvaloniaProperty property, + IObservable source) + { + _owner = owner; + _property = property; + _owner._directBindings.Add(this); + _subscription = source.Subscribe(this); + } + + public void Dispose() + { + _subscription.Dispose(); + _owner._directBindings.Remove(this); + } + + public void OnCompleted() => Dispose(); + public void OnError(Exception error) => Dispose(); + + public void OnNext(object value) + { + var castValue = CastOrDefault(value, _property.PropertyType); + _owner.SetDirectValue(_property, castValue); + } + } } } From a8d5dc1da4e49d57d241bc21beac4df7412d4b70 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 14:13:58 +0200 Subject: [PATCH 26/42] Use custom rx for ResourceObservable. --- .../Controls/ResourceProviderExtensions.cs | 39 ++++++++++++++++--- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Styling/Controls/ResourceProviderExtensions.cs b/src/Avalonia.Styling/Controls/ResourceProviderExtensions.cs index 1f25fa132d..1586c61185 100644 --- a/src/Avalonia.Styling/Controls/ResourceProviderExtensions.cs +++ b/src/Avalonia.Styling/Controls/ResourceProviderExtensions.cs @@ -1,6 +1,7 @@ using System; using System.Reactive; using System.Reactive.Linq; +using Avalonia.Reactive; namespace Avalonia.Controls { @@ -55,11 +56,39 @@ namespace Avalonia.Controls public static IObservable GetResourceObservable(this IResourceNode target, string key) { - return Observable.FromEventPattern( - x => target.ResourcesChanged += x, - x => target.ResourcesChanged -= x) - .StartWith((EventPattern)null) - .Select(x => target.FindResource(key)); + return new ResourceObservable(target, key); + } + + private class ResourceObservable : LightweightObservableBase + { + private readonly IResourceNode _target; + private readonly string _key; + + public ResourceObservable(IResourceNode target, string key) + { + _target = target; + _key = key; + } + + protected override void Initialize() + { + _target.ResourcesChanged += ResourcesChanged; + } + + protected override void Deinitialize() + { + _target.ResourcesChanged -= ResourcesChanged; + } + + protected override void Subscribed(IObserver observer, bool first) + { + observer.OnNext(_target.FindResource(_key)); + } + + private void ResourcesChanged(object sender, ResourcesChangedEventArgs e) + { + PublishNext(_target.FindResource(_key)); + } } } } From 26b3efd8d402b4a182e20152f9d7f5f6d84cc1a9 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 15:27:53 +0200 Subject: [PATCH 27/42] Use custom rx for control locators. --- .../LogicalTree/ControlLocator.cs | 162 ++++++++++++------ .../VisualTree/VisualLocator.cs | 67 +++++--- 2 files changed, 147 insertions(+), 82 deletions(-) diff --git a/src/Avalonia.Styling/LogicalTree/ControlLocator.cs b/src/Avalonia.Styling/LogicalTree/ControlLocator.cs index 2858d11d9d..7186143bf9 100644 --- a/src/Avalonia.Styling/LogicalTree/ControlLocator.cs +++ b/src/Avalonia.Styling/LogicalTree/ControlLocator.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Reactive.Linq; using System.Reflection; using Avalonia.Controls; +using Avalonia.Reactive; namespace Avalonia.LogicalTree { @@ -23,75 +24,122 @@ namespace Avalonia.LogicalTree /// The name of the control to find. public static IObservable Track(ILogical relativeTo, string name) { - var attached = Observable.FromEventPattern( - x => relativeTo.AttachedToLogicalTree += x, - x => relativeTo.AttachedToLogicalTree -= x) - .Select(x => ((ILogical)x.Sender).FindNameScope()) - .StartWith(relativeTo.FindNameScope()); - - var detached = Observable.FromEventPattern( - x => relativeTo.DetachedFromLogicalTree += x, - x => relativeTo.DetachedFromLogicalTree -= x) - .Select(x => (INameScope)null); - - return attached.Merge(detached).Select(nameScope => + return new ControlTracker(relativeTo, name); + } + + public static IObservable Track(ILogical relativeTo, int ancestorLevel, Type ancestorType = null) + { + return new ControlTracker(relativeTo, ancestorLevel, ancestorType); + } + + private class ControlTracker : LightweightObservableBase + { + private readonly ILogical _relativeTo; + private readonly string _name; + private readonly int _ancestorLevel; + private readonly Type _ancestorType; + INameScope _nameScope; + ILogical _value; + + public ControlTracker(ILogical relativeTo, string name) + { + _relativeTo = relativeTo; + _name = name; + } + + public ControlTracker(ILogical relativeTo, int ancestorLevel, Type ancestorType) { - if (nameScope != null) + _relativeTo = relativeTo; + _ancestorLevel = ancestorLevel; + _ancestorType = ancestorType; + } + + protected override void Initialize() + { + Update(); + _relativeTo.AttachedToLogicalTree += Attached; + _relativeTo.DetachedFromLogicalTree += Detached; + } + + protected override void Deinitialize() + { + _relativeTo.AttachedToLogicalTree -= Attached; + _relativeTo.DetachedFromLogicalTree -= Detached; + + if (_nameScope != null) { - var registered = Observable.FromEventPattern( - x => nameScope.Registered += x, - x => nameScope.Registered -= x) - .Where(x => x.EventArgs.Name == name) - .Select(x => x.EventArgs.Element) - .OfType(); - var unregistered = Observable.FromEventPattern( - x => nameScope.Unregistered += x, - x => nameScope.Unregistered -= x) - .Where(x => x.EventArgs.Name == name) - .Select(_ => (ILogical)null); - return registered - .StartWith(nameScope.Find(name)) - .Merge(unregistered); + _nameScope.Registered -= Registered; + _nameScope.Unregistered -= Unregistered; } - else + + _value = null; + } + + protected override void Subscribed(IObserver observer, bool first) + { + observer.OnNext(_value); + } + + private void Attached(object sender, LogicalTreeAttachmentEventArgs e) + { + Update(); + PublishNext(_value); + } + + private void Detached(object sender, LogicalTreeAttachmentEventArgs e) + { + if (_nameScope != null) { - return Observable.Return(null); + _nameScope.Registered -= Registered; + _nameScope.Unregistered -= Unregistered; } - }).Switch(); - } - public static IObservable Track(ILogical relativeTo, int ancestorLevel, Type ancestorType = null) - { - return TrackAttachmentToTree(relativeTo).Select(isAttachedToTree => + _value = null; + PublishNext(null); + } + + private void Registered(object sender, NameScopeEventArgs e) { - if (isAttachedToTree) + if (e.Name == _name && e.Element is ILogical logical) { - return relativeTo.GetLogicalAncestors() - .Where(x => ancestorType?.GetTypeInfo().IsAssignableFrom(x.GetType().GetTypeInfo()) ?? true) - .ElementAtOrDefault(ancestorLevel); + _value = logical; + PublishNext(logical); } - else + } + + private void Unregistered(object sender, NameScopeEventArgs e) + { + if (e.Name == _name) { - return null; + _value = null; + PublishNext(null); } - }); - } + } - private static IObservable TrackAttachmentToTree(ILogical relativeTo) - { - var attached = Observable.FromEventPattern( - x => relativeTo.AttachedToLogicalTree += x, - x => relativeTo.AttachedToLogicalTree -= x) - .Select(x => true) - .StartWith(relativeTo.IsAttachedToLogicalTree); - - var detached = Observable.FromEventPattern( - x => relativeTo.DetachedFromLogicalTree += x, - x => relativeTo.DetachedFromLogicalTree -= x) - .Select(x => false); - - var attachmentStatus = attached.Merge(detached); - return attachmentStatus; + private void Update() + { + if (_name != null) + { + _nameScope = _relativeTo.FindNameScope(); + + if (_nameScope != null) + { + _nameScope.Registered += Registered; + _nameScope.Unregistered += Unregistered; + _value = _nameScope.Find(_name); + } + else + { + _value = null; + } + } + else + { + _value = _relativeTo.GetLogicalAncestors() + .Where(x => _ancestorType?.GetTypeInfo().IsAssignableFrom(x.GetType().GetTypeInfo()) ?? true) + .ElementAtOrDefault(_ancestorLevel); + } + } } } } diff --git a/src/Avalonia.Visuals/VisualTree/VisualLocator.cs b/src/Avalonia.Visuals/VisualTree/VisualLocator.cs index 6fcb6c0db1..eaca9ed9d7 100644 --- a/src/Avalonia.Visuals/VisualTree/VisualLocator.cs +++ b/src/Avalonia.Visuals/VisualTree/VisualLocator.cs @@ -1,9 +1,8 @@ using System; -using System.Collections.Generic; using System.Linq; using System.Reactive.Linq; using System.Reflection; -using System.Text; +using Avalonia.Reactive; namespace Avalonia.VisualTree { @@ -11,36 +10,54 @@ namespace Avalonia.VisualTree { public static IObservable Track(IVisual relativeTo, int ancestorLevel, Type ancestorType = null) { - return TrackAttachmentToTree(relativeTo).Select(isAttachedToTree => + return new VisualTracker(relativeTo, ancestorLevel, ancestorType); + } + + private class VisualTracker : LightweightObservableBase + { + private readonly IVisual _relativeTo; + private readonly int _ancestorLevel; + private readonly Type _ancestorType; + + public VisualTracker(IVisual relativeTo, int ancestorLevel, Type ancestorType) + { + _relativeTo = relativeTo; + _ancestorLevel = ancestorLevel; + _ancestorType = ancestorType; + } + + protected override void Initialize() + { + _relativeTo.AttachedToVisualTree += AttachedDetached; + _relativeTo.DetachedFromVisualTree += AttachedDetached; + } + + protected override void Deinitialize() { - if (isAttachedToTree) + _relativeTo.AttachedToVisualTree -= AttachedDetached; + _relativeTo.DetachedFromVisualTree -= AttachedDetached; + } + + protected override void Subscribed(IObserver observer, bool first) + { + observer.OnNext(GetResult()); + } + + private void AttachedDetached(object sender, VisualTreeAttachmentEventArgs e) => PublishNext(GetResult()); + + private IVisual GetResult() + { + if (_relativeTo.IsAttachedToVisualTree) { - return relativeTo.GetVisualAncestors() - .Where(x => ancestorType?.GetTypeInfo().IsAssignableFrom(x.GetType().GetTypeInfo()) ?? true) - .ElementAtOrDefault(ancestorLevel); + return _relativeTo.GetVisualAncestors() + .Where(x => _ancestorType?.GetTypeInfo().IsAssignableFrom(x.GetType().GetTypeInfo()) ?? true) + .ElementAtOrDefault(_ancestorLevel); } else { return null; } - }); - } - - private static IObservable TrackAttachmentToTree(IVisual relativeTo) - { - var attached = Observable.FromEventPattern( - x => relativeTo.AttachedToVisualTree += x, - x => relativeTo.AttachedToVisualTree -= x) - .Select(x => true) - .StartWith(relativeTo.IsAttachedToVisualTree); - - var detached = Observable.FromEventPattern( - x => relativeTo.DetachedFromVisualTree += x, - x => relativeTo.DetachedFromVisualTree -= x) - .Select(x => false); - - var attachmentStatus = attached.Merge(detached); - return attachmentStatus; + } } } } From 6720bb9633d120663952fa07c3db1019ead539da Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 16:31:16 +0200 Subject: [PATCH 28/42] Updated ignore list in ncrunch. --- .ncrunch/Avalonia.Designer.HostApp.NetFX.v3.ncrunchproject | 5 +++++ .ncrunch/BindingDemo.net461.v3.ncrunchproject | 5 +++++ .ncrunch/BindingDemo.netcoreapp2.0.v3.ncrunchproject | 5 +++++ .ncrunch/Previewer.v3.ncrunchproject | 5 +++++ .ncrunch/RemoteDemo.v3.ncrunchproject | 5 +++++ .ncrunch/RenderDemo.net461.v3.ncrunchproject | 5 +++++ .ncrunch/RenderDemo.netcoreapp2.0.v3.ncrunchproject | 5 +++++ .ncrunch/VirtualizationDemo.net461.v3.ncrunchproject | 5 +++++ .ncrunch/VirtualizationDemo.netcoreapp2.0.v3.ncrunchproject | 5 +++++ 9 files changed, 45 insertions(+) create mode 100644 .ncrunch/Avalonia.Designer.HostApp.NetFX.v3.ncrunchproject create mode 100644 .ncrunch/BindingDemo.net461.v3.ncrunchproject create mode 100644 .ncrunch/BindingDemo.netcoreapp2.0.v3.ncrunchproject create mode 100644 .ncrunch/Previewer.v3.ncrunchproject create mode 100644 .ncrunch/RemoteDemo.v3.ncrunchproject create mode 100644 .ncrunch/RenderDemo.net461.v3.ncrunchproject create mode 100644 .ncrunch/RenderDemo.netcoreapp2.0.v3.ncrunchproject create mode 100644 .ncrunch/VirtualizationDemo.net461.v3.ncrunchproject create mode 100644 .ncrunch/VirtualizationDemo.netcoreapp2.0.v3.ncrunchproject diff --git a/.ncrunch/Avalonia.Designer.HostApp.NetFX.v3.ncrunchproject b/.ncrunch/Avalonia.Designer.HostApp.NetFX.v3.ncrunchproject new file mode 100644 index 0000000000..319cd523ce --- /dev/null +++ b/.ncrunch/Avalonia.Designer.HostApp.NetFX.v3.ncrunchproject @@ -0,0 +1,5 @@ + + + True + + \ No newline at end of file diff --git a/.ncrunch/BindingDemo.net461.v3.ncrunchproject b/.ncrunch/BindingDemo.net461.v3.ncrunchproject new file mode 100644 index 0000000000..319cd523ce --- /dev/null +++ b/.ncrunch/BindingDemo.net461.v3.ncrunchproject @@ -0,0 +1,5 @@ + + + True + + \ No newline at end of file diff --git a/.ncrunch/BindingDemo.netcoreapp2.0.v3.ncrunchproject b/.ncrunch/BindingDemo.netcoreapp2.0.v3.ncrunchproject new file mode 100644 index 0000000000..319cd523ce --- /dev/null +++ b/.ncrunch/BindingDemo.netcoreapp2.0.v3.ncrunchproject @@ -0,0 +1,5 @@ + + + True + + \ No newline at end of file diff --git a/.ncrunch/Previewer.v3.ncrunchproject b/.ncrunch/Previewer.v3.ncrunchproject new file mode 100644 index 0000000000..319cd523ce --- /dev/null +++ b/.ncrunch/Previewer.v3.ncrunchproject @@ -0,0 +1,5 @@ + + + True + + \ No newline at end of file diff --git a/.ncrunch/RemoteDemo.v3.ncrunchproject b/.ncrunch/RemoteDemo.v3.ncrunchproject new file mode 100644 index 0000000000..319cd523ce --- /dev/null +++ b/.ncrunch/RemoteDemo.v3.ncrunchproject @@ -0,0 +1,5 @@ + + + True + + \ No newline at end of file diff --git a/.ncrunch/RenderDemo.net461.v3.ncrunchproject b/.ncrunch/RenderDemo.net461.v3.ncrunchproject new file mode 100644 index 0000000000..319cd523ce --- /dev/null +++ b/.ncrunch/RenderDemo.net461.v3.ncrunchproject @@ -0,0 +1,5 @@ + + + True + + \ No newline at end of file diff --git a/.ncrunch/RenderDemo.netcoreapp2.0.v3.ncrunchproject b/.ncrunch/RenderDemo.netcoreapp2.0.v3.ncrunchproject new file mode 100644 index 0000000000..319cd523ce --- /dev/null +++ b/.ncrunch/RenderDemo.netcoreapp2.0.v3.ncrunchproject @@ -0,0 +1,5 @@ + + + True + + \ No newline at end of file diff --git a/.ncrunch/VirtualizationDemo.net461.v3.ncrunchproject b/.ncrunch/VirtualizationDemo.net461.v3.ncrunchproject new file mode 100644 index 0000000000..319cd523ce --- /dev/null +++ b/.ncrunch/VirtualizationDemo.net461.v3.ncrunchproject @@ -0,0 +1,5 @@ + + + True + + \ No newline at end of file diff --git a/.ncrunch/VirtualizationDemo.netcoreapp2.0.v3.ncrunchproject b/.ncrunch/VirtualizationDemo.netcoreapp2.0.v3.ncrunchproject new file mode 100644 index 0000000000..319cd523ce --- /dev/null +++ b/.ncrunch/VirtualizationDemo.netcoreapp2.0.v3.ncrunchproject @@ -0,0 +1,5 @@ + + + True + + \ No newline at end of file From ef5be70c4b6451a1f142b33dfd6b154b7c3d0ddd Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 21:13:48 +0200 Subject: [PATCH 29/42] Don't double-check nulls. --- .../Reactive/SingleSubscriberObservableBase.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs b/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs index af48df177e..cd8ce2cd80 100644 --- a/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs +++ b/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs @@ -46,17 +46,14 @@ namespace Avalonia.Reactive protected void PublishNext(T value) { - if (_observer != null) - { - _observer?.OnNext(value); - } + _observer?.OnNext(value); } protected void PublishCompleted() { if (_observer != null) { - _observer?.OnCompleted(); + _observer.OnCompleted(); _completed = true; Unsubscribed(); _observer = null; @@ -67,7 +64,7 @@ namespace Avalonia.Reactive { if (_observer != null) { - _observer?.OnError(error); + _observer.OnError(error); _error = error; Unsubscribed(); _observer = null; From 4f0764ea6bee6d4f24f445491b3169ba4830fcd9 Mon Sep 17 00:00:00 2001 From: Robert Hencke Date: Sat, 23 Jun 2018 19:29:49 -0400 Subject: [PATCH 30/42] Make Delete on macOS delete correctly. On a Mac keyboard, the Delete key functions as the Backspace key does on a PC keyboard. Map VK_Delete to Key.Back (VK_ForwardDelete correctly maps to Key.Delete) --- src/OSX/Avalonia.MonoMac/KeyTransform.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OSX/Avalonia.MonoMac/KeyTransform.cs b/src/OSX/Avalonia.MonoMac/KeyTransform.cs index 6d4b58031e..4abd5a24f1 100644 --- a/src/OSX/Avalonia.MonoMac/KeyTransform.cs +++ b/src/OSX/Avalonia.MonoMac/KeyTransform.cs @@ -200,7 +200,7 @@ namespace Avalonia.MonoMac [kVK_Return] = Key.Return, [kVK_Tab] = Key.Tab, [kVK_Space] = Key.Space, - [kVK_Delete] = Key.Delete, + [kVK_Delete] = Key.Back, [kVK_Escape] = Key.Escape, [kVK_Command] = Key.LWin, [kVK_Shift] = Key.LeftShift, From 0c078c9dec3086b262dcd1bde391106625e106eb Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 18:23:08 +0200 Subject: [PATCH 31/42] Don't use rx for ExpressionNodes. `ExpressionNode`s were always single-subscriber and making them use `IObservable<>` meant that we had to have extra allocations in order to return `IDisposable`s. Instead of using `IObservable` use a simpler `Subscribe`/`Unsubscribe` pattern. This saves a bunch more memory. --- .../Data/Core/EmptyExpressionNode.cs | 5 - src/Avalonia.Base/Data/Core/ExpressionNode.cs | 120 +++++++++--------- .../Data/Core/ExpressionObserver.cs | 35 ++--- src/Avalonia.Base/Data/Core/IndexerNode.cs | 11 +- .../Plugins/AvaloniaPropertyAccessorPlugin.cs | 10 +- .../Data/Core/Plugins/DataValidatiorBase.cs | 10 +- .../Core/Plugins/ExceptionValidationPlugin.cs | 5 +- .../Data/Core/Plugins/IPropertyAccessor.cs | 14 +- .../Core/Plugins/IndeiValidationPlugin.cs | 19 ++- .../Plugins/InpcPropertyAccessorPlugin.cs | 16 +-- .../Data/Core/Plugins/MethodAccessorPlugin.cs | 8 +- .../Data/Core/Plugins/PropertyAccessorBase.cs | 68 +++++----- .../Data/Core/Plugins/PropertyError.cs | 11 +- .../Data/Core/PropertyAccessorNode.cs | 23 ++-- src/Avalonia.Base/Data/Core/StreamNode.cs | 17 ++- .../Plugins/IndeiValidationPluginTests.cs | 5 +- 16 files changed, 193 insertions(+), 184 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/EmptyExpressionNode.cs b/src/Avalonia.Base/Data/Core/EmptyExpressionNode.cs index 93e0d5947a..c4166b44e5 100644 --- a/src/Avalonia.Base/Data/Core/EmptyExpressionNode.cs +++ b/src/Avalonia.Base/Data/Core/EmptyExpressionNode.cs @@ -9,10 +9,5 @@ namespace Avalonia.Data.Core internal class EmptyExpressionNode : ExpressionNode { public override string Description => "."; - - protected override IObservable StartListeningCore(WeakReference reference) - { - return Observable.Return(reference.Target); - } } } diff --git a/src/Avalonia.Base/Data/Core/ExpressionNode.cs b/src/Avalonia.Base/Data/Core/ExpressionNode.cs index ac7e97a4b1..600cd68d60 100644 --- a/src/Avalonia.Base/Data/Core/ExpressionNode.cs +++ b/src/Avalonia.Base/Data/Core/ExpressionNode.cs @@ -2,22 +2,18 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Reactive.Disposables; -using System.Reactive.Linq; -using System.Reactive.Subjects; -using Avalonia.Data; namespace Avalonia.Data.Core { - internal abstract class ExpressionNode : ISubject + internal abstract class ExpressionNode { private static readonly object CacheInvalid = new object(); protected static readonly WeakReference UnsetReference = new WeakReference(AvaloniaProperty.UnsetValue); private WeakReference _target = UnsetReference; - private IDisposable _valueSubscription; - private IObserver _observer; + private Action _subscriber; + private bool _listening; protected WeakReference LastValue { get; private set; } @@ -33,92 +29,66 @@ namespace Avalonia.Data.Core var oldTarget = _target?.Target; var newTarget = value.Target; - var running = _valueSubscription != null; if (!ReferenceEquals(oldTarget, newTarget)) { - _valueSubscription?.Dispose(); - _valueSubscription = null; + if (_listening) + { + StopListening(); + } + _target = value; - if (running) + if (_subscriber != null) { - _valueSubscription = StartListening(); + StartListening(); } } } } - public IDisposable Subscribe(IObserver observer) + public void Subscribe(Action subscriber) { - if (_observer != null) + if (_subscriber != null) { throw new AvaloniaInternalException("ExpressionNode can only be subscribed once."); } - _observer = observer; - var nextSubscription = Next?.Subscribe(this); - _valueSubscription = StartListening(); - - return Disposable.Create(() => - { - _valueSubscription?.Dispose(); - _valueSubscription = null; - LastValue = null; - nextSubscription?.Dispose(); - _observer = null; - }); + _subscriber = subscriber; + Next?.Subscribe(NextValueChanged); + StartListening(); } - void IObserver.OnCompleted() + public void Unsubscribe() { - throw new AvaloniaInternalException("ExpressionNode.OnCompleted should not be called."); - } + Next?.Unsubscribe(); - void IObserver.OnError(Exception error) - { - throw new AvaloniaInternalException("ExpressionNode.OnError should not be called."); + if (_listening) + { + StopListening(); + } + + LastValue = null; + _subscriber = null; } - void IObserver.OnNext(object value) + protected virtual void StartListeningCore(WeakReference reference) { - NextValueChanged(value); + ValueChanged(reference.Target); } - protected virtual IObservable StartListeningCore(WeakReference reference) + protected virtual void StopListeningCore() { - return Observable.Return(reference.Target); } protected virtual void NextValueChanged(object value) { var bindingBroken = BindingNotification.ExtractError(value) as MarkupBindingChainException; bindingBroken?.AddNode(Description); - _observer.OnNext(value); - } - - private IDisposable StartListening() - { - var target = _target.Target; - IObservable source; - - if (target == null) - { - source = Observable.Return(TargetNullNotification()); - } - else if (target == AvaloniaProperty.UnsetValue) - { - source = Observable.Empty(); - } - else - { - source = StartListeningCore(_target); - } - - return source.Subscribe(ValueChanged); + _subscriber(value); } - private void ValueChanged(object value) + protected void ValueChanged(object value) { var notification = value as BindingNotification; @@ -131,24 +101,50 @@ namespace Avalonia.Data.Core } else { - _observer.OnNext(value); + _subscriber(value); } } else { LastValue = new WeakReference(notification.Value); + if (Next != null) { Next.Target = new WeakReference(notification.Value); } - + if (Next == null || notification.Error != null) { - _observer.OnNext(value); + _subscriber(value); } } } + private void StartListening() + { + var target = _target.Target; + + if (target == null) + { + ValueChanged(TargetNullNotification()); + _listening = false; + } + else if (target != AvaloniaProperty.UnsetValue) + { + StartListeningCore(_target); + _listening = true; + } + else + { + _listening = false; + } + } + + private void StopListening() + { + StopListeningCore(); + } + private BindingNotification TargetNullNotification() { return new BindingNotification( diff --git a/src/Avalonia.Base/Data/Core/ExpressionObserver.cs b/src/Avalonia.Base/Data/Core/ExpressionObserver.cs index 3a25407133..3a061206bf 100644 --- a/src/Avalonia.Base/Data/Core/ExpressionObserver.cs +++ b/src/Avalonia.Base/Data/Core/ExpressionObserver.cs @@ -14,9 +14,7 @@ namespace Avalonia.Data.Core /// /// Observes and sets the value of an expression on an object. /// - public class ExpressionObserver : LightweightObservableBase, - IDescription, - IObserver + public class ExpressionObserver : LightweightObservableBase, IDescription { /// /// An ordered collection of property accessor plugins that can be used to customize @@ -55,7 +53,6 @@ namespace Avalonia.Data.Core private static readonly object UninitializedValue = new object(); private readonly ExpressionNode _node; - private IDisposable _nodeSubscription; private object _root; private IDisposable _rootSubscription; private WeakReference _value; @@ -202,34 +199,18 @@ namespace Avalonia.Data.Core } } - void IObserver.OnNext(object value) - { - var broken = BindingNotification.ExtractError(value) as MarkupBindingChainException; - broken?.Commit(Description); - _value = new WeakReference(value); - PublishNext(value); - } - - void IObserver.OnCompleted() - { - } - - void IObserver.OnError(Exception error) - { - } - protected override void Initialize() { _value = null; - _nodeSubscription = _node.Subscribe(this); + _node.Subscribe(ValueChanged); StartRoot(); } protected override void Deinitialize() { _rootSubscription?.Dispose(); - _nodeSubscription?.Dispose(); - _rootSubscription = _nodeSubscription = null; + _rootSubscription = null; + _node.Unsubscribe(); } protected override void Subscribed(IObserver observer, bool first) @@ -266,5 +247,13 @@ namespace Avalonia.Data.Core _node.Target = (WeakReference)_root; } } + + private void ValueChanged(object value) + { + var broken = BindingNotification.ExtractError(value) as MarkupBindingChainException; + broken?.Commit(Description); + _value = new WeakReference(value); + PublishNext(value); + } } } diff --git a/src/Avalonia.Base/Data/Core/IndexerNode.cs b/src/Avalonia.Base/Data/Core/IndexerNode.cs index 633d3558ee..afdfe90c4c 100644 --- a/src/Avalonia.Base/Data/Core/IndexerNode.cs +++ b/src/Avalonia.Base/Data/Core/IndexerNode.cs @@ -17,6 +17,8 @@ namespace Avalonia.Data.Core { internal class IndexerNode : SettableNode { + private IDisposable _subscription; + public IndexerNode(IList arguments) { Arguments = arguments; @@ -24,7 +26,7 @@ namespace Avalonia.Data.Core public override string Description => "[" + string.Join(",", Arguments) + "]"; - protected override IObservable StartListeningCore(WeakReference reference) + protected override void StartListeningCore(WeakReference reference) { var target = reference.Target; var incc = target as INotifyCollectionChanged; @@ -49,7 +51,12 @@ namespace Avalonia.Data.Core .Select(_ => GetValue(target))); } - return Observable.Merge(inputs).StartWith(GetValue(target)); + _subscription = Observable.Merge(inputs).StartWith(GetValue(target)).Subscribe(ValueChanged); + } + + protected override void StopListeningCore() + { + _subscription.Dispose(); } protected override bool SetTargetValueCore(object value, BindingPriority priority) diff --git a/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs index 48edb218dc..a163c07f87 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs @@ -145,15 +145,15 @@ namespace Avalonia.Data.Core.Plugins return false; } - protected override void Dispose(bool disposing) + protected override void SubscribeCore() { - _subscription?.Dispose(); - _subscription = null; + _subscription = Instance?.GetObservable(_property).Subscribe(PublishValue); } - protected override void SubscribeCore(IObserver observer) + protected override void UnsubscribeCore() { - _subscription = Instance?.GetObservable(_property).Subscribe(observer); + _subscription?.Dispose(); + _subscription = null; } } } diff --git a/src/Avalonia.Base/Data/Core/Plugins/DataValidatiorBase.cs b/src/Avalonia.Base/Data/Core/Plugins/DataValidatiorBase.cs index bd429f04d6..03ab7712bd 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/DataValidatiorBase.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/DataValidatiorBase.cs @@ -55,13 +55,13 @@ namespace Avalonia.Data.Core.Plugins /// The value. void IObserver.OnNext(object value) => InnerValueChanged(value); - /// - protected override void Dispose(bool disposing) => _inner.Dispose(); - /// /// Begins listening to the inner . /// - protected override void SubscribeCore(IObserver observer) => _inner.Subscribe(this); + protected override void SubscribeCore() => _inner.Subscribe(InnerValueChanged); + + /// + protected override void UnsubscribeCore() => _inner.Dispose(); /// /// Called when the inner notifies with a new value. @@ -74,7 +74,7 @@ namespace Avalonia.Data.Core.Plugins protected virtual void InnerValueChanged(object value) { var notification = value as BindingNotification ?? new BindingNotification(value); - Observer.OnNext(notification); + PublishValue(notification); } } } \ No newline at end of file diff --git a/src/Avalonia.Base/Data/Core/Plugins/ExceptionValidationPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/ExceptionValidationPlugin.cs index 35f9f7e59a..4507b32e0c 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/ExceptionValidationPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/ExceptionValidationPlugin.cs @@ -1,7 +1,6 @@ // 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.Data; using System; using System.Reflection; @@ -36,11 +35,11 @@ namespace Avalonia.Data.Core.Plugins } catch (TargetInvocationException ex) { - Observer.OnNext(new BindingNotification(ex.InnerException, BindingErrorType.DataValidationError)); + PublishValue(new BindingNotification(ex.InnerException, BindingErrorType.DataValidationError)); } catch (Exception ex) { - Observer.OnNext(new BindingNotification(ex, BindingErrorType.DataValidationError)); + PublishValue(new BindingNotification(ex, BindingErrorType.DataValidationError)); } return false; diff --git a/src/Avalonia.Base/Data/Core/Plugins/IPropertyAccessor.cs b/src/Avalonia.Base/Data/Core/Plugins/IPropertyAccessor.cs index d7dda57a72..33ea5bba08 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/IPropertyAccessor.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/IPropertyAccessor.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using Avalonia.Data; namespace Avalonia.Data.Core.Plugins { @@ -10,7 +9,7 @@ namespace Avalonia.Data.Core.Plugins /// Defines an accessor to a property on an object returned by a /// /// - public interface IPropertyAccessor : IObservable, IDisposable + public interface IPropertyAccessor : IDisposable { /// /// Gets the type of the property. @@ -38,5 +37,16 @@ namespace Avalonia.Data.Core.Plugins /// True if the property was set; false if the property could not be set. /// bool SetValue(object value, BindingPriority priority); + + /// + /// Subscribes to the value of the member. + /// + /// A method that receives the values. + void Subscribe(Action listener); + + /// + /// Unsubscribes to the value of the member. + /// + void Unsubscribe(); } } diff --git a/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs index 436046f3fa..4d6fc01229 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.ComponentModel; using System.Linq; -using Avalonia.Data; using Avalonia.Utilities; namespace Avalonia.Data.Core.Plugins @@ -40,43 +39,43 @@ namespace Avalonia.Data.Core.Plugins { if (e.PropertyName == _name || string.IsNullOrEmpty(e.PropertyName)) { - Observer.OnNext(CreateBindingNotification(Value)); + PublishValue(CreateBindingNotification(Value)); } } - protected override void Dispose(bool disposing) + protected override void SubscribeCore() { - base.Dispose(disposing); - var target = _reference.Target as INotifyDataErrorInfo; if (target != null) { - WeakSubscriptionManager.Unsubscribe( + WeakSubscriptionManager.Subscribe( target, nameof(target.ErrorsChanged), this); } + + base.SubscribeCore(); } - protected override void SubscribeCore(IObserver observer) + protected override void UnsubscribeCore() { var target = _reference.Target as INotifyDataErrorInfo; if (target != null) { - WeakSubscriptionManager.Subscribe( + WeakSubscriptionManager.Unsubscribe( target, nameof(target.ErrorsChanged), this); } - base.SubscribeCore(observer); + base.UnsubscribeCore(); } protected override void InnerValueChanged(object value) { - base.InnerValueChanged(CreateBindingNotification(value)); + PublishValue(CreateBindingNotification(value)); } private BindingNotification CreateBindingNotification(object value) diff --git a/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs index ba4e60eb74..dab32b639a 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs @@ -103,7 +103,13 @@ namespace Avalonia.Data.Core.Plugins } } - protected override void Dispose(bool disposing) + protected override void SubscribeCore() + { + SendCurrentValue(); + SubscribeToChanges(); + } + + protected override void UnsubscribeCore() { var inpc = _reference.Target as INotifyPropertyChanged; @@ -116,18 +122,12 @@ namespace Avalonia.Data.Core.Plugins } } - protected override void SubscribeCore(IObserver observer) - { - SendCurrentValue(); - SubscribeToChanges(); - } - private void SendCurrentValue() { try { var value = Value; - Observer.OnNext(value); + PublishValue(value); } catch { } } diff --git a/src/Avalonia.Base/Data/Core/Plugins/MethodAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/MethodAccessorPlugin.cs index b2b3a107fa..cf0abc6f35 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/MethodAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/MethodAccessorPlugin.cs @@ -74,14 +74,18 @@ namespace Avalonia.Data.Core.Plugins public override bool SetValue(object value, BindingPriority priority) => false; - protected override void SubscribeCore(IObserver observer) + protected override void SubscribeCore() { try { - Observer.OnNext(Value); + PublishValue(Value); } catch { } } + + protected override void UnsubscribeCore() + { + } } } } diff --git a/src/Avalonia.Base/Data/Core/Plugins/PropertyAccessorBase.cs b/src/Avalonia.Base/Data/Core/Plugins/PropertyAccessorBase.cs index 9cc78369a7..e840b2c5c9 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/PropertyAccessorBase.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/PropertyAccessorBase.cs @@ -2,67 +2,75 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using Avalonia.Data; namespace Avalonia.Data.Core.Plugins { /// /// Defines a default base implementation for a . /// - /// - /// is an observable that will only be subscribed to one time. - /// In addition, the subscription can be disposed by calling on the - /// property accessor itself - this prevents needing to hold two references for a subscription. - /// public abstract class PropertyAccessorBase : IPropertyAccessor { + private Action _listener; + /// public abstract Type PropertyType { get; } /// public abstract object Value { get; } - /// - /// Stops the subscription. - /// - public void Dispose() => Dispose(true); + /// + public void Dispose() + { + if (_listener != null) + { + Unsubscribe(); + } + } /// public abstract bool SetValue(object value, BindingPriority priority); - /// - /// The currently subscribed observer. - /// - protected IObserver Observer { get; private set; } - /// - public IDisposable Subscribe(IObserver observer) + public void Subscribe(Action listener) { - Contract.Requires(observer != null); + Contract.Requires(listener != null); - if (Observer != null) + if (_listener != null) { throw new InvalidOperationException( - "A property accessor can be subscribed to only once."); + "A member accessor can be subscribed to only once."); } - Observer = observer; - SubscribeCore(observer); - return this; + _listener = listener; + SubscribeCore(); } + public void Unsubscribe() + { + if (_listener == null) + { + throw new InvalidOperationException( + "The member accessor was not subscribed."); + } + + UnsubscribeCore(); + _listener = null; + } + + /// + /// Publishes a value to the listener. + /// + /// The value. + protected void PublishValue(object value) => _listener?.Invoke(value); + /// - /// Stops listening to the property. + /// When overridden in a derived class, begins listening to the member. /// - /// - /// True if the method was called, false if the object is being - /// finalized. - /// - protected virtual void Dispose(bool disposing) => Observer = null; + protected abstract void SubscribeCore(); /// - /// When overridden in a derived class, begins listening to the property. + /// When overridden in a derived class, stops listening to the member. /// - protected abstract void SubscribeCore(IObserver observer); + protected abstract void UnsubscribeCore(); } } diff --git a/src/Avalonia.Base/Data/Core/Plugins/PropertyError.cs b/src/Avalonia.Base/Data/Core/Plugins/PropertyError.cs index 647adc36cb..eb2400807a 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/PropertyError.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/PropertyError.cs @@ -1,6 +1,4 @@ using System; -using System.Reactive.Disposables; -using Avalonia.Data; namespace Avalonia.Data.Core.Plugins { @@ -37,10 +35,13 @@ namespace Avalonia.Data.Core.Plugins return false; } - public IDisposable Subscribe(IObserver observer) + public void Subscribe(Action listener) + { + listener(_error); + } + + public void Unsubscribe() { - observer.OnNext(_error); - return Disposable.Empty; } } } diff --git a/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs b/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs index 9d657b3144..2565a34322 100644 --- a/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs +++ b/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs @@ -3,9 +3,7 @@ using System; using System.Linq; -using System.Reactive.Disposables; using System.Reactive.Linq; -using Avalonia.Data; using Avalonia.Data.Core.Plugins; namespace Avalonia.Data.Core @@ -39,7 +37,7 @@ namespace Avalonia.Data.Core return false; } - protected override IObservable StartListeningCore(WeakReference reference) + protected override void StartListeningCore(WeakReference reference) { var plugin = ExpressionObserver.PropertyAccessors.FirstOrDefault(x => x.Match(reference.Target, PropertyName)); var accessor = plugin?.Start(reference, PropertyName); @@ -55,17 +53,14 @@ namespace Avalonia.Data.Core } } - // Ensure that _accessor is set for the duration of the subscription. - return Observable.Using( - () => - { - _accessor = accessor; - return Disposable.Create(() => - { - _accessor = null; - }); - }, - _ => accessor); + accessor.Subscribe(ValueChanged); + _accessor = accessor; + } + + protected override void StopListeningCore() + { + _accessor.Dispose(); + _accessor = null; } } } diff --git a/src/Avalonia.Base/Data/Core/StreamNode.cs b/src/Avalonia.Base/Data/Core/StreamNode.cs index 187c79af49..415def4d30 100644 --- a/src/Avalonia.Base/Data/Core/StreamNode.cs +++ b/src/Avalonia.Base/Data/Core/StreamNode.cs @@ -2,30 +2,37 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Globalization; -using Avalonia.Data; using System.Reactive.Linq; namespace Avalonia.Data.Core { internal class StreamNode : ExpressionNode { + private IDisposable _subscription; + public override string Description => "^"; - protected override IObservable StartListeningCore(WeakReference reference) + protected override void StartListeningCore(WeakReference reference) { foreach (var plugin in ExpressionObserver.StreamHandlers) { if (plugin.Match(reference)) { - return plugin.Start(reference); + _subscription = plugin.Start(reference).Subscribe(ValueChanged); + return; } } // TODO: Improve error. - return Observable.Return(new BindingNotification( + ValueChanged(new BindingNotification( new MarkupBindingChainException("Stream operator applied to unsupported type", Description), BindingErrorType.Error)); } + + protected override void StopListeningCore() + { + _subscription?.Dispose(); + _subscription = null; + } } } diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs b/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs index 45c084014b..383030cb6c 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs @@ -4,7 +4,6 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Reactive.Linq; using Avalonia.Data; using Avalonia.Data.Core.Plugins; using Xunit; @@ -58,9 +57,9 @@ namespace Avalonia.Base.UnitTests.Data.Core.Plugins var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.Value), accessor); Assert.Equal(0, data.ErrorsChangedSubscriptionCount); - var sub = validator.Subscribe(_ => { }); + validator.Subscribe(_ => { }); Assert.Equal(1, data.ErrorsChangedSubscriptionCount); - sub.Dispose(); + validator.Unsubscribe(); Assert.Equal(0, data.ErrorsChangedSubscriptionCount); } From 6d0e46134919956ba20a115218b5ba7f43fa933c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 25 Jun 2018 10:46:50 +0200 Subject: [PATCH 32/42] Throw if no matching property accessor found. This shouldn't happen normally as `InpcPropertyAcessorPlugin` matches everything. --- src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs b/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs index 2565a34322..e9831eb047 100644 --- a/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs +++ b/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs @@ -53,6 +53,12 @@ namespace Avalonia.Data.Core } } + if (accessor == null) + { + throw new NotSupportedException( + $"Could not find a matching property accessor for {PropertyName}."); + } + accessor.Subscribe(ValueChanged); _accessor = accessor; } From af7e139ed4cb7e06850ccef07d6cc8e934ca061d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 26 Jun 2018 00:33:24 +0200 Subject: [PATCH 33/42] Added failing test for #1698. --- .../SelectorTests_Class.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/Avalonia.Styling.UnitTests/SelectorTests_Class.cs b/tests/Avalonia.Styling.UnitTests/SelectorTests_Class.cs index b41c21fbf4..75599925b7 100644 --- a/tests/Avalonia.Styling.UnitTests/SelectorTests_Class.cs +++ b/tests/Avalonia.Styling.UnitTests/SelectorTests_Class.cs @@ -1,6 +1,7 @@ // 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 System; using System.Linq; using System.Reactive.Linq; using System.Threading.Tasks; @@ -8,6 +9,7 @@ using Moq; using Avalonia.Controls; using Avalonia.Styling; using Xunit; +using System.Collections.Generic; namespace Avalonia.Styling.UnitTests { @@ -117,6 +119,28 @@ namespace Avalonia.Styling.UnitTests Assert.False(await activator.Take(1)); } + [Fact] + public void Only_Notifies_When_Result_Changes() + { + // Test for #1698 + var control = new Control1 + { + Classes = new Classes { "foo" }, + }; + + var target = default(Selector).Class("foo"); + var activator = target.Match(control).ObservableResult; + var result = new List(); + + using (activator.Subscribe(x => result.Add(x))) + { + control.Classes.Add("bar"); + control.Classes.Remove("foo"); + } + + Assert.Equal(new[] { true, false }, result); + } + public class Control1 : TestControlBase { } From 114b393813d38683ebc4ea013555ed5f7fa6956b Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 26 Jun 2018 00:33:55 +0200 Subject: [PATCH 34/42] Only publish distinct values from ClassObserver. Fixes #1698. --- .../Styling/TypeNameAndClassSelector.cs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs b/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs index 93d4e14f27..94c0b75c6e 100644 --- a/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs +++ b/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs @@ -202,6 +202,7 @@ namespace Avalonia.Styling { readonly IList _match; IAvaloniaReadOnlyList _classes; + bool _value; public ClassObserver(IAvaloniaReadOnlyList classes, IList match) { @@ -210,18 +211,29 @@ namespace Avalonia.Styling } protected override void Deinitialize() => _classes.CollectionChanged -= ClassesChanged; - protected override void Initialize() => _classes.CollectionChanged += ClassesChanged; + + protected override void Initialize() + { + _value = GetResult(); + _classes.CollectionChanged += ClassesChanged; + } protected override void Subscribed(IObserver observer, bool first) { - observer.OnNext(GetResult()); + observer.OnNext(_value); } private void ClassesChanged(object sender, NotifyCollectionChangedEventArgs e) { if (e.Action != NotifyCollectionChangedAction.Move) { - PublishNext(GetResult()); + var value = GetResult(); + + if (value != _value) + { + PublishNext(GetResult()); + _value = value; + } } } From 617dd6a5ab9718043440e4cccbece7ae8e566a01 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Tue, 26 Jun 2018 15:43:33 +0100 Subject: [PATCH 35/42] fix win32 clipboard settext async. --- src/Windows/Avalonia.Win32/ClipboardImpl.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Windows/Avalonia.Win32/ClipboardImpl.cs b/src/Windows/Avalonia.Win32/ClipboardImpl.cs index 3dae8e37d9..a908c9e1e2 100644 --- a/src/Windows/Avalonia.Win32/ClipboardImpl.cs +++ b/src/Windows/Avalonia.Win32/ClipboardImpl.cs @@ -57,6 +57,9 @@ namespace Avalonia.Win32 } await OpenClipboard(); + + UnmanagedMethods.EmptyClipboard(); + try { var hGlobal = Marshal.StringToHGlobalUni(text); From b3cfc22e27cb18e9c758ab9f3fc551c3577ec80e Mon Sep 17 00:00:00 2001 From: Benedikt Schroeder Date: Thu, 28 Jun 2018 19:55:25 +0200 Subject: [PATCH 36/42] Add scientific notation support for double values --- src/Avalonia.Visuals/Media/PathMarkupParser.cs | 15 +++++++++++++++ .../Media/PathMarkupParserTests.cs | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/src/Avalonia.Visuals/Media/PathMarkupParser.cs b/src/Avalonia.Visuals/Media/PathMarkupParser.cs index 384c96a7d6..bfb6ec6450 100644 --- a/src/Avalonia.Visuals/Media/PathMarkupParser.cs +++ b/src/Avalonia.Visuals/Media/PathMarkupParser.cs @@ -451,6 +451,21 @@ namespace Avalonia.Media } for (; i < remaining.Length && char.IsNumber(remaining[i]); i++) valid = true; + if (i < remaining.Length) + { + // scientific notation + if (remaining[i] == 'E') + { + valid = false; + i++; + if (remaining[i] == '-' || remaining[i] == '+') + { + i++; + for (; i < remaining.Length && char.IsNumber(remaining[i]); i++) valid = true; + } + } + } + if (!valid) { argument = ReadOnlySpan.Empty; diff --git a/tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs b/tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs index 5074d306fd..6c456a9321 100644 --- a/tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs @@ -142,6 +142,10 @@ namespace Avalonia.Visuals.UnitTests.Media } [Theory] + [InlineData("F1M9.0771,11C9.1161,10.701,9.1801,10.352,9.3031,10L9.0001,10 9.0001,6.166 3.0001,9.767 3.0001,10 " + + "9.99999999997669E-05,10 9.99999999997669E-05,0 3.0001,0 3.0001,0.234 9.0001,3.834 9.0001,0 " + + "12.0001,0 12.0001,8.062C12.1861,8.043 12.3821,8.031 12.5941,8.031 15.3481,8.031 15.7961,9.826 " + + "15.9201,11L16.0001,16 9.0001,16 9.0001,12.562 9.0001,11z")] // issue #1708 [InlineData(" M0 0")] [InlineData("F1 M24,14 A2,2,0,1,1,20,14 A2,2,0,1,1,24,14 z")] // issue #1107 [InlineData("M0 0L10 10z")] From 9ccf63d51bd4fe548d920219f256a0df2620766d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 28 Jun 2018 23:45:10 +0200 Subject: [PATCH 37/42] Add failing test for clearing templated child's parent. --- .../Primitives/TemplatedControlTests.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs index cd71717619..166586ace1 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs @@ -160,6 +160,24 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal(target, child.GetLogicalParent()); } + [Fact] + public void Changing_Template_Should_Clear_Old_Templated_Childs_Parent() + { + var target = new TemplatedControl + { + Template = new FuncControlTemplate(_ => new Decorator()) + }; + + target.ApplyTemplate(); + + var child = (Decorator)target.GetVisualChildren().Single(); + + target.Template = new FuncControlTemplate(_ => new Canvas()); + target.ApplyTemplate(); + + Assert.Null(child.Parent); + } + [Fact] public void Nested_Templated_Control_Should_Not_Have_Template_Applied() { From 95fe6f4cdfa9ca67be64037f956ad0a70c3c567c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 28 Jun 2018 23:46:03 +0200 Subject: [PATCH 38/42] Clear templated child's parent when template detached. --- src/Avalonia.Controls/Primitives/TemplatedControl.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Avalonia.Controls/Primitives/TemplatedControl.cs b/src/Avalonia.Controls/Primitives/TemplatedControl.cs index 8514104c91..296134ca48 100644 --- a/src/Avalonia.Controls/Primitives/TemplatedControl.cs +++ b/src/Avalonia.Controls/Primitives/TemplatedControl.cs @@ -247,6 +247,7 @@ namespace Avalonia.Controls.Primitives foreach (var child in this.GetTemplateChildren()) { child.SetValue(TemplatedParentProperty, null); + ((ISetLogicalParent)child).SetParent(null); } VisualChildren.Clear(); From 9c1c8749ddd3e11a717901726f7e61119a8c9e64 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 29 Jun 2018 00:50:15 +0200 Subject: [PATCH 39/42] Added failing test for #1709. --- .../ItemsControlTests.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs index 4da803353e..9ef1e9f0d2 100644 --- a/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs @@ -315,6 +315,26 @@ namespace Avalonia.Controls.UnitTests Assert.Same(before, after); } + [Fact] + public void Should_Clear_Containers_When_ItemsPresenter_Changes() + { + var target = new ItemsControl + { + Items = new[] { "foo", "bar" }, + Template = GetTemplate(), + }; + + target.ApplyTemplate(); + target.Presenter.ApplyTemplate(); + + Assert.Equal(2, target.ItemContainerGenerator.Containers.Count()); + + target.Template = GetTemplate(); + target.ApplyTemplate(); + + Assert.Empty(target.ItemContainerGenerator.Containers); + } + [Fact] public void Empty_Class_Should_Initially_Be_Applied() { From cf14976dcce4ce011a664a83d9ea57184706f183 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 29 Jun 2018 00:50:28 +0200 Subject: [PATCH 40/42] Fix #1709. --- src/Avalonia.Controls/ItemsControl.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Avalonia.Controls/ItemsControl.cs b/src/Avalonia.Controls/ItemsControl.cs index 5119096965..3cb997f615 100644 --- a/src/Avalonia.Controls/ItemsControl.cs +++ b/src/Avalonia.Controls/ItemsControl.cs @@ -155,6 +155,7 @@ namespace Avalonia.Controls void IItemsPresenterHost.RegisterItemsPresenter(IItemsPresenter presenter) { Presenter = presenter; + ItemContainerGenerator.Clear(); } /// From 2f7a578c38eddc108028ea2aff34e7e4e9d68932 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 29 Jun 2018 00:51:03 +0200 Subject: [PATCH 41/42] Added null check for panel. Problem reared its head when #1709 was fixed. --- .../Primitives/SelectingItemsControl.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 2e668fda95..a7b8981583 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -408,12 +408,15 @@ namespace Avalonia.Controls.Primitives var panel = (InputElement)Presenter.Panel; - foreach (var container in e.Containers) + if (panel != null) { - if (KeyboardNavigation.GetTabOnceActiveElement(panel) == container.ContainerControl) + foreach (var container in e.Containers) { - KeyboardNavigation.SetTabOnceActiveElement(panel, null); - break; + if (KeyboardNavigation.GetTabOnceActiveElement(panel) == container.ContainerControl) + { + KeyboardNavigation.SetTabOnceActiveElement(panel, null); + break; + } } } } From ac8342ebf89d35f1b0030edc566325b40668e057 Mon Sep 17 00:00:00 2001 From: Benedikt Schroeder Date: Fri, 29 Jun 2018 06:34:54 +0200 Subject: [PATCH 42/42] Allow lower case e for scientific notation --- .../Media/PathMarkupParser.cs | 2 +- .../Media/PathMarkupParserTests.cs | 22 ++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Visuals/Media/PathMarkupParser.cs b/src/Avalonia.Visuals/Media/PathMarkupParser.cs index bfb6ec6450..b6232c6803 100644 --- a/src/Avalonia.Visuals/Media/PathMarkupParser.cs +++ b/src/Avalonia.Visuals/Media/PathMarkupParser.cs @@ -454,7 +454,7 @@ namespace Avalonia.Media if (i < remaining.Length) { // scientific notation - if (remaining[i] == 'E') + if (remaining[i] == 'E' || remaining[i] == 'e') { valid = false; i++; diff --git a/tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs b/tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs index 6c456a9321..4d3de034d8 100644 --- a/tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Media/PathMarkupParserTests.cs @@ -7,6 +7,7 @@ using Xunit; namespace Avalonia.Visuals.UnitTests.Media { + using System.Globalization; using System.IO; public class PathMarkupParserTests @@ -139,7 +140,26 @@ namespace Avalonia.Visuals.UnitTests.Media Assert.Equal(new Point(30, 30), lineSegment.Point); } - } + } + + [Fact] + public void Parses_Scientific_Notation_Double() + { + var pathGeometry = new PathGeometry(); + using (var context = new PathGeometryContext(pathGeometry)) + using (var parser = new PathMarkupParser(context)) + { + parser.Parse("M -1.01725E-005 -1.01725e-005"); + + var figure = pathGeometry.Figures[0]; + + Assert.Equal( + new Point( + double.Parse("-1.01725E-005", NumberStyles.Float, CultureInfo.InvariantCulture), + double.Parse("-1.01725E-005", NumberStyles.Float, CultureInfo.InvariantCulture)), + figure.StartPoint); + } + } [Theory] [InlineData("F1M9.0771,11C9.1161,10.701,9.1801,10.352,9.3031,10L9.0001,10 9.0001,6.166 3.0001,9.767 3.0001,10 "