From 7f8dc7766ea500a1046763e73e0640d62973660c Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 20 Jun 2018 12:46:29 -0500 Subject: [PATCH 01/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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 ef5be70c4b6451a1f142b33dfd6b154b7c3d0ddd Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 21:13:48 +0200 Subject: [PATCH 28/31] 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 af7e139ed4cb7e06850ccef07d6cc8e934ca061d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 26 Jun 2018 00:33:24 +0200 Subject: [PATCH 29/31] 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 30/31] 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 31/31] 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);