From 01a404836ee80d7dc43113f97da25139391c50d9 Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Tue, 1 Oct 2019 22:57:44 +0200 Subject: [PATCH 1/9] Reduce allocations caused by logging. --- src/Avalonia.Base/AvaloniaObject.cs | 68 ++++++++---- src/Avalonia.Base/Logging/ILogSink.cs | 71 ++++++++++++ src/Avalonia.Base/Logging/Logger.cs | 90 +++++++++++++++ .../Primitives/TemplatedControl.cs | 2 +- src/Avalonia.Layout/LayoutManager.cs | 32 ++++-- src/Avalonia.Layout/Layoutable.cs | 8 +- src/Avalonia.Logging.Serilog/SerilogLogger.cs | 104 +++++++++++++++++- src/Avalonia.Visuals/Visual.cs | 4 +- tests/Avalonia.UnitTests/TestLogSink.cs | 32 +++++- 9 files changed, 365 insertions(+), 46 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 8cc512d132..22a85663c7 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -326,8 +326,6 @@ namespace Avalonia VerifyAccess(); - var description = GetDescription(source); - if (property.IsDirect) { if (property.IsReadOnly) @@ -335,12 +333,18 @@ namespace Avalonia throw new ArgumentException($"The property {property.Name} is readonly."); } - Logger.Verbose( - LogArea.Property, - this, - "Bound {Property} to {Binding} with priority LocalValue", - property, - description); + if (Logger.IsEnabled(LogEventLevel.Verbose)) + { + var description = GetDescription(source); + + Logger.Log( + LogEventLevel.Verbose, + LogArea.Property, + this, + "Bound {Property} to {Binding} with priority LocalValue", + property, + description); + } if (_directBindings == null) { @@ -351,13 +355,19 @@ namespace Avalonia } else { - Logger.Verbose( - LogArea.Property, - this, - "Bound {Property} to {Binding} with priority {Priority}", - property, - description, - priority); + if (Logger.IsEnabled(LogEventLevel.Verbose)) + { + var description = GetDescription(source); + + Logger.Log( + LogEventLevel.Verbose, + LogArea.Property, + this, + "Bound {Property} to {Binding} with priority {Priority}", + property, + description, + priority); + } return Values.AddBinding(property, source, priority); } @@ -406,14 +416,18 @@ namespace Avalonia { RaisePropertyChanged(property, oldValue, newValue, (BindingPriority)priority); - Logger.Verbose( - LogArea.Property, - this, - "{Property} changed from {$Old} to {$Value} with priority {Priority}", - property, - oldValue, - newValue, - (BindingPriority)priority); + if (Logger.IsEnabled(LogEventLevel.Verbose)) + { + Logger.Log( + LogEventLevel.Verbose, + LogArea.Property, + this, + "{Property} changed from {$Old} to {$Value} with priority {Priority}", + property, + oldValue, + newValue, + (BindingPriority)priority); + } } } @@ -812,7 +826,13 @@ namespace Avalonia /// The priority. private void LogPropertySet(AvaloniaProperty property, object value, BindingPriority priority) { - Logger.Verbose( + if (!Logger.IsEnabled(LogEventLevel.Verbose)) + { + return; + } + + Logger.Log( + LogEventLevel.Verbose, LogArea.Property, this, "Set {Property} to {$Value} with priority {Priority}", diff --git a/src/Avalonia.Base/Logging/ILogSink.cs b/src/Avalonia.Base/Logging/ILogSink.cs index 0ed4eede8f..8b5751b0af 100644 --- a/src/Avalonia.Base/Logging/ILogSink.cs +++ b/src/Avalonia.Base/Logging/ILogSink.cs @@ -8,6 +8,77 @@ namespace Avalonia.Logging /// public interface ILogSink { + /// + /// Checks if given log level is enabled. + /// + /// The log event level. + /// if given log level is enabled. + bool IsEnabled(LogEventLevel level); + + /// + /// Logs an event. + /// + /// The log event level. + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + void Log( + LogEventLevel level, + string area, + object source, + string messageTemplate); + + /// + /// Logs an event. + /// + /// The log event level. + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + /// Message property value. + void Log( + LogEventLevel level, + string area, + object source, + string messageTemplate, + T0 propertyValue0); + + /// + /// Logs an event. + /// + /// The log event level. + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + /// Message property value. + /// Message property value. + void Log( + LogEventLevel level, + string area, + object source, + string messageTemplate, + T0 propertyValue0, + T1 propertyValue1); + + /// + /// Logs an event. + /// + /// The log event level. + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + /// Message property value. + /// Message property value. + /// Message property value. + void Log( + LogEventLevel level, + string area, + object source, + string messageTemplate, + T0 propertyValue0, + T1 propertyValue1, + T2 propertyValue2); + /// /// Logs a new event. /// diff --git a/src/Avalonia.Base/Logging/Logger.cs b/src/Avalonia.Base/Logging/Logger.cs index b1132ff4a9..c46edd4b4b 100644 --- a/src/Avalonia.Base/Logging/Logger.cs +++ b/src/Avalonia.Base/Logging/Logger.cs @@ -15,6 +15,96 @@ namespace Avalonia.Logging /// public static ILogSink Sink { get; set; } + /// + /// Checks if given log level is enabled. + /// + /// The log event level. + /// if given log level is enabled. + public static bool IsEnabled(LogEventLevel level) + { + return Sink?.IsEnabled(level) == true; + } + + /// + /// Logs an event. + /// + /// The log event level. + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void Log( + LogEventLevel level, + string area, + object source, + string messageTemplate) + { + Sink?.Log(level, area, source, messageTemplate); + } + + /// + /// Logs an event. + /// + /// The log event level. + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + /// Message property value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void Log( + LogEventLevel level, + string area, + object source, + string messageTemplate, + T0 propertyValue0) + { + Sink?.Log(level, area, source, messageTemplate, propertyValue0); + } + + /// + /// Logs an event. + /// + /// The log event level. + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + /// Message property value. + /// Message property value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void Log( + LogEventLevel level, + string area, + object source, + string messageTemplate, + T0 propertyValue0, + T1 propertyValue1) + { + Sink?.Log(level, area, source, messageTemplate, propertyValue0, propertyValue1); + } + + /// + /// Logs an event. + /// + /// The log event level. + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + /// Message property value. + /// Message property value. + /// Message property value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void Log( + LogEventLevel level, + string area, + object source, + string messageTemplate, + T0 propertyValue0, + T1 propertyValue1, + T2 propertyValue2) + { + Sink?.Log(level, area, source, messageTemplate, propertyValue0, propertyValue1, propertyValue2); + } + /// /// Logs an event. /// diff --git a/src/Avalonia.Controls/Primitives/TemplatedControl.cs b/src/Avalonia.Controls/Primitives/TemplatedControl.cs index 47c3240374..daf79b3558 100644 --- a/src/Avalonia.Controls/Primitives/TemplatedControl.cs +++ b/src/Avalonia.Controls/Primitives/TemplatedControl.cs @@ -255,7 +255,7 @@ namespace Avalonia.Controls.Primitives if (template != null) { - Logger.Verbose(LogArea.Control, this, "Creating control template"); + Logger.Log(LogEventLevel.Verbose, LogArea.Control, this, "Creating control template"); var (child, nameScope) = template.Build(this); ApplyTemplatedParent(child); diff --git a/src/Avalonia.Layout/LayoutManager.cs b/src/Avalonia.Layout/LayoutManager.cs index 45efccc1fa..ef5464c4ae 100644 --- a/src/Avalonia.Layout/LayoutManager.cs +++ b/src/Avalonia.Layout/LayoutManager.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; +using System.Diagnostics; using Avalonia.Logging; using Avalonia.Threading; @@ -69,15 +70,22 @@ namespace Avalonia.Layout { _running = true; - Logger.Information( - LogArea.Layout, - this, - "Started layout pass. To measure: {Measure} To arrange: {Arrange}", - _toMeasure.Count, - _toArrange.Count); + Stopwatch stopwatch = null; - var stopwatch = new System.Diagnostics.Stopwatch(); - stopwatch.Start(); + bool captureTiming = Logger.IsEnabled(LogEventLevel.Information); + + if (captureTiming) + { + Logger.Log(LogEventLevel.Information, + LogArea.Layout, + this, + "Started layout pass. To measure: {Measure} To arrange: {Arrange}", + _toMeasure.Count, + _toArrange.Count); + + stopwatch = new Stopwatch(); + stopwatch.Start(); + } _toMeasure.BeginLoop(MaxPasses); _toArrange.BeginLoop(MaxPasses); @@ -103,8 +111,12 @@ namespace Avalonia.Layout _toMeasure.EndLoop(); _toArrange.EndLoop(); - stopwatch.Stop(); - Logger.Information(LogArea.Layout, this, "Layout pass finished in {Time}", stopwatch.Elapsed); + if (captureTiming) + { + stopwatch.Stop(); + + Logger.Information(LogArea.Layout, this, "Layout pass finished in {Time}", stopwatch.Elapsed); + } } _queued = false; diff --git a/src/Avalonia.Layout/Layoutable.cs b/src/Avalonia.Layout/Layoutable.cs index bd248d6d44..9b5f66b19d 100644 --- a/src/Avalonia.Layout/Layoutable.cs +++ b/src/Avalonia.Layout/Layoutable.cs @@ -329,7 +329,7 @@ namespace Avalonia.Layout DesiredSize = desiredSize; _previousMeasure = availableSize; - Logger.Verbose(LogArea.Layout, this, "Measure requested {DesiredSize}", DesiredSize); + Logger.Log(LogEventLevel.Verbose, LogArea.Layout, this, "Measure requested {DesiredSize}", DesiredSize); if (DesiredSize != previousDesiredSize) { @@ -356,7 +356,7 @@ namespace Avalonia.Layout if (!IsArrangeValid || _previousArrange != rect) { - Logger.Verbose(LogArea.Layout, this, "Arrange to {Rect} ", rect); + Logger.Log(LogEventLevel.Verbose, LogArea.Layout, this, "Arrange to {Rect} ", rect); IsArrangeValid = true; ArrangeCore(rect); @@ -381,7 +381,7 @@ namespace Avalonia.Layout { if (IsMeasureValid) { - Logger.Verbose(LogArea.Layout, this, "Invalidated measure"); + Logger.Log(LogEventLevel.Verbose, LogArea.Layout, this, "Invalidated measure"); IsMeasureValid = false; IsArrangeValid = false; @@ -402,7 +402,7 @@ namespace Avalonia.Layout { if (IsArrangeValid) { - Logger.Verbose(LogArea.Layout, this, "Invalidated arrange"); + Logger.Log(LogEventLevel.Verbose, LogArea.Layout, this, "Invalidated arrange"); IsArrangeValid = false; (VisualRoot as ILayoutRoot)?.LayoutManager?.InvalidateArrange(this); diff --git a/src/Avalonia.Logging.Serilog/SerilogLogger.cs b/src/Avalonia.Logging.Serilog/SerilogLogger.cs index 0534fe3012..895ee268d2 100644 --- a/src/Avalonia.Logging.Serilog/SerilogLogger.cs +++ b/src/Avalonia.Logging.Serilog/SerilogLogger.cs @@ -34,6 +34,76 @@ namespace Avalonia.Logging.Serilog Logger.Sink = new SerilogLogger(output); } + public bool IsEnabled(LogEventLevel level) + { + return _output.IsEnabled((SerilogLogEventLevel)level); + } + + public void Log( + LogEventLevel level, + string area, + object source, + string messageTemplate) + { + Contract.Requires(area != null); + Contract.Requires(messageTemplate != null); + + using (PushLogContextProperties(area, source)) + { + _output.Write((SerilogLogEventLevel)level, messageTemplate); + } + } + + public void Log( + LogEventLevel level, + string area, object source, + string messageTemplate, + T0 propertyValue0) + { + Contract.Requires(area != null); + Contract.Requires(messageTemplate != null); + + using (PushLogContextProperties(area, source)) + { + _output.Write((SerilogLogEventLevel)level, messageTemplate, propertyValue0); + } + } + + public void Log( + LogEventLevel level, + string area, + object source, + string messageTemplate, + T0 propertyValue0, + T1 propertyValue1) + { + Contract.Requires(area != null); + Contract.Requires(messageTemplate != null); + + using (PushLogContextProperties(area, source)) + { + _output.Write((SerilogLogEventLevel)level, messageTemplate, propertyValue0, propertyValue1); + } + } + + public void Log( + LogEventLevel level, + string area, + object source, + string messageTemplate, + T0 propertyValue0, + T1 propertyValue1, + T2 propertyValue2) + { + Contract.Requires(area != null); + Contract.Requires(messageTemplate != null); + + using (PushLogContextProperties(area, source)) + { + _output.Write((SerilogLogEventLevel)level, messageTemplate, propertyValue0, propertyValue1, propertyValue2); + } + } + /// public void Log( AvaloniaLogEventLevel level, @@ -45,12 +115,40 @@ namespace Avalonia.Logging.Serilog Contract.Requires(area != null); Contract.Requires(messageTemplate != null); - using (LogContext.PushProperty("Area", area)) - using (LogContext.PushProperty("SourceType", source?.GetType())) - using (LogContext.PushProperty("SourceHash", source?.GetHashCode())) + using (PushLogContextProperties(area, source)) { _output.Write((SerilogLogEventLevel)level, messageTemplate, propertyValues); } } + + private static LogContextDisposable PushLogContextProperties(string area, object source) + { + return new LogContextDisposable( + LogContext.PushProperty("Area", area), + LogContext.PushProperty("SourceType", source?.GetType()), + LogContext.PushProperty("SourceHash", source?.GetHashCode()) + ); + } + + private readonly struct LogContextDisposable : IDisposable + { + private readonly IDisposable _areaDisposable; + private readonly IDisposable _sourceTypeDisposable; + private readonly IDisposable _sourceHashDisposable; + + public LogContextDisposable(IDisposable areaDisposable, IDisposable sourceTypeDisposable, IDisposable sourceHashDisposable) + { + _areaDisposable = areaDisposable; + _sourceTypeDisposable = sourceTypeDisposable; + _sourceHashDisposable = sourceHashDisposable; + } + + public void Dispose() + { + _areaDisposable.Dispose(); + _sourceTypeDisposable.Dispose(); + _sourceHashDisposable.Dispose(); + } + } } } diff --git a/src/Avalonia.Visuals/Visual.cs b/src/Avalonia.Visuals/Visual.cs index 1f2d67b69e..8d32371c6d 100644 --- a/src/Avalonia.Visuals/Visual.cs +++ b/src/Avalonia.Visuals/Visual.cs @@ -359,7 +359,7 @@ namespace Avalonia /// The event args. protected virtual void OnAttachedToVisualTreeCore(VisualTreeAttachmentEventArgs e) { - Logger.Verbose(LogArea.Visual, this, "Attached to visual tree"); + Logger.Log(LogEventLevel.Verbose, LogArea.Visual, this, "Attached to visual tree"); _visualRoot = e.Root; @@ -388,7 +388,7 @@ namespace Avalonia /// The event args. protected virtual void OnDetachedFromVisualTreeCore(VisualTreeAttachmentEventArgs e) { - Logger.Verbose(LogArea.Visual, this, "Detached from visual tree"); + Logger.Log(LogEventLevel.Verbose, LogArea.Visual, this, "Detached from visual tree"); _visualRoot = null; diff --git a/tests/Avalonia.UnitTests/TestLogSink.cs b/tests/Avalonia.UnitTests/TestLogSink.cs index 8e4dd7164f..a2b188c273 100644 --- a/tests/Avalonia.UnitTests/TestLogSink.cs +++ b/tests/Avalonia.UnitTests/TestLogSink.cs @@ -16,7 +16,7 @@ namespace Avalonia.UnitTests public class TestLogSink : ILogSink { - private LogCallback _callback; + private readonly LogCallback _callback; public TestLogSink(LogCallback callback) { @@ -30,7 +30,35 @@ namespace Avalonia.UnitTests return Disposable.Create(() => Logger.Sink = null); } - public void Log(LogEventLevel level, string area, object source, string messageTemplate, params object[] propertyValues) + public bool IsEnabled(LogEventLevel level) + { + return true; + } + + public void Log(LogEventLevel level, string area, object source, string messageTemplate) + { + _callback(level, area, source, messageTemplate); + } + + public void Log(LogEventLevel level, string area, object source, string messageTemplate, T0 propertyValue0) + { + _callback(level, area, source, messageTemplate, propertyValue0); + } + + public void Log(LogEventLevel level, string area, object source, string messageTemplate, + T0 propertyValue0, T1 propertyValue1) + { + _callback(level, area, source, messageTemplate, propertyValue0, propertyValue1); + } + + public void Log(LogEventLevel level, string area, object source, string messageTemplate, + T0 propertyValue0, T1 propertyValue1, T2 propertyValue2) + { + _callback(level, area, source, messageTemplate, propertyValue0, propertyValue1, propertyValue2); + } + + public void Log(LogEventLevel level, string area, object source, string messageTemplate, + params object[] propertyValues) { _callback(level, area, source, messageTemplate, propertyValues); } From 068b73750b0f7bb5adc67c5470e30589eb36b79f Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Fri, 4 Oct 2019 00:01:44 +0200 Subject: [PATCH 2/9] Cleanup conditional logging API. --- src/Avalonia.Base/AvaloniaObject.cs | 49 +++----- src/Avalonia.Base/Logging/Logger.cs | 30 +++++ .../Logging/ParametrizedLogger.cs | 116 ++++++++++++++++++ 3 files changed, 166 insertions(+), 29 deletions(-) create mode 100644 src/Avalonia.Base/Logging/ParametrizedLogger.cs diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 22a85663c7..ddcf16e11a 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -333,12 +333,11 @@ namespace Avalonia throw new ArgumentException($"The property {property.Name} is readonly."); } - if (Logger.IsEnabled(LogEventLevel.Verbose)) + if (Logger.TryGetLogger(LogEventLevel.Verbose, out var logger)) { var description = GetDescription(source); - Logger.Log( - LogEventLevel.Verbose, + logger.Log( LogArea.Property, this, "Bound {Property} to {Binding} with priority LocalValue", @@ -355,12 +354,11 @@ namespace Avalonia } else { - if (Logger.IsEnabled(LogEventLevel.Verbose)) + if (Logger.TryGetLogger(LogEventLevel.Verbose, out var logger)) { var description = GetDescription(source); - Logger.Log( - LogEventLevel.Verbose, + logger.Log( LogArea.Property, this, "Bound {Property} to {Binding} with priority {Priority}", @@ -416,18 +414,14 @@ namespace Avalonia { RaisePropertyChanged(property, oldValue, newValue, (BindingPriority)priority); - if (Logger.IsEnabled(LogEventLevel.Verbose)) - { - Logger.Log( - LogEventLevel.Verbose, - LogArea.Property, - this, - "{Property} changed from {$Old} to {$Value} with priority {Priority}", - property, - oldValue, - newValue, - (BindingPriority)priority); - } + Logger.TryGetLogger(LogEventLevel.Verbose)?.Log( + LogArea.Property, + this, + "{Property} changed from {$Old} to {$Value} with priority {Priority}", + property, + oldValue, + newValue, + (BindingPriority)priority); } } @@ -826,19 +820,16 @@ namespace Avalonia /// The priority. private void LogPropertySet(AvaloniaProperty property, object value, BindingPriority priority) { - if (!Logger.IsEnabled(LogEventLevel.Verbose)) + if (Logger.TryGetLogger(LogEventLevel.Verbose, out var logger)) { - return; + logger.Log( + LogArea.Property, + this, + "Set {Property} to {$Value} with priority {Priority}", + property, + value, + priority); } - - Logger.Log( - LogEventLevel.Verbose, - LogArea.Property, - this, - "Set {Property} to {$Value} with priority {Priority}", - property, - value, - priority); } private class DirectBindingSubscription : IObserver, IDisposable diff --git a/src/Avalonia.Base/Logging/Logger.cs b/src/Avalonia.Base/Logging/Logger.cs index c46edd4b4b..ef4c29b9f3 100644 --- a/src/Avalonia.Base/Logging/Logger.cs +++ b/src/Avalonia.Base/Logging/Logger.cs @@ -25,6 +25,36 @@ namespace Avalonia.Logging return Sink?.IsEnabled(level) == true; } + /// + /// Returns parametrized logging sink if given log level is enabled. + /// + /// The log event level. + /// Log sink or if log level is not enabled. + public static ParametrizedLogger? TryGetLogger(LogEventLevel level) + { + if (!IsEnabled(level)) + { + return null; + } + + return new ParametrizedLogger(Sink, level); + } + + /// + /// Returns parametrized logging sink if given log level is enabled. + /// + /// The log event level. + /// Log sink that is valid only if method returns . + /// if logger was obtained successfully. + public static bool TryGetLogger(LogEventLevel level, out ParametrizedLogger outLogger) + { + ParametrizedLogger? logger = TryGetLogger(level); + + outLogger = logger.GetValueOrDefault(); + + return logger.HasValue; + } + /// /// Logs an event. /// diff --git a/src/Avalonia.Base/Logging/ParametrizedLogger.cs b/src/Avalonia.Base/Logging/ParametrizedLogger.cs new file mode 100644 index 0000000000..1a0678c79a --- /dev/null +++ b/src/Avalonia.Base/Logging/ParametrizedLogger.cs @@ -0,0 +1,116 @@ +// 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.Runtime.CompilerServices; + +namespace Avalonia.Logging +{ + /// + /// Logger sink parametrized for given logging level. + /// + public readonly struct ParametrizedLogger + { + private readonly ILogSink _sink; + private readonly LogEventLevel _level; + + public ParametrizedLogger(ILogSink sink, LogEventLevel level) + { + _sink = sink; + _level = level; + } + + /// + /// Checks if this logger can be used. + /// + public bool IsValid => _sink != null; + + /// + /// Logs an event. + /// + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Log( + string area, + object source, + string messageTemplate) + { + _sink.Log(_level, area, source, messageTemplate); + } + + /// + /// Logs an event. + /// + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + /// Message property value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Log( + string area, + object source, + string messageTemplate, + T0 propertyValue0) + { + _sink.Log(_level, area, source, messageTemplate, propertyValue0); + } + + /// + /// Logs an event. + /// + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + /// Message property value. + /// Message property value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Log( + string area, + object source, + string messageTemplate, + T0 propertyValue0, + T1 propertyValue1) + { + _sink.Log(_level, area, source, messageTemplate, propertyValue0, propertyValue1); + } + + /// + /// Logs an event. + /// + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + /// Message property value. + /// Message property value. + /// Message property value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Log( + string area, + object source, + string messageTemplate, + T0 propertyValue0, + T1 propertyValue1, + T2 propertyValue2) + { + _sink.Log(_level, area, source, messageTemplate, propertyValue0, propertyValue1, propertyValue2); + } + + /// + /// Logs an event. + /// + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + /// The message property values. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Log( + string area, + object source, + string messageTemplate, + params object[] propertyValues) + { + _sink.Log(_level, area, source, messageTemplate, propertyValues); + } + } +} From 1f693404526da965fad8f6d30aef138125b9e9f6 Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Sun, 6 Oct 2019 00:26:10 +0200 Subject: [PATCH 3/9] Cleanup. --- src/Avalonia.Base/AvaloniaObject.cs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index ddcf16e11a..8d9a98d087 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -820,16 +820,13 @@ namespace Avalonia /// The priority. private void LogPropertySet(AvaloniaProperty property, object value, BindingPriority priority) { - if (Logger.TryGetLogger(LogEventLevel.Verbose, out var logger)) - { - logger.Log( - LogArea.Property, - this, - "Set {Property} to {$Value} with priority {Priority}", - property, - value, - priority); - } + Logger.TryGetLogger(LogEventLevel.Verbose)?.Log( + LogArea.Property, + this, + "Set {Property} to {$Value} with priority {Priority}", + property, + value, + priority); } private class DirectBindingSubscription : IObserver, IDisposable From 2116439c727d8e9001975e3cdf4691e10b9b39d4 Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Sun, 6 Oct 2019 15:53:39 +0200 Subject: [PATCH 4/9] Optimize TypeNameAndClassSelector. --- .../Styling/TypeNameAndClassSelector.cs | 90 +++++++++++-------- 1 file changed, 51 insertions(+), 39 deletions(-) diff --git a/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs b/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs index 362ac86e50..f1fd2f6c7f 100644 --- a/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs +++ b/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs @@ -18,8 +18,9 @@ namespace Avalonia.Styling internal class TypeNameAndClassSelector : Selector { private readonly Selector _previous; + private readonly Lazy> _classes = new Lazy>(() => new List()); private Type _targetType; - private Lazy> _classes = new Lazy>(() => new List()); + private string _selectorString; public static TypeNameAndClassSelector OfType(Selector previous, Type targetType) @@ -27,6 +28,7 @@ namespace Avalonia.Styling var result = new TypeNameAndClassSelector(previous); result._targetType = targetType; result.IsConcreteType = true; + return result; } @@ -35,6 +37,7 @@ namespace Avalonia.Styling var result = new TypeNameAndClassSelector(previous); result._targetType = targetType; result.IsConcreteType = false; + return result; } @@ -42,6 +45,7 @@ namespace Avalonia.Styling { var result = new TypeNameAndClassSelector(previous); result.Name = name; + return result; } @@ -49,6 +53,7 @@ namespace Avalonia.Styling { var result = new TypeNameAndClassSelector(previous); result.Classes.Add(className); + return result; } @@ -126,9 +131,11 @@ namespace Avalonia.Styling if (subscribe) { var observable = new ClassObserver(control.Classes, _classes.Value); + return new SelectorMatch(observable); } - else if (!Matches(control.Classes)) + + if (!AreClassesMatching(control.Classes, Classes)) { return SelectorMatch.NeverThisInstance; } @@ -139,21 +146,6 @@ namespace Avalonia.Styling protected override Selector MovePrevious() => _previous; - private bool Matches(IEnumerable classes) - { - int remaining = Classes.Count; - - foreach (var c in classes) - { - if (Classes.Contains(c)) - { - --remaining; - } - } - - return remaining == 0; - } - private string BuildSelectorString() { var builder = new StringBuilder(); @@ -199,11 +191,41 @@ namespace Avalonia.Styling return builder.ToString(); } - private class ClassObserver : LightweightObservableBase + private static bool AreClassesMatching(IReadOnlyList classes, IList toMatch) { - readonly IList _match; - IAvaloniaReadOnlyList _classes; - bool _value; + int remainingMatches = toMatch.Count; + int classesCount = classes.Count; + + // Early bail out - we can't match if control does not have enough classes. + if (classesCount < remainingMatches) + { + return false; + } + + for (var i = 0; i < classesCount; i++) + { + var c = classes[i]; + + if (toMatch.Contains(c)) + { + --remainingMatches; + + // Already matched so we can skip checking other classes. + if (remainingMatches == 0) + { + break; + } + } + } + + return remainingMatches == 0; + } + + private sealed class ClassObserver : LightweightObservableBase + { + private readonly IList _match; + private readonly IAvaloniaReadOnlyList _classes; + private bool _hasMatch; public ClassObserver(IAvaloniaReadOnlyList classes, IList match) { @@ -215,42 +237,32 @@ namespace Avalonia.Styling protected override void Initialize() { - _value = GetResult(); + _hasMatch = IsMatching(); _classes.CollectionChanged += ClassesChanged; } protected override void Subscribed(IObserver observer, bool first) { - observer.OnNext(_value); + observer.OnNext(_hasMatch); } private void ClassesChanged(object sender, NotifyCollectionChangedEventArgs e) { if (e.Action != NotifyCollectionChangedAction.Move) { - var value = GetResult(); + var hasMatch = IsMatching(); - if (value != _value) + if (hasMatch != _hasMatch) { - PublishNext(GetResult()); - _value = value; + PublishNext(hasMatch); + _hasMatch = hasMatch; } } } - private bool GetResult() + private bool IsMatching() { - int remaining = _match.Count; - - foreach (var c in _classes) - { - if (_match.Contains(c)) - { - --remaining; - } - } - - return remaining == 0; + return AreClassesMatching(_classes, _match); } } } From 5814bd7163193c614dbffd8364b04bece257afa3 Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Sun, 6 Oct 2019 22:16:07 +0200 Subject: [PATCH 5/9] Remove direct logging functions. --- src/Avalonia.Base/AvaloniaObject.cs | 11 +- .../Data/Core/BindingExpression.cs | 2 +- src/Avalonia.Base/Logging/Logger.cs | 209 +----------------- .../Logging/ParametrizedLogger.cs | 66 +++++- src/Avalonia.Base/PriorityValue.cs | 2 +- src/Avalonia.Controls/DropDown.cs | 4 +- .../Primitives/SelectingItemsControl.cs | 2 +- .../Primitives/TemplatedControl.cs | 2 +- src/Avalonia.Controls/TopLevel.cs | 2 +- src/Avalonia.Layout/LayoutManager.cs | 7 +- src/Avalonia.Layout/Layoutable.cs | 8 +- src/Avalonia.OpenGL/EglGlPlatformFeature.cs | 2 +- src/Avalonia.Styling/StyledElement.cs | 4 +- .../Animation/Animators/TransformAnimator.cs | 4 +- .../Rendering/DeferredRenderer.cs | 3 +- .../Rendering/ImmediateRenderer.cs | 3 +- src/Avalonia.Visuals/Rendering/RenderLoop.cs | 4 +- src/Avalonia.Visuals/Visual.cs | 7 +- src/Avalonia.X11/Glx/GlxPlatformFeature.cs | 2 +- .../AvaloniaPropertyTypeConverter.cs | 2 +- .../Markup/Data/DelayedBinding.cs | 2 +- .../Media/StreamGeometryContextImpl.cs | 2 +- 22 files changed, 103 insertions(+), 247 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 8d9a98d087..1d49401d99 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -333,7 +333,7 @@ namespace Avalonia throw new ArgumentException($"The property {property.Name} is readonly."); } - if (Logger.TryGetLogger(LogEventLevel.Verbose, out var logger)) + if (Logger.TryGet(LogEventLevel.Verbose, out var logger)) { var description = GetDescription(source); @@ -354,7 +354,7 @@ namespace Avalonia } else { - if (Logger.TryGetLogger(LogEventLevel.Verbose, out var logger)) + if (Logger.TryGet(LogEventLevel.Verbose, out var logger)) { var description = GetDescription(source); @@ -414,7 +414,7 @@ namespace Avalonia { RaisePropertyChanged(property, oldValue, newValue, (BindingPriority)priority); - Logger.TryGetLogger(LogEventLevel.Verbose)?.Log( + Logger.TryGet(LogEventLevel.Verbose)?.Log( LogArea.Property, this, "{Property} changed from {$Old} to {$Value} with priority {Priority}", @@ -466,8 +466,7 @@ namespace Avalonia /// The binding error. protected internal virtual void LogBindingError(AvaloniaProperty property, Exception e) { - Logger.Log( - LogEventLevel.Warning, + Logger.TryGet(LogEventLevel.Warning)?.Log( LogArea.Binding, this, "Error in binding to {Target}.{Property}: {Message}", @@ -820,7 +819,7 @@ namespace Avalonia /// The priority. private void LogPropertySet(AvaloniaProperty property, object value, BindingPriority priority) { - Logger.TryGetLogger(LogEventLevel.Verbose)?.Log( + Logger.TryGet(LogEventLevel.Verbose)?.Log( LogArea.Property, this, "Set {Property} to {$Value} with priority {Priority}", diff --git a/src/Avalonia.Base/Data/Core/BindingExpression.cs b/src/Avalonia.Base/Data/Core/BindingExpression.cs index 7f8396cdfa..986e2cf012 100644 --- a/src/Avalonia.Base/Data/Core/BindingExpression.cs +++ b/src/Avalonia.Base/Data/Core/BindingExpression.cs @@ -165,7 +165,7 @@ namespace Avalonia.Data.Core } else { - Logger.Error( + Logger.TryGet(LogEventLevel.Error)?.Log( LogArea.Binding, this, "Could not convert FallbackValue {FallbackValue} to {Type}", diff --git a/src/Avalonia.Base/Logging/Logger.cs b/src/Avalonia.Base/Logging/Logger.cs index ef4c29b9f3..c895c70094 100644 --- a/src/Avalonia.Base/Logging/Logger.cs +++ b/src/Avalonia.Base/Logging/Logger.cs @@ -1,8 +1,6 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. -using System.Runtime.CompilerServices; - namespace Avalonia.Logging { /// @@ -30,7 +28,7 @@ namespace Avalonia.Logging /// /// The log event level. /// Log sink or if log level is not enabled. - public static ParametrizedLogger? TryGetLogger(LogEventLevel level) + public static ParametrizedLogger? TryGet(LogEventLevel level) { if (!IsEnabled(level)) { @@ -46,214 +44,13 @@ namespace Avalonia.Logging /// The log event level. /// Log sink that is valid only if method returns . /// if logger was obtained successfully. - public static bool TryGetLogger(LogEventLevel level, out ParametrizedLogger outLogger) + public static bool TryGet(LogEventLevel level, out ParametrizedLogger outLogger) { - ParametrizedLogger? logger = TryGetLogger(level); + ParametrizedLogger? logger = TryGet(level); outLogger = logger.GetValueOrDefault(); return logger.HasValue; } - - /// - /// Logs an event. - /// - /// The log event level. - /// The area that the event originates. - /// The object from which the event originates. - /// The message template. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Log( - LogEventLevel level, - string area, - object source, - string messageTemplate) - { - Sink?.Log(level, area, source, messageTemplate); - } - - /// - /// Logs an event. - /// - /// The log event level. - /// The area that the event originates. - /// The object from which the event originates. - /// The message template. - /// Message property value. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Log( - LogEventLevel level, - string area, - object source, - string messageTemplate, - T0 propertyValue0) - { - Sink?.Log(level, area, source, messageTemplate, propertyValue0); - } - - /// - /// Logs an event. - /// - /// The log event level. - /// The area that the event originates. - /// The object from which the event originates. - /// The message template. - /// Message property value. - /// Message property value. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Log( - LogEventLevel level, - string area, - object source, - string messageTemplate, - T0 propertyValue0, - T1 propertyValue1) - { - Sink?.Log(level, area, source, messageTemplate, propertyValue0, propertyValue1); - } - - /// - /// Logs an event. - /// - /// The log event level. - /// The area that the event originates. - /// The object from which the event originates. - /// The message template. - /// Message property value. - /// Message property value. - /// Message property value. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Log( - LogEventLevel level, - string area, - object source, - string messageTemplate, - T0 propertyValue0, - T1 propertyValue1, - T2 propertyValue2) - { - Sink?.Log(level, area, source, messageTemplate, propertyValue0, propertyValue1, propertyValue2); - } - - /// - /// Logs an event. - /// - /// The log event level. - /// The area that the event originates. - /// The object from which the event originates. - /// The message template. - /// The message property values. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Log( - LogEventLevel level, - string area, - object source, - string messageTemplate, - params object[] propertyValues) - { - Sink?.Log(level, area, source, messageTemplate, propertyValues); - } - - /// - /// Logs an event with the level. - /// - /// The area that the event originates. - /// The object from which the event originates. - /// The message template. - /// The message property values. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Verbose( - string area, - object source, - string messageTemplate, - params object[] propertyValues) - { - Log(LogEventLevel.Verbose, area, source, messageTemplate, propertyValues); - } - - /// - /// Logs an event with the level. - /// - /// The area that the event originates. - /// The object from which the event originates. - /// The message template. - /// The message property values. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Debug( - string area, - object source, - string messageTemplate, - params object[] propertyValues) - { - Log(LogEventLevel.Debug, area, source, messageTemplate, propertyValues); - } - - /// - /// Logs an event with the level. - /// - /// The area that the event originates. - /// The object from which the event originates. - /// The message template. - /// The message property values. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Information( - string area, - object source, - string messageTemplate, - params object[] propertyValues) - { - Log(LogEventLevel.Information, area, source, messageTemplate, propertyValues); - } - - /// - /// Logs an event with the level. - /// - /// The area that the event originates. - /// The object from which the event originates. - /// The message template. - /// The message property values. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Warning( - string area, - object source, - string messageTemplate, - params object[] propertyValues) - { - Log(LogEventLevel.Warning, area, source, messageTemplate, propertyValues); - } - - /// - /// Logs an event with the level. - /// - /// The area that the event originates. - /// The object from which the event originates. - /// The message template. - /// The message property values. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Error( - string area, - object source, - string messageTemplate, - params object[] propertyValues) - { - Log(LogEventLevel.Error, area, source, messageTemplate, propertyValues); - } - - /// - /// Logs an event with the level. - /// - /// The area that the event originates. - /// The object from which the event originates. - /// The message template. - /// The message property values. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Fatal( - string area, - object source, - string messageTemplate, - params object[] propertyValues) - { - Log(LogEventLevel.Fatal, area, source, messageTemplate, propertyValues); - } } } diff --git a/src/Avalonia.Base/Logging/ParametrizedLogger.cs b/src/Avalonia.Base/Logging/ParametrizedLogger.cs index 1a0678c79a..1550cc1b40 100644 --- a/src/Avalonia.Base/Logging/ParametrizedLogger.cs +++ b/src/Avalonia.Base/Logging/ParametrizedLogger.cs @@ -102,15 +102,73 @@ namespace Avalonia.Logging /// The area that the event originates. /// The object from which the event originates. /// The message template. - /// The message property values. + /// Message property value. + /// Message property value. + /// Message property value. + /// Message property value. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Log( + public void Log( + string area, + object source, + string messageTemplate, + T0 propertyValue0, + T1 propertyValue1, + T2 propertyValue2, + T3 propertyValue3) + { + _sink.Log(_level, area, source, messageTemplate, propertyValue0, propertyValue1, propertyValue2, propertyValue3); + } + + /// + /// Logs an event. + /// + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + /// Message property value. + /// Message property value. + /// Message property value. + /// Message property value. + /// Message property value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Log( string area, object source, string messageTemplate, - params object[] propertyValues) + T0 propertyValue0, + T1 propertyValue1, + T2 propertyValue2, + T3 propertyValue3, + T4 propertyValue4) + { + _sink.Log(_level, area, source, messageTemplate, propertyValue0, propertyValue1, propertyValue2, propertyValue3, propertyValue4); + } + + /// + /// Logs an event. + /// + /// The area that the event originates. + /// The object from which the event originates. + /// The message template. + /// Message property value. + /// Message property value. + /// Message property value. + /// Message property value. + /// Message property value. + /// Message property value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Log( + string area, + object source, + string messageTemplate, + T0 propertyValue0, + T1 propertyValue1, + T2 propertyValue2, + T3 propertyValue3, + T4 propertyValue4, + T5 propertyValue5) { - _sink.Log(_level, area, source, messageTemplate, propertyValues); + _sink.Log(_level, area, source, messageTemplate, propertyValue0, propertyValue1, propertyValue2, propertyValue3, propertyValue4, propertyValue5); } } } diff --git a/src/Avalonia.Base/PriorityValue.cs b/src/Avalonia.Base/PriorityValue.cs index 2871271062..61184ef7b1 100644 --- a/src/Avalonia.Base/PriorityValue.cs +++ b/src/Avalonia.Base/PriorityValue.cs @@ -301,7 +301,7 @@ namespace Avalonia } else { - Logger.Error( + Logger.TryGet(LogEventLevel.Error)?.Log( LogArea.Binding, Owner, "Binding produced invalid value for {$Property} ({$PropertyType}): {$Value} ({$ValueType})", diff --git a/src/Avalonia.Controls/DropDown.cs b/src/Avalonia.Controls/DropDown.cs index b67d9ef89a..9da803d16d 100644 --- a/src/Avalonia.Controls/DropDown.cs +++ b/src/Avalonia.Controls/DropDown.cs @@ -9,7 +9,7 @@ namespace Avalonia.Controls { public DropDown() { - Logger.Warning(LogArea.Control, this, "DropDown is deprecated: Use ComboBox"); + Logger.TryGet(LogEventLevel.Warning)?.Log(LogArea.Control, this, "DropDown is deprecated: Use ComboBox"); } Type IStyleable.StyleKey => typeof(ComboBox); @@ -20,7 +20,7 @@ namespace Avalonia.Controls { public DropDownItem() { - Logger.Warning(LogArea.Control, this, "DropDownItem is deprecated: Use ComboBoxItem"); + Logger.TryGet(LogEventLevel.Warning)?.Log(LogArea.Control, this, "DropDownItem is deprecated: Use ComboBoxItem"); } Type IStyleable.StyleKey => typeof(ComboBoxItem); diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 6869ea0822..c6172c0f36 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -1062,7 +1062,7 @@ namespace Avalonia.Controls.Primitives } catch (Exception ex) { - Logger.Error( + Logger.TryGet(LogEventLevel.Error)?.Log( LogArea.Property, this, "Error thrown updating SelectedItems: {Error}", diff --git a/src/Avalonia.Controls/Primitives/TemplatedControl.cs b/src/Avalonia.Controls/Primitives/TemplatedControl.cs index daf79b3558..7d0f306db8 100644 --- a/src/Avalonia.Controls/Primitives/TemplatedControl.cs +++ b/src/Avalonia.Controls/Primitives/TemplatedControl.cs @@ -255,7 +255,7 @@ namespace Avalonia.Controls.Primitives if (template != null) { - Logger.Log(LogEventLevel.Verbose, LogArea.Control, this, "Creating control template"); + Logger.TryGet(LogEventLevel.Verbose)?.Log(LogArea.Control, this, "Creating control template"); var (child, nameScope) = template.Build(this); ApplyTemplatedParent(child); diff --git a/src/Avalonia.Controls/TopLevel.cs b/src/Avalonia.Controls/TopLevel.cs index e0acab1133..c54ebd5360 100644 --- a/src/Avalonia.Controls/TopLevel.cs +++ b/src/Avalonia.Controls/TopLevel.cs @@ -330,7 +330,7 @@ namespace Avalonia.Controls if (result == null) { - Logger.Warning( + Logger.TryGet(LogEventLevel.Warning)?.Log( LogArea.Control, this, "Could not create {Service} : maybe Application.RegisterServices() wasn't called?", diff --git a/src/Avalonia.Layout/LayoutManager.cs b/src/Avalonia.Layout/LayoutManager.cs index ef5464c4ae..855f123748 100644 --- a/src/Avalonia.Layout/LayoutManager.cs +++ b/src/Avalonia.Layout/LayoutManager.cs @@ -72,11 +72,12 @@ namespace Avalonia.Layout Stopwatch stopwatch = null; - bool captureTiming = Logger.IsEnabled(LogEventLevel.Information); + const LogEventLevel timingLogLevel = LogEventLevel.Information; + bool captureTiming = Logger.IsEnabled(timingLogLevel); if (captureTiming) { - Logger.Log(LogEventLevel.Information, + Logger.TryGet(timingLogLevel)?.Log( LogArea.Layout, this, "Started layout pass. To measure: {Measure} To arrange: {Arrange}", @@ -115,7 +116,7 @@ namespace Avalonia.Layout { stopwatch.Stop(); - Logger.Information(LogArea.Layout, this, "Layout pass finished in {Time}", stopwatch.Elapsed); + Logger.TryGet(timingLogLevel)?.Log(LogArea.Layout, this, "Layout pass finished in {Time}", stopwatch.Elapsed); } } diff --git a/src/Avalonia.Layout/Layoutable.cs b/src/Avalonia.Layout/Layoutable.cs index 9b5f66b19d..b0757a823d 100644 --- a/src/Avalonia.Layout/Layoutable.cs +++ b/src/Avalonia.Layout/Layoutable.cs @@ -329,7 +329,7 @@ namespace Avalonia.Layout DesiredSize = desiredSize; _previousMeasure = availableSize; - Logger.Log(LogEventLevel.Verbose, LogArea.Layout, this, "Measure requested {DesiredSize}", DesiredSize); + Logger.TryGet(LogEventLevel.Verbose)?.Log(LogArea.Layout, this, "Measure requested {DesiredSize}", DesiredSize); if (DesiredSize != previousDesiredSize) { @@ -356,7 +356,7 @@ namespace Avalonia.Layout if (!IsArrangeValid || _previousArrange != rect) { - Logger.Log(LogEventLevel.Verbose, LogArea.Layout, this, "Arrange to {Rect} ", rect); + Logger.TryGet(LogEventLevel.Verbose)?.Log(LogArea.Layout, this, "Arrange to {Rect} ", rect); IsArrangeValid = true; ArrangeCore(rect); @@ -381,7 +381,7 @@ namespace Avalonia.Layout { if (IsMeasureValid) { - Logger.Log(LogEventLevel.Verbose, LogArea.Layout, this, "Invalidated measure"); + Logger.TryGet(LogEventLevel.Verbose)?.Log(LogArea.Layout, this, "Invalidated measure"); IsMeasureValid = false; IsArrangeValid = false; @@ -402,7 +402,7 @@ namespace Avalonia.Layout { if (IsArrangeValid) { - Logger.Log(LogEventLevel.Verbose, LogArea.Layout, this, "Invalidated arrange"); + Logger.TryGet(LogEventLevel.Verbose)?.Log(LogArea.Layout, this, "Invalidated arrange"); IsArrangeValid = false; (VisualRoot as ILayoutRoot)?.LayoutManager?.InvalidateArrange(this); diff --git a/src/Avalonia.OpenGL/EglGlPlatformFeature.cs b/src/Avalonia.OpenGL/EglGlPlatformFeature.cs index 86411b89da..5f5064fba5 100644 --- a/src/Avalonia.OpenGL/EglGlPlatformFeature.cs +++ b/src/Avalonia.OpenGL/EglGlPlatformFeature.cs @@ -31,7 +31,7 @@ namespace Avalonia.OpenGL } catch(Exception e) { - Logger.Error("OpenGL", null, "Unable to initialize EGL-based rendering: {0}", e); + Logger.TryGet(LogEventLevel.Error)?.Log("OpenGL", null, "Unable to initialize EGL-based rendering: {0}", e); return null; } } diff --git a/src/Avalonia.Styling/StyledElement.cs b/src/Avalonia.Styling/StyledElement.cs index 38c29289b6..1465b9eb85 100644 --- a/src/Avalonia.Styling/StyledElement.cs +++ b/src/Avalonia.Styling/StyledElement.cs @@ -743,11 +743,11 @@ namespace Avalonia #if DEBUG if (((INotifyCollectionChangedDebug)_classes).GetCollectionChangedSubscribers()?.Length > 0) { - Logger.Warning( + Logger.TryGet(LogEventLevel.Warning)?.Log( LogArea.Control, this, "{Type} detached from logical tree but still has class listeners", - this.GetType()); + GetType()); } #endif } diff --git a/src/Avalonia.Visuals/Animation/Animators/TransformAnimator.cs b/src/Avalonia.Visuals/Animation/Animators/TransformAnimator.cs index e7be272f13..1f1590bdcd 100644 --- a/src/Avalonia.Visuals/Animation/Animators/TransformAnimator.cs +++ b/src/Avalonia.Visuals/Animation/Animators/TransformAnimator.cs @@ -65,14 +65,14 @@ namespace Avalonia.Animation.Animators } } - Logger.Warning( + Logger.TryGet(LogEventLevel.Warning)?.Log( LogArea.Animations, control, $"Cannot find the appropriate transform: \"{Property.OwnerType}\" in {control}."); } else { - Logger.Error( + Logger.TryGet(LogEventLevel.Error)?.Log( LogArea.Animations, control, $"Cannot apply animation: Target property owner {Property.OwnerType} is not a Transform object."); diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs index efcc555159..d9a68b236a 100644 --- a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.IO; using System.Linq; +using Avalonia.Logging; using Avalonia.Media; using Avalonia.Media.Immutable; using Avalonia.Platform; @@ -269,7 +270,7 @@ namespace Avalonia.Rendering } catch (RenderTargetCorruptedException ex) { - Logging.Logger.Information("Renderer", this, "Render target was corrupted. Exception: {0}", ex); + Logger.TryGet(LogEventLevel.Information)?.Log("Renderer", this, "Render target was corrupted. Exception: {0}", ex); RenderTarget?.Dispose(); RenderTarget = null; } diff --git a/src/Avalonia.Visuals/Rendering/ImmediateRenderer.cs b/src/Avalonia.Visuals/Rendering/ImmediateRenderer.cs index b2d242d4af..68d56eeedd 100644 --- a/src/Avalonia.Visuals/Rendering/ImmediateRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/ImmediateRenderer.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Avalonia.Logging; using Avalonia.Media; using Avalonia.Platform; using Avalonia.VisualTree; @@ -80,7 +81,7 @@ namespace Avalonia.Rendering } catch (RenderTargetCorruptedException ex) { - Logging.Logger.Information("Renderer", this, "Render target was corrupted. Exception: {0}", ex); + Logger.TryGet(LogEventLevel.Information)?.Log("Renderer", this, "Render target was corrupted. Exception: {0}", ex); _renderTarget.Dispose(); _renderTarget = null; } diff --git a/src/Avalonia.Visuals/Rendering/RenderLoop.cs b/src/Avalonia.Visuals/Rendering/RenderLoop.cs index 140688f8bc..c2594658b9 100644 --- a/src/Avalonia.Visuals/Rendering/RenderLoop.cs +++ b/src/Avalonia.Visuals/Rendering/RenderLoop.cs @@ -120,7 +120,7 @@ namespace Avalonia.Rendering } catch (Exception ex) { - Logger.Error(LogArea.Visual, this, "Exception in render update: {Error}", ex); + Logger.TryGet(LogEventLevel.Error)?.Log(LogArea.Visual, this, "Exception in render update: {Error}", ex); } } } @@ -136,7 +136,7 @@ namespace Avalonia.Rendering } catch (Exception ex) { - Logger.Error(LogArea.Visual, this, "Exception in render loop: {Error}", ex); + Logger.TryGet(LogEventLevel.Error)?.Log(LogArea.Visual, this, "Exception in render loop: {Error}", ex); } finally { diff --git a/src/Avalonia.Visuals/Visual.cs b/src/Avalonia.Visuals/Visual.cs index 8d32371c6d..f4306d3929 100644 --- a/src/Avalonia.Visuals/Visual.cs +++ b/src/Avalonia.Visuals/Visual.cs @@ -359,7 +359,7 @@ namespace Avalonia /// The event args. protected virtual void OnAttachedToVisualTreeCore(VisualTreeAttachmentEventArgs e) { - Logger.Log(LogEventLevel.Verbose, LogArea.Visual, this, "Attached to visual tree"); + Logger.TryGet(LogEventLevel.Verbose)?.Log(LogArea.Visual, this, "Attached to visual tree"); _visualRoot = e.Root; @@ -388,7 +388,7 @@ namespace Avalonia /// The event args. protected virtual void OnDetachedFromVisualTreeCore(VisualTreeAttachmentEventArgs e) { - Logger.Log(LogEventLevel.Verbose, LogArea.Visual, this, "Detached from visual tree"); + Logger.TryGet(LogEventLevel.Verbose)?.Log(LogArea.Visual, this, "Detached from visual tree"); _visualRoot = null; @@ -453,8 +453,7 @@ namespace Avalonia return; } - Logger.Log( - LogEventLevel.Warning, + Logger.TryGet(LogEventLevel.Warning)?.Log( LogArea.Binding, this, "Error in binding to {Target}.{Property}: {Message}", diff --git a/src/Avalonia.X11/Glx/GlxPlatformFeature.cs b/src/Avalonia.X11/Glx/GlxPlatformFeature.cs index d15b5fe4b8..3dc2e8e41f 100644 --- a/src/Avalonia.X11/Glx/GlxPlatformFeature.cs +++ b/src/Avalonia.X11/Glx/GlxPlatformFeature.cs @@ -36,7 +36,7 @@ namespace Avalonia.X11.Glx } catch(Exception e) { - Logger.Error("OpenGL", null, "Unable to initialize GLX-based rendering: {0}", e); + Logger.TryGet(LogEventLevel.Error)?.Log("OpenGL", null, "Unable to initialize GLX-based rendering: {0}", e); return null; } } diff --git a/src/Markup/Avalonia.Markup.Xaml/Converters/AvaloniaPropertyTypeConverter.cs b/src/Markup/Avalonia.Markup.Xaml/Converters/AvaloniaPropertyTypeConverter.cs index b42bd53619..6000b71f9d 100644 --- a/src/Markup/Avalonia.Markup.Xaml/Converters/AvaloniaPropertyTypeConverter.cs +++ b/src/Markup/Avalonia.Markup.Xaml/Converters/AvaloniaPropertyTypeConverter.cs @@ -42,7 +42,7 @@ namespace Avalonia.Markup.Xaml.Converters !property.IsAttached && !registry.IsRegistered(targetType, property)) { - Logger.Warning( + Logger.TryGet(LogEventLevel.Warning)?.Log( LogArea.Property, this, "Property '{Owner}.{Name}' is not registered on '{Type}'.", diff --git a/src/Markup/Avalonia.Markup/Markup/Data/DelayedBinding.cs b/src/Markup/Avalonia.Markup/Markup/Data/DelayedBinding.cs index e03427c161..f7d228609d 100644 --- a/src/Markup/Avalonia.Markup/Markup/Data/DelayedBinding.cs +++ b/src/Markup/Avalonia.Markup/Markup/Data/DelayedBinding.cs @@ -150,7 +150,7 @@ namespace Avalonia.Markup.Data } catch (Exception e) { - Logger.Error( + Logger.TryGet(LogEventLevel.Error)?.Log( LogArea.Property, control, "Error setting {Property} on {Target}: {Exception}", diff --git a/src/Windows/Avalonia.Direct2D1/Media/StreamGeometryContextImpl.cs b/src/Windows/Avalonia.Direct2D1/Media/StreamGeometryContextImpl.cs index 10b89d79b8..fd2f14fc58 100644 --- a/src/Windows/Avalonia.Direct2D1/Media/StreamGeometryContextImpl.cs +++ b/src/Windows/Avalonia.Direct2D1/Media/StreamGeometryContextImpl.cs @@ -85,7 +85,7 @@ namespace Avalonia.Direct2D1.Media } catch (Exception ex) { - Logger.Error( + Logger.TryGet(LogEventLevel.Error)?.Log( LogArea.Visual, this, "GeometrySink.Close exception: {Exception}", From b7efe2037c5ac3cb9976e95d36b2285ec95d0306 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 8 Oct 2019 00:35:20 +0200 Subject: [PATCH 6/9] Only set SizeToContent for non-set width/height. Always setting `SizeToContext = WidthAndHeight` was causing `Grid` to default to `Auto` sizing for `Star` rows/columns at design-time. Fixes #2862 --- src/Avalonia.DesignerSupport/DesignWindowLoader.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.DesignerSupport/DesignWindowLoader.cs b/src/Avalonia.DesignerSupport/DesignWindowLoader.cs index a7d4b96974..f3bb0edce5 100644 --- a/src/Avalonia.DesignerSupport/DesignWindowLoader.cs +++ b/src/Avalonia.DesignerSupport/DesignWindowLoader.cs @@ -69,7 +69,17 @@ namespace Avalonia.DesignerSupport } if (!window.IsSet(Window.SizeToContentProperty)) - window.SizeToContent = SizeToContent.WidthAndHeight; + { + if (double.IsNaN(window.Width)) + { + window.SizeToContent |= SizeToContent.Width; + } + + if (double.IsNaN(window.Height)) + { + window.SizeToContent |= SizeToContent.Height; + } + } } window.Show(); Design.ApplyDesignModeProperties(window, control); From d2dbc12ee8540aeb80d6e914617bfe99c14d2ce1 Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Tue, 8 Oct 2019 00:49:23 +0200 Subject: [PATCH 7/9] Final review fixes. --- src/Avalonia.Base/AvaloniaObject.cs | 36 +++++++++++------------------ 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 1d49401d99..2450f1a3a1 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -333,17 +333,12 @@ namespace Avalonia throw new ArgumentException($"The property {property.Name} is readonly."); } - if (Logger.TryGet(LogEventLevel.Verbose, out var logger)) - { - var description = GetDescription(source); - - logger.Log( - LogArea.Property, - this, - "Bound {Property} to {Binding} with priority LocalValue", - property, - description); - } + Logger.TryGet(LogEventLevel.Verbose)?.Log( + LogArea.Property, + this, + "Bound {Property} to {Binding} with priority LocalValue", + property, + GetDescription(source)); if (_directBindings == null) { @@ -354,18 +349,13 @@ namespace Avalonia } else { - if (Logger.TryGet(LogEventLevel.Verbose, out var logger)) - { - var description = GetDescription(source); - - logger.Log( - LogArea.Property, - this, - "Bound {Property} to {Binding} with priority {Priority}", - property, - description, - priority); - } + Logger.TryGet(LogEventLevel.Verbose)?.Log( + LogArea.Property, + this, + "Bound {Property} to {Binding} with priority {Priority}", + property, + GetDescription(source), + priority); return Values.AddBinding(property, source, priority); } From cb663f98b18f638ec63abaa5226cc79a6c085718 Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Sun, 6 Oct 2019 19:03:05 +0200 Subject: [PATCH 8/9] Add missing IEquatable interfaces to structs. Standardize parsing error messages. Fix two instances of boxing in hash code. --- src/Avalonia.Visuals/CornerRadius.cs | 68 +++++++++++++++---- src/Avalonia.Visuals/Matrix.cs | 6 +- src/Avalonia.Visuals/Media/PixelPoint.cs | 20 ++++-- src/Avalonia.Visuals/Media/PixelRect.cs | 18 +++-- src/Avalonia.Visuals/Media/PixelSize.cs | 18 +++-- src/Avalonia.Visuals/Point.cs | 27 ++++++-- src/Avalonia.Visuals/Rect.cs | 38 ++++++++--- src/Avalonia.Visuals/RelativePoint.cs | 7 +- src/Avalonia.Visuals/RelativeRect.cs | 9 +-- src/Avalonia.Visuals/Size.cs | 26 +++++-- src/Avalonia.Visuals/Thickness.cs | 34 +++++++--- src/Avalonia.Visuals/Vector.cs | 2 +- .../VisualTree/TransformedBounds.cs | 3 +- 13 files changed, 200 insertions(+), 76 deletions(-) diff --git a/src/Avalonia.Visuals/CornerRadius.cs b/src/Avalonia.Visuals/CornerRadius.cs index 09163a2dac..5445c5a200 100644 --- a/src/Avalonia.Visuals/CornerRadius.cs +++ b/src/Avalonia.Visuals/CornerRadius.cs @@ -8,7 +8,10 @@ using Avalonia.Utilities; namespace Avalonia { - public struct CornerRadius + /// + /// Represents the radii of a rectangle's corners. + /// + public readonly struct CornerRadius : IEquatable { static CornerRadius() { @@ -33,20 +36,60 @@ namespace Avalonia BottomLeft = bottomLeft; } + /// + /// Radius of the top left corner. + /// public double TopLeft { get; } + + /// + /// Radius of the top right corner. + /// public double TopRight { get; } + + /// + /// Radius of the bottom right corner. + /// public double BottomRight { get; } + + /// + /// Radius of the bottom left corner. + /// public double BottomLeft { get; } + + /// + /// Gets a value indicating whether all corner radii are set to 0. + /// public bool IsEmpty => TopLeft.Equals(0) && IsUniform; + + /// + /// Gets a value indicating whether all corner radii are equal. + /// public bool IsUniform => TopLeft.Equals(TopRight) && BottomLeft.Equals(BottomRight) && TopRight.Equals(BottomRight); + /// + /// Returns a boolean indicating whether the corner radius is equal to the other given corner radius. + /// + /// The other corner radius to test equality against. + /// True if this corner radius is equal to other; False otherwise. + public bool Equals(CornerRadius other) + { + // ReSharper disable CompareOfFloatsByEqualityOperator + return TopLeft == other.TopLeft && + + TopRight == other.TopRight && + BottomRight == other.BottomRight && + BottomLeft == other.BottomLeft; + // ReSharper restore CompareOfFloatsByEqualityOperator + } + public override bool Equals(object obj) { - if (obj is CornerRadius) + if (!(obj is CornerRadius)) { - return this == (CornerRadius)obj; + return false; } - return false; + + return Equals((CornerRadius)obj); } public override int GetHashCode() @@ -61,7 +104,9 @@ namespace Avalonia public static CornerRadius Parse(string s) { - using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid Thickness")) + const string exceptionMessage = "Invalid CornerRadius."; + + using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage)) { if (tokenizer.TryReadDouble(out var a)) { @@ -78,21 +123,18 @@ namespace Avalonia return new CornerRadius(a); } - throw new FormatException("Invalid CornerRadius."); + throw new FormatException(exceptionMessage); } } - public static bool operator ==(CornerRadius cr1, CornerRadius cr2) + public static bool operator ==(CornerRadius left, CornerRadius right) { - return cr1.TopLeft.Equals(cr2.TopLeft) - && cr1.TopRight.Equals(cr2.TopRight) - && cr1.BottomRight.Equals(cr2.BottomRight) - && cr1.BottomLeft.Equals(cr2.BottomLeft); + return left.Equals(right); } - public static bool operator !=(CornerRadius cr1, CornerRadius cr2) + public static bool operator !=(CornerRadius left, CornerRadius right) { - return !(cr1 == cr2); + return !(left == right); } } } diff --git a/src/Avalonia.Visuals/Matrix.cs b/src/Avalonia.Visuals/Matrix.cs index d083a2aaf8..6f9839b6a1 100644 --- a/src/Avalonia.Visuals/Matrix.cs +++ b/src/Avalonia.Visuals/Matrix.cs @@ -10,7 +10,7 @@ namespace Avalonia /// /// A 2x3 matrix. /// - public readonly struct Matrix + public readonly struct Matrix : IEquatable { private readonly double _m11; private readonly double _m12; @@ -235,12 +235,14 @@ namespace Avalonia /// True if this matrix is equal to other; False otherwise. public bool Equals(Matrix other) { + // ReSharper disable CompareOfFloatsByEqualityOperator return _m11 == other.M11 && _m12 == other.M12 && _m21 == other.M21 && _m22 == other.M22 && _m31 == other.M31 && _m32 == other.M32; + // ReSharper restore CompareOfFloatsByEqualityOperator } /// @@ -316,7 +318,7 @@ namespace Avalonia /// The . public static Matrix Parse(string s) { - using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid Matrix")) + using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid Matrix.")) { return new Matrix( tokenizer.ReadDouble(), diff --git a/src/Avalonia.Visuals/Media/PixelPoint.cs b/src/Avalonia.Visuals/Media/PixelPoint.cs index d62c2a2e55..5a329d0238 100644 --- a/src/Avalonia.Visuals/Media/PixelPoint.cs +++ b/src/Avalonia.Visuals/Media/PixelPoint.cs @@ -10,7 +10,7 @@ namespace Avalonia /// /// Represents a point in device pixels. /// - public readonly struct PixelPoint + public readonly struct PixelPoint : IEquatable { /// /// A point representing 0,0. @@ -46,7 +46,7 @@ namespace Avalonia /// True if the points are equal; otherwise false. public static bool operator ==(PixelPoint left, PixelPoint right) { - return left.X == right.X && left.Y == right.Y; + return left.Equals(right); } /// @@ -120,7 +120,7 @@ namespace Avalonia /// The . public static PixelPoint Parse(string s) { - using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid PixelPoint")) + using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid PixelPoint.")) { return new PixelPoint( tokenizer.ReadInt32(), @@ -128,6 +128,18 @@ namespace Avalonia } } + /// + /// Returns a boolean indicating whether the point is equal to the other given point. + /// + /// The other point to test equality against. + /// True if this point is equal to other; False otherwise. + public bool Equals(PixelPoint other) + { + // ReSharper disable CompareOfFloatsByEqualityOperator + return X == other.X && Y == other.Y; + // ReSharper restore CompareOfFloatsByEqualityOperator + } + /// /// Checks for equality between a point and an object. /// @@ -139,7 +151,7 @@ namespace Avalonia { if (obj is PixelPoint other) { - return this == other; + return Equals(other); } return false; diff --git a/src/Avalonia.Visuals/Media/PixelRect.cs b/src/Avalonia.Visuals/Media/PixelRect.cs index 0e2094da07..b830f4b4b4 100644 --- a/src/Avalonia.Visuals/Media/PixelRect.cs +++ b/src/Avalonia.Visuals/Media/PixelRect.cs @@ -10,7 +10,7 @@ namespace Avalonia /// /// Represents a rectangle in device pixels. /// - public readonly struct PixelRect + public readonly struct PixelRect : IEquatable { /// /// An empty rectangle. @@ -148,7 +148,7 @@ namespace Avalonia /// True if the rects are equal; otherwise false. public static bool operator ==(PixelRect left, PixelRect right) { - return left.Position == right.Position && left.Size == right.Size; + return left.Equals(right); } /// @@ -196,6 +196,16 @@ namespace Avalonia rect.Height); } + /// + /// Returns a boolean indicating whether the rect is equal to the other given rect. + /// + /// The other rect to test equality against. + /// True if this rect is equal to other; False otherwise. + public bool Equals(PixelRect other) + { + return Position == other.Position && Size == other.Size; + } + /// /// Returns a boolean indicating whether the given object is equal to this rectangle. /// @@ -205,7 +215,7 @@ namespace Avalonia { if (obj is PixelRect other) { - return this == other; + return Equals(other); } return false; @@ -432,7 +442,7 @@ namespace Avalonia /// The parsed . public static PixelRect Parse(string s) { - using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid PixelRect")) + using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid PixelRect.")) { return new PixelRect( tokenizer.ReadInt32(), diff --git a/src/Avalonia.Visuals/Media/PixelSize.cs b/src/Avalonia.Visuals/Media/PixelSize.cs index b903b804f9..e2d6b46225 100644 --- a/src/Avalonia.Visuals/Media/PixelSize.cs +++ b/src/Avalonia.Visuals/Media/PixelSize.cs @@ -10,7 +10,7 @@ namespace Avalonia /// /// Represents a size in device pixels. /// - public readonly struct PixelSize + public readonly struct PixelSize : IEquatable { /// /// A size representing zero @@ -51,7 +51,7 @@ namespace Avalonia /// True if the sizes are equal; otherwise false. public static bool operator ==(PixelSize left, PixelSize right) { - return left.Width == right.Width && left.Height == right.Height; + return left.Equals(right); } /// @@ -72,7 +72,7 @@ namespace Avalonia /// The . public static PixelSize Parse(string s) { - using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid PixelSize")) + using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid PixelSize.")) { return new PixelSize( tokenizer.ReadInt32(), @@ -80,6 +80,16 @@ namespace Avalonia } } + /// + /// Returns a boolean indicating whether the size is equal to the other given size. + /// + /// The other size to test equality against. + /// True if this size is equal to other; False otherwise. + public bool Equals(PixelSize other) + { + return Width == other.Width && Height == other.Height; + } + /// /// Checks for equality between a size and an object. /// @@ -91,7 +101,7 @@ namespace Avalonia { if (obj is PixelSize other) { - return this == other; + return Equals(other); } return false; diff --git a/src/Avalonia.Visuals/Point.cs b/src/Avalonia.Visuals/Point.cs index 0d3e354615..0d4dcb7c73 100644 --- a/src/Avalonia.Visuals/Point.cs +++ b/src/Avalonia.Visuals/Point.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.Globalization; using Avalonia.Animation.Animators; using Avalonia.Utilities; @@ -10,7 +11,7 @@ namespace Avalonia /// /// Defines a point. /// - public readonly struct Point + public readonly struct Point : IEquatable { static Point() { @@ -75,7 +76,7 @@ namespace Avalonia /// True if the points are equal; otherwise false. public static bool operator ==(Point left, Point right) { - return left.X == right.X && left.Y == right.Y; + return left.Equals(right); } /// @@ -177,7 +178,7 @@ namespace Avalonia /// The . public static Point Parse(string s) { - using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid Point")) + using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid Point.")) { return new Point( tokenizer.ReadDouble(), @@ -186,6 +187,19 @@ namespace Avalonia } } + /// + /// Returns a boolean indicating whether the point is equal to the other given point. + /// + /// The other point to test equality against. + /// True if this point is equal to other; False otherwise. + public bool Equals(Point other) + { + // ReSharper disable CompareOfFloatsByEqualityOperator + return _x == other._x && + _y == other._y; + // ReSharper enable CompareOfFloatsByEqualityOperator + } + /// /// Checks for equality between a point and an object. /// @@ -195,13 +209,12 @@ namespace Avalonia /// public override bool Equals(object obj) { - if (obj is Point) + if (!(obj is Point)) { - var other = (Point)obj; - return X == other.X && Y == other.Y; + return false; } - return false; + return Equals((Point)obj); } /// diff --git a/src/Avalonia.Visuals/Rect.cs b/src/Avalonia.Visuals/Rect.cs index 8f08f7f51f..94fd2afb3d 100644 --- a/src/Avalonia.Visuals/Rect.cs +++ b/src/Avalonia.Visuals/Rect.cs @@ -11,7 +11,7 @@ namespace Avalonia /// /// Defines a rectangle. /// - public readonly struct Rect + public readonly struct Rect : IEquatable { static Rect() { @@ -164,7 +164,9 @@ namespace Avalonia /// /// Gets a value that indicates whether the rectangle is empty. /// + // ReSharper disable CompareOfFloatsByEqualityOperator public bool IsEmpty => _width == 0 && _height == 0; + // ReSharper restore CompareOfFloatsByEqualityOperator /// /// Checks for equality between two s. @@ -174,7 +176,7 @@ namespace Avalonia /// True if the rects are equal; otherwise false. public static bool operator ==(Rect left, Rect right) { - return left.Position == right.Position && left.Size == right.Size; + return left.Equals(right); } /// @@ -297,6 +299,21 @@ namespace Avalonia Size.Deflate(thickness)); } + /// + /// Returns a boolean indicating whether the rect is equal to the other given rect. + /// + /// The other rect to test equality against. + /// True if this rect is equal to other; False otherwise. + public bool Equals(Rect other) + { + // ReSharper disable CompareOfFloatsByEqualityOperator + return _x == other._x && + _y == other._y && + _width == other._width && + _height == other._height; + // ReSharper enable CompareOfFloatsByEqualityOperator + } + /// /// Returns a boolean indicating whether the given object is equal to this rectangle. /// @@ -304,13 +321,12 @@ namespace Avalonia /// True if the object is equal to this rectangle; false otherwise. public override bool Equals(object obj) { - if (obj is Rect) + if (!(obj is Rect)) { - var other = (Rect)obj; - return Position == other.Position && Size == other.Size; + return false; } - return false; + return Equals((Rect)obj); } /// @@ -422,10 +438,10 @@ namespace Avalonia } else { - var x1 = Math.Min(this.X, rect.X); - var x2 = Math.Max(this.Right, rect.Right); - var y1 = Math.Min(this.Y, rect.Y); - var y2 = Math.Max(this.Bottom, rect.Bottom); + var x1 = Math.Min(X, rect.X); + var x2 = Math.Max(Right, rect.Right); + var y1 = Math.Min(Y, rect.Y); + var y2 = Math.Max(Bottom, rect.Bottom); return new Rect(new Point(x1, y1), new Point(x2, y2)); } @@ -493,7 +509,7 @@ namespace Avalonia /// The parsed . public static Rect Parse(string s) { - using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid Rect")) + using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid Rect.")) { return new Rect( tokenizer.ReadDouble(), diff --git a/src/Avalonia.Visuals/RelativePoint.cs b/src/Avalonia.Visuals/RelativePoint.cs index d38bb1d496..c822486767 100644 --- a/src/Avalonia.Visuals/RelativePoint.cs +++ b/src/Avalonia.Visuals/RelativePoint.cs @@ -130,10 +130,7 @@ namespace Avalonia { unchecked { - int hash = 17; - hash = (hash * 23) + Unit.GetHashCode(); - hash = (hash * 23) + Point.GetHashCode(); - return hash; + return (_point.GetHashCode() * 397) ^ (int)_unit; } } @@ -156,7 +153,7 @@ namespace Avalonia /// The parsed . public static RelativePoint Parse(string s) { - using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid RelativePoint")) + using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid RelativePoint.")) { var x = tokenizer.ReadString(); var y = tokenizer.ReadString(); diff --git a/src/Avalonia.Visuals/RelativeRect.cs b/src/Avalonia.Visuals/RelativeRect.cs index 927ec3ef75..c69dab3c42 100644 --- a/src/Avalonia.Visuals/RelativeRect.cs +++ b/src/Avalonia.Visuals/RelativeRect.cs @@ -139,10 +139,7 @@ namespace Avalonia { unchecked { - int hash = 17; - hash = (hash * 23) + Unit.GetHashCode(); - hash = (hash * 23) + Rect.GetHashCode(); - return hash; + return ((int)Unit * 397) ^ Rect.GetHashCode(); } } @@ -161,7 +158,7 @@ namespace Avalonia Rect.Width * size.Width, Rect.Height * size.Height); } - + /// /// Parses a string. /// @@ -169,7 +166,7 @@ namespace Avalonia /// The parsed . public static RelativeRect Parse(string s) { - using (var tokenizer = new StringTokenizer(s, exceptionMessage: "Invalid RelativeRect")) + using (var tokenizer = new StringTokenizer(s, exceptionMessage: "Invalid RelativeRect.")) { var x = tokenizer.ReadString(); var y = tokenizer.ReadString(); diff --git a/src/Avalonia.Visuals/Size.cs b/src/Avalonia.Visuals/Size.cs index 782c5ea67b..9d524d6fa7 100644 --- a/src/Avalonia.Visuals/Size.cs +++ b/src/Avalonia.Visuals/Size.cs @@ -11,7 +11,7 @@ namespace Avalonia /// /// Defines a size. /// - public readonly struct Size + public readonly struct Size : IEquatable { static Size() { @@ -72,7 +72,7 @@ namespace Avalonia /// True if the sizes are equal; otherwise false. public static bool operator ==(Size left, Size right) { - return left._width == right._width && left._height == right._height; + return left.Equals(right); } /// @@ -158,7 +158,7 @@ namespace Avalonia /// The . public static Size Parse(string s) { - using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid Size")) + using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid Size.")) { return new Size( tokenizer.ReadDouble(), @@ -191,6 +191,19 @@ namespace Avalonia Math.Max(0, _height - thickness.Top - thickness.Bottom)); } + /// + /// Returns a boolean indicating whether the size is equal to the other given size. + /// + /// The other size to test equality against. + /// True if this size is equal to other; False otherwise. + public bool Equals(Size other) + { + // ReSharper disable CompareOfFloatsByEqualityOperator + return _width == other._width && + _height == other._height; + // ReSharper enable CompareOfFloatsByEqualityOperator + } + /// /// Checks for equality between a size and an object. /// @@ -200,13 +213,12 @@ namespace Avalonia /// public override bool Equals(object obj) { - if (obj is Size) + if (!(obj is Size)) { - var other = (Size)obj; - return Width == other.Width && Height == other.Height; + return false; } - return false; + return Equals((Size)obj); } /// diff --git a/src/Avalonia.Visuals/Thickness.cs b/src/Avalonia.Visuals/Thickness.cs index 830ee4666e..d0fc63e254 100644 --- a/src/Avalonia.Visuals/Thickness.cs +++ b/src/Avalonia.Visuals/Thickness.cs @@ -3,7 +3,6 @@ using System; using System.Globalization; -using Avalonia.Animation; using Avalonia.Animation.Animators; using Avalonia.Utilities; @@ -12,7 +11,7 @@ namespace Avalonia /// /// Describes the thickness of a frame around a rectangle. /// - public readonly struct Thickness + public readonly struct Thickness : IEquatable { static Thickness() { @@ -204,7 +203,9 @@ namespace Avalonia /// The . public static Thickness Parse(string s) { - using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage: "Invalid Thickness")) + const string exceptionMessage = "Invalid Thickness."; + + using (var tokenizer = new StringTokenizer(s, CultureInfo.InvariantCulture, exceptionMessage)) { if (tokenizer.TryReadDouble(out var a)) { @@ -221,10 +222,25 @@ namespace Avalonia return new Thickness(a); } - throw new FormatException("Invalid Thickness."); + throw new FormatException(exceptionMessage); } } + /// + /// Returns a boolean indicating whether the thickness is equal to the other given point. + /// + /// The other thickness to test equality against. + /// True if this thickness is equal to other; False otherwise. + public bool Equals(Thickness other) + { + // ReSharper disable CompareOfFloatsByEqualityOperator + return _left == other._left && + _top == other._top && + _right == other._right && + _bottom == other._bottom; + // ReSharper restore CompareOfFloatsByEqualityOperator + } + /// /// Checks for equality between a thickness and an object. /// @@ -234,16 +250,12 @@ namespace Avalonia /// public override bool Equals(object obj) { - if (obj is Thickness) + if (!(obj is Thickness)) { - Thickness other = (Thickness)obj; - return Left == other.Left && - Top == other.Top && - Right == other.Right && - Bottom == other.Bottom; + return false; } - return false; + return Equals((Thickness)obj); } /// diff --git a/src/Avalonia.Visuals/Vector.cs b/src/Avalonia.Visuals/Vector.cs index 11bda8b00e..bd2dfdc828 100644 --- a/src/Avalonia.Visuals/Vector.cs +++ b/src/Avalonia.Visuals/Vector.cs @@ -11,7 +11,7 @@ namespace Avalonia /// /// Defines a vector. /// - public readonly struct Vector + public readonly struct Vector : IEquatable { static Vector() { diff --git a/src/Avalonia.Visuals/VisualTree/TransformedBounds.cs b/src/Avalonia.Visuals/VisualTree/TransformedBounds.cs index 39b328adc2..5fb22680dc 100644 --- a/src/Avalonia.Visuals/VisualTree/TransformedBounds.cs +++ b/src/Avalonia.Visuals/VisualTree/TransformedBounds.cs @@ -1,13 +1,14 @@ // 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; namespace Avalonia.VisualTree { /// /// Holds information about the bounds of a control, together with a transform and a clip. /// - public readonly struct TransformedBounds + public readonly struct TransformedBounds : IEquatable { /// /// Initializes a new instance of the struct. From 58af05644088e9590dc03d40f27abf6aa207d4fd Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Tue, 8 Oct 2019 01:01:41 +0200 Subject: [PATCH 9/9] Streamline object.Equals implementation. --- src/Avalonia.Visuals/CornerRadius.cs | 15 ++++++--------- src/Avalonia.Visuals/Matrix.cs | 10 +--------- src/Avalonia.Visuals/Media/PixelPoint.cs | 10 +--------- src/Avalonia.Visuals/Media/PixelRect.cs | 10 +--------- src/Avalonia.Visuals/Media/PixelSize.cs | 10 +--------- src/Avalonia.Visuals/Point.cs | 10 +--------- src/Avalonia.Visuals/Rect.cs | 10 +--------- src/Avalonia.Visuals/RelativePoint.cs | 5 +---- src/Avalonia.Visuals/RelativeRect.cs | 5 +---- src/Avalonia.Visuals/Size.cs | 10 +--------- src/Avalonia.Visuals/Thickness.cs | 10 +--------- src/Avalonia.Visuals/Vector.cs | 9 +-------- .../VisualTree/TransformedBounds.cs | 10 +--------- 13 files changed, 18 insertions(+), 106 deletions(-) diff --git a/src/Avalonia.Visuals/CornerRadius.cs b/src/Avalonia.Visuals/CornerRadius.cs index 5445c5a200..ecb9e75d82 100644 --- a/src/Avalonia.Visuals/CornerRadius.cs +++ b/src/Avalonia.Visuals/CornerRadius.cs @@ -82,15 +82,12 @@ namespace Avalonia // ReSharper restore CompareOfFloatsByEqualityOperator } - public override bool Equals(object obj) - { - if (!(obj is CornerRadius)) - { - return false; - } - - return Equals((CornerRadius)obj); - } + /// + /// Returns a boolean indicating whether the given Object is equal to this corner radius instance. + /// + /// The Object to compare against. + /// True if the Object is equal to this corner radius; False otherwise. + public override bool Equals(object obj) => obj is CornerRadius other && Equals(other); public override int GetHashCode() { diff --git a/src/Avalonia.Visuals/Matrix.cs b/src/Avalonia.Visuals/Matrix.cs index 6f9839b6a1..92b7dae904 100644 --- a/src/Avalonia.Visuals/Matrix.cs +++ b/src/Avalonia.Visuals/Matrix.cs @@ -250,15 +250,7 @@ namespace Avalonia /// /// The Object to compare against. /// True if the Object is equal to this matrix; False otherwise. - public override bool Equals(object obj) - { - if (!(obj is Matrix)) - { - return false; - } - - return Equals((Matrix)obj); - } + public override bool Equals(object obj) => obj is Matrix other && Equals(other); /// /// Returns the hash code for this instance. diff --git a/src/Avalonia.Visuals/Media/PixelPoint.cs b/src/Avalonia.Visuals/Media/PixelPoint.cs index 5a329d0238..5384a40be9 100644 --- a/src/Avalonia.Visuals/Media/PixelPoint.cs +++ b/src/Avalonia.Visuals/Media/PixelPoint.cs @@ -147,15 +147,7 @@ namespace Avalonia /// /// True if is a point that equals the current point. /// - public override bool Equals(object obj) - { - if (obj is PixelPoint other) - { - return Equals(other); - } - - return false; - } + public override bool Equals(object obj) => obj is PixelPoint other && Equals(other); /// /// Returns a hash code for a . diff --git a/src/Avalonia.Visuals/Media/PixelRect.cs b/src/Avalonia.Visuals/Media/PixelRect.cs index b830f4b4b4..a024e1af11 100644 --- a/src/Avalonia.Visuals/Media/PixelRect.cs +++ b/src/Avalonia.Visuals/Media/PixelRect.cs @@ -211,15 +211,7 @@ namespace Avalonia /// /// The object to compare against. /// True if the object is equal to this rectangle; false otherwise. - public override bool Equals(object obj) - { - if (obj is PixelRect other) - { - return Equals(other); - } - - return false; - } + public override bool Equals(object obj) => obj is PixelRect other && Equals(other); /// /// Returns the hash code for this instance. diff --git a/src/Avalonia.Visuals/Media/PixelSize.cs b/src/Avalonia.Visuals/Media/PixelSize.cs index e2d6b46225..a49aa82c0d 100644 --- a/src/Avalonia.Visuals/Media/PixelSize.cs +++ b/src/Avalonia.Visuals/Media/PixelSize.cs @@ -97,15 +97,7 @@ namespace Avalonia /// /// True if is a size that equals the current size. /// - public override bool Equals(object obj) - { - if (obj is PixelSize other) - { - return Equals(other); - } - - return false; - } + public override bool Equals(object obj) => obj is PixelSize other && Equals(other); /// /// Returns a hash code for a . diff --git a/src/Avalonia.Visuals/Point.cs b/src/Avalonia.Visuals/Point.cs index 0d4dcb7c73..d92f8b0fc4 100644 --- a/src/Avalonia.Visuals/Point.cs +++ b/src/Avalonia.Visuals/Point.cs @@ -207,15 +207,7 @@ namespace Avalonia /// /// True if is a point that equals the current point. /// - public override bool Equals(object obj) - { - if (!(obj is Point)) - { - return false; - } - - return Equals((Point)obj); - } + public override bool Equals(object obj) => obj is Point other && Equals(other); /// /// Returns a hash code for a . diff --git a/src/Avalonia.Visuals/Rect.cs b/src/Avalonia.Visuals/Rect.cs index 94fd2afb3d..4dfd641525 100644 --- a/src/Avalonia.Visuals/Rect.cs +++ b/src/Avalonia.Visuals/Rect.cs @@ -319,15 +319,7 @@ namespace Avalonia /// /// The object to compare against. /// True if the object is equal to this rectangle; false otherwise. - public override bool Equals(object obj) - { - if (!(obj is Rect)) - { - return false; - } - - return Equals((Rect)obj); - } + public override bool Equals(object obj) => obj is Rect other && Equals(other); /// /// Returns the hash code for this instance. diff --git a/src/Avalonia.Visuals/RelativePoint.cs b/src/Avalonia.Visuals/RelativePoint.cs index c822486767..2e8fb16bc1 100644 --- a/src/Avalonia.Visuals/RelativePoint.cs +++ b/src/Avalonia.Visuals/RelativePoint.cs @@ -107,10 +107,7 @@ namespace Avalonia /// /// The other object. /// True if the objects are equal, otherwise false. - public override bool Equals(object obj) - { - return (obj is RelativePoint) && Equals((RelativePoint)obj); - } + public override bool Equals(object obj) => obj is RelativePoint other && Equals(other); /// /// Checks if the equals another point. diff --git a/src/Avalonia.Visuals/RelativeRect.cs b/src/Avalonia.Visuals/RelativeRect.cs index c69dab3c42..d2e4b2dc26 100644 --- a/src/Avalonia.Visuals/RelativeRect.cs +++ b/src/Avalonia.Visuals/RelativeRect.cs @@ -116,10 +116,7 @@ namespace Avalonia /// /// The other object. /// True if the objects are equal, otherwise false. - public override bool Equals(object obj) - { - return (obj is RelativeRect) && Equals((RelativeRect)obj); - } + public override bool Equals(object obj) => obj is RelativeRect other && Equals(other); /// /// Checks if the equals another rectangle. diff --git a/src/Avalonia.Visuals/Size.cs b/src/Avalonia.Visuals/Size.cs index 9d524d6fa7..aba2ed8d62 100644 --- a/src/Avalonia.Visuals/Size.cs +++ b/src/Avalonia.Visuals/Size.cs @@ -211,15 +211,7 @@ namespace Avalonia /// /// True if is a size that equals the current size. /// - public override bool Equals(object obj) - { - if (!(obj is Size)) - { - return false; - } - - return Equals((Size)obj); - } + public override bool Equals(object obj) => obj is Size other && Equals(other); /// /// Returns a hash code for a . diff --git a/src/Avalonia.Visuals/Thickness.cs b/src/Avalonia.Visuals/Thickness.cs index d0fc63e254..44ff66069f 100644 --- a/src/Avalonia.Visuals/Thickness.cs +++ b/src/Avalonia.Visuals/Thickness.cs @@ -248,15 +248,7 @@ namespace Avalonia /// /// True if is a size that equals the current size. /// - public override bool Equals(object obj) - { - if (!(obj is Thickness)) - { - return false; - } - - return Equals((Thickness)obj); - } + public override bool Equals(object obj) => obj is Thickness other && Equals(other); /// /// Returns a hash code for a . diff --git a/src/Avalonia.Visuals/Vector.cs b/src/Avalonia.Visuals/Vector.cs index bd2dfdc828..576d2daaaa 100644 --- a/src/Avalonia.Visuals/Vector.cs +++ b/src/Avalonia.Visuals/Vector.cs @@ -138,7 +138,6 @@ namespace Avalonia /// /// The other vector. /// True if vectors are nearly equal. - [Pure] public bool NearlyEquals(Vector other) { const float tolerance = float.Epsilon; @@ -146,13 +145,7 @@ namespace Avalonia return Math.Abs(_x - other._x) < tolerance && Math.Abs(_y - other._y) < tolerance; } - public override bool Equals(object obj) - { - if (ReferenceEquals(null, obj)) - return false; - - return obj is Vector vector && Equals(vector); - } + public override bool Equals(object obj) => obj is Vector other && Equals(other); public override int GetHashCode() { diff --git a/src/Avalonia.Visuals/VisualTree/TransformedBounds.cs b/src/Avalonia.Visuals/VisualTree/TransformedBounds.cs index 5fb22680dc..b2121aa8da 100644 --- a/src/Avalonia.Visuals/VisualTree/TransformedBounds.cs +++ b/src/Avalonia.Visuals/VisualTree/TransformedBounds.cs @@ -57,15 +57,7 @@ namespace Avalonia.VisualTree return Bounds == other.Bounds && Clip == other.Clip && Transform == other.Transform; } - public override bool Equals(object obj) - { - if (obj is null) - { - return false; - } - - return obj is TransformedBounds other && Equals(other); - } + public override bool Equals(object obj) => obj is TransformedBounds other && Equals(other); public override int GetHashCode() {