From 679b4bff3306cfd7c52f89a3bf3d0287a64ae188 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Fri, 22 Nov 2024 16:11:05 -0500 Subject: [PATCH] perf(brush): Don't use reflection to invoke brush updates --- .../Helpers/WeakEvents/WeakEventManager.cs | 142 ------------------ src/Uno.UI/UI/Xaml/Controls/Border/Border.cs | 5 +- .../Border/BorderLayerRenderer.Android.cs | 7 +- .../Border/BorderLayerRenderer.iOSmacOS.cs | 18 +-- .../Border/BorderLayerRenderer.wasm.cs | 10 +- .../UI/Xaml/Controls/TextBlock/TextBlock.cs | 5 +- .../Xaml/Controls/TextBlock/TextBlock.skia.cs | 5 +- .../TextBox/MultilineTextBoxView.iOS.cs | 4 +- .../TextBox/SinglelineTextBoxView.iOS.cs | 4 +- .../UI/Xaml/Controls/TextBox/TextBox.cs | 9 +- .../Controls/TextBox/TextBoxView.Android.cs | 3 +- .../Controls/TextBox/TextBoxView.macOS.cs | 3 +- .../Xaml/Controls/TextBox/TextBoxView.wasm.cs | 4 +- ...IFrameworkElementImplementation.Android.tt | 6 +- .../IFrameworkElementImplementation.iOS.tt | 6 +- .../IFrameworkElementImplementation.macOS.tt | 5 +- src/Uno.UI/UI/Xaml/Media/Brush.cs | 36 +++-- src/Uno.UI/UI/Xaml/Shapes/Shape.cs | 8 +- src/Uno.UI/Uno.UI.netcoremobile.csproj | 11 +- 19 files changed, 93 insertions(+), 198 deletions(-) delete mode 100644 src/Uno.UI/Helpers/WeakEvents/WeakEventManager.cs diff --git a/src/Uno.UI/Helpers/WeakEvents/WeakEventManager.cs b/src/Uno.UI/Helpers/WeakEvents/WeakEventManager.cs deleted file mode 100644 index 0cd4d354cf4b..000000000000 --- a/src/Uno.UI/Helpers/WeakEvents/WeakEventManager.cs +++ /dev/null @@ -1,142 +0,0 @@ -#nullable enable - -using System; -using System.Buffers; -using System.Collections.Generic; -using System.Diagnostics; -using System.Reflection; -using System.Runtime.CompilerServices; - -// Mostly from https://github.com/dotnet/maui/blob/c92d44c68fe81c57ef8caec2506e6788309b8ff4/src/Core/src/WeakEventManager.cs -// But adjusted a little bit - -namespace Uno.UI.Helpers -{ - internal sealed class WeakEventManager - { - private readonly Dictionary> _eventHandlers = new(StringComparer.Ordinal); - - public void AddEventHandler(Action? handler, [CallerMemberName] string eventName = "") - { - if (string.IsNullOrEmpty(eventName)) - throw new ArgumentNullException(nameof(eventName)); - - if (handler == null) - throw new ArgumentNullException(nameof(handler)); - - AddEventHandler(eventName, handler.Target, handler.GetMethodInfo()); - } - - public void HandleEvent(string eventName) - { - if (_eventHandlers.TryGetValue(eventName, out List? target)) - { - // clone the target array just in case one of the subscriptions calls RemoveEventHandler - var targetClone = ArrayPool.Shared.Rent(target.Count); - target.CopyTo(targetClone, 0); - var count = target.Count; - for (int i = 0; i < count; i++) - { - Subscription subscription = targetClone[i]; - bool isStatic = subscription.Subscriber == null; - if (isStatic) - { - // For a static method, we'll just pass null as the first parameter of MethodInfo.Invoke - subscription.Handler.Invoke(null, null); - continue; - } - - object? subscriber = subscription.Subscriber?.Target; - - if (subscriber == null) - { - // The subscriber was collected, so there's no need to keep this subscription around - target.Remove(subscription); - } - else - { - subscription.Handler.Invoke(subscriber, null); - } - } - - ArrayPool.Shared.Return(targetClone); - } - } - - public void RemoveEventHandler(Action? handler, [CallerMemberName] string eventName = "") - { - if (string.IsNullOrEmpty(eventName)) - throw new ArgumentNullException(nameof(eventName)); - - if (handler == null) - throw new ArgumentNullException(nameof(handler)); - - RemoveEventHandler(eventName, handler.Target, handler.GetMethodInfo()); - } - - private void AddEventHandler(string eventName, object? handlerTarget, MethodInfo methodInfo) - { - if (!_eventHandlers.TryGetValue(eventName, out List? targets)) - { - targets = new List(); - _eventHandlers.Add(eventName, targets); - } - else - { - targets.RemoveAll(subscription => subscription.Subscriber is { IsAlive: false }); - } - - if (handlerTarget == null) - { - // This event handler is a static method - targets.Add(new Subscription(null, methodInfo)); - return; - } - - targets.Add(new Subscription(new WeakReference(handlerTarget), methodInfo)); - } - - private void RemoveEventHandler(string eventName, object? handlerTarget, MethodInfo methodInfo) - { - if (!_eventHandlers.TryGetValue(eventName, out List? subscriptions)) - return; - - for (int n = subscriptions.Count - 1; n >= 0; n--) - { - Subscription current = subscriptions[n]; - - if (current.Subscriber != null && !current.Subscriber.IsAlive) - { - // If not alive, remove and continue - subscriptions.RemoveAt(n); - continue; - } - - if (current.Subscriber?.Target == handlerTarget && current.Handler == methodInfo) - { - // Found the match, we can break - subscriptions.RemoveAt(n); - break; - } - } - } - - private readonly struct Subscription : IEquatable - { - public Subscription(WeakReference? subscriber, MethodInfo handler) - { - Subscriber = subscriber; - Handler = handler ?? throw new ArgumentNullException(nameof(handler)); - } - - public readonly WeakReference? Subscriber; - public readonly MethodInfo Handler; - - public bool Equals(Subscription other) => Subscriber == other.Subscriber && Handler == other.Handler; - - public override bool Equals(object? obj) => obj is Subscription other && Equals(other); - - public override int GetHashCode() => Subscriber?.GetHashCode() ?? 0 ^ Handler.GetHashCode(); - } - } -} diff --git a/src/Uno.UI/UI/Xaml/Controls/Border/Border.cs b/src/Uno.UI/UI/Xaml/Controls/Border/Border.cs index db368da9ffb4..a6b8ad9e339b 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Border/Border.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Border/Border.cs @@ -275,6 +275,7 @@ private void OnBorderThicknessChanged(Thickness oldValue, Thickness newValue) #if !__SKIA__ private Action _borderBrushChanged; + private IDisposable _brushChangedSubscription; #endif #if __ANDROID__ @@ -305,7 +306,9 @@ private void OnBorderBrushChanged(Brush oldValue, Brush newValue) #if __SKIA__ this.UpdateBorderBrush(); #else - Brush.SetupBrushChanged(oldValue, newValue, ref _borderBrushChanged, _borderBrushChanged ?? (() => + _brushChangedSubscription?.Dispose(); + + _brushChangedSubscription = Brush.SetupBrushChanged(newValue, ref _borderBrushChanged, _borderBrushChanged ?? (() => { UpdateBorder(); })); diff --git a/src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.Android.cs b/src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.Android.cs index c3ea30d794a3..2cce87d9f5f8 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.Android.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.Android.cs @@ -261,8 +261,11 @@ private IDisposable InnerCreateLayers( // because even though the brush instance is the same, there are additional properties // that BorderLayerState tracks on Android. This is not ideal and we should avoid it by refactoring // this file to handle brush changes on the same brush instance on its own instead. - Brush.SetupBrushChanged(_currentState.Background, background, ref _backgroundChanged, () => Update(true), false); - Brush.SetupBrushChanged(_currentState.BorderBrush, borderBrush, ref _borderChanged, () => Update(true), false); + _backgroundBrushChangedSubscription?.Dispose(); + _backgroundBrushChangedSubscription = Brush.SetupBrushChanged(background, ref _backgroundChanged, () => Update(true), false); + + _borderBrushChangedSubscription?.Dispose(); + _borderBrushChangedSubscription = Brush.SetupBrushChanged(borderBrush, ref _borderChanged, () => Update(true), false); return disposables; } diff --git a/src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.iOSmacOS.cs b/src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.iOSmacOS.cs index 117b7fd22928..e8c64e41d70f 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.iOSmacOS.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.iOSmacOS.cs @@ -228,8 +228,8 @@ void OnImageChanged(_Image _) { Action onInvalidateRender = () => backgroundLayer.FillColor = Brush.GetFallbackColor(background); onInvalidateRender(); - background.InvalidateRender += onInvalidateRender; - new DisposableAction(() => background.InvalidateRender -= onInvalidateRender).DisposeWith(disposables); + + background.RegisterInvalidateRender(onInvalidateRender).DisposeWith(disposables); } else { @@ -284,8 +284,7 @@ GradientBrush gradientBorder when gradientBorder.CanApplyToBorder(cornerRadius) else { onInvalidateRender(); - borderBrush.InvalidateRender += onInvalidateRender; - new DisposableAction(() => borderBrush.InvalidateRender -= onInvalidateRender).DisposeWith(disposables); + borderBrush.RegisterInvalidateRender(onInvalidateRender).DisposeWith(disposables); } } @@ -334,9 +333,7 @@ GradientBrush gradientBorder when gradientBorder.CanApplyToBorder(cornerRadius) Action onInvalidateRender = () => backgroundLayer.FillColor = Brush.GetFallbackColor(background); onInvalidateRender(); - background.InvalidateRender += onInvalidateRender; - new DisposableAction(() => background.InvalidateRender -= onInvalidateRender) - .DisposeWith(disposables); + background.RegisterInvalidateRender(onInvalidateRender).DisposeWith(disposables); // This is required because changing the CornerRadius changes the background drawing // implementation and we don't want a rectangular background behind a rounded background. @@ -362,10 +359,8 @@ imageData.NativeImage is _Image uiImage && else if (background is XamlCompositionBrushBase) { Action onInvalidateRender = () => backgroundLayer.FillColor = Brush.GetFallbackColor(background); - background.InvalidateRender += onInvalidateRender; + background.RegisterInvalidateRender(onInvalidateRender).DisposeWith(disposables); onInvalidateRender(); - new DisposableAction(() => background.InvalidateRender -= onInvalidateRender) - .DisposeWith(disposables); // This is required because changing the CornerRadius changes the background drawing // implementation and we don't want a rectangular background behind a rounded background. @@ -417,8 +412,7 @@ GradientBrush gradientBorder when gradientBorder.CanApplyToBorder(cornerRadius) Action onInvalidateRender = () => layer.FillColor = Brush.GetFallbackColor(borderBrush); onInvalidateRender(); - borderBrush.InvalidateRender += onInvalidateRender; - new DisposableAction(() => borderBrush.InvalidateRender -= onInvalidateRender).DisposeWith(disposables); + borderBrush.RegisterInvalidateRender(onInvalidateRender).DisposeWith(disposables); } } diff --git a/src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.wasm.cs b/src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.wasm.cs index fd3d8daeaf86..5d9965781ad8 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.wasm.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.wasm.cs @@ -20,6 +20,8 @@ partial class BorderLayerRenderer { private Action _backgroundChanged; private Action _borderChanged; + private IDisposable _borderBrushChangedSubscription; + private IDisposable _brushChangedSubscription; partial void UpdatePlatform(bool forceUpdate) { @@ -56,8 +58,9 @@ partial void UpdatePlatform(bool forceUpdate) private void SetAndObserveBorder(BorderLayerState newState) { SetBorder(newState); - Brush.SetupBrushChanged( - _currentState.BorderBrush, + + _borderBrushChangedSubscription?.Dispose(); + _borderBrushChangedSubscription = Brush.SetupBrushChanged( newState.BorderBrush, ref _borderChanged, () => @@ -217,7 +220,8 @@ private void SetAndObserveBackgroundBrush(BorderLayerState newState, ref Action if (newOnInvalidateRender is not null) { - Brush.SetupBrushChanged(oldValue, newValue, ref brushChanged, newOnInvalidateRender); + _brushChangedSubscription?.Dispose(); + _brushChangedSubscription = Brush.SetupBrushChanged(newValue, ref brushChanged, newOnInvalidateRender); } } diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.cs b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.cs index 8051b4f06de9..8941520ee87f 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.cs @@ -40,6 +40,7 @@ public partial class TextBlock : DependencyObject, IThemeChangeAware { private InlineCollection _inlines; private string _inlinesText; // Text derived from the content of Inlines + private IDisposable _foregroundBrushChangedSubscription; #if !__WASM__ // Used for text selection which is handled natively @@ -485,7 +486,9 @@ Brush Foreground private void Subscribe(Brush oldValue, Brush newValue) { var newOnInvalidateRender = _foregroundChanged ?? (() => OnForegroundChanged()); - Brush.SetupBrushChanged(oldValue, newValue, ref _foregroundChanged, newOnInvalidateRender); + + _foregroundBrushChangedSubscription?.Dispose(); + _foregroundBrushChangedSubscription = Brush.SetupBrushChanged(newValue, ref _foregroundChanged, newOnInvalidateRender); } private void OnForegroundChanged() diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs index e2717ed5a46f..116156733681 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs @@ -26,6 +26,7 @@ partial class TextBlock : FrameworkElement, IBlock private readonly TextVisual _textVisual; private Action? _selectionHighlightColorChanged; private MenuFlyout? _contextMenu; + private IDisposable? _selectionHighlightBrushChangedSubscription; private readonly Dictionary _flyoutItems = new(); private readonly VirtualKeyModifiers _platformCtrlKey = OperatingSystem.IsMacOS() ? VirtualKeyModifiers.Windows : VirtualKeyModifiers.Control; @@ -357,7 +358,9 @@ private void OnSelectionHighlightColorChanged(SolidColorBrush? oldBrush, SolidCo { oldBrush ??= DefaultBrushes.SelectionHighlightColor; newBrush ??= DefaultBrushes.SelectionHighlightColor; - Brush.SetupBrushChanged(oldBrush, newBrush, ref _selectionHighlightColorChanged, () => OnSelectionHighlightColorChangedPartial(newBrush)); + + _selectionHighlightBrushChangedSubscription?.Dispose(); + _selectionHighlightBrushChangedSubscription = Brush.SetupBrushChanged(newBrush, ref _selectionHighlightColorChanged, () => OnSelectionHighlightColorChangedPartial(newBrush)); } partial void OnSelectionHighlightColorChangedPartial(SolidColorBrush brush); diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/MultilineTextBoxView.iOS.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/MultilineTextBoxView.iOS.cs index 8b53fa7c0730..37b676b20cf8 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBox/MultilineTextBoxView.iOS.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBox/MultilineTextBoxView.iOS.cs @@ -26,6 +26,7 @@ public partial class MultilineTextBoxView : UITextView, ITextBoxView, Dependency private readonly WeakReference _textBox; private WeakReference _window; private Action _foregroundChanged; + private IDisposable _foregroundBrushChangedSubscription; public override void Paste(NSObject sender) => HandlePaste(() => base.Paste(sender)); @@ -234,7 +235,8 @@ public void OnForegroundChanged(Brush oldValue, Brush newValue) { if (newValue is SolidColorBrush scb) { - Brush.SetupBrushChanged(oldValue, newValue, ref _foregroundChanged, () => ApplyColor()); + _foregroundBrushChangedSubscription?.Dispose(); + _foregroundBrushChangedSubscription = Brush.SetupBrushChanged(newValue, ref _foregroundChanged, () => ApplyColor()); void ApplyColor() { diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/SinglelineTextBoxView.iOS.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/SinglelineTextBoxView.iOS.cs index 7d0635312c36..91a18eaf8e88 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBox/SinglelineTextBoxView.iOS.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBox/SinglelineTextBoxView.iOS.cs @@ -22,6 +22,7 @@ public partial class SinglelineTextBoxView : UITextField, ITextBoxView, Dependen private SinglelineTextBoxDelegate _delegate; private readonly WeakReference _textBox; private Action _foregroundChanged; + private IDisposable _foregroundBrushChangedSubscription; public SinglelineTextBoxView(TextBox textBox) { @@ -204,7 +205,8 @@ public void OnForegroundChanged(Brush oldValue, Brush newValue) { if (newValue is SolidColorBrush scb) { - Brush.SetupBrushChanged(oldValue, newValue, ref _foregroundChanged, () => ApplyColor()); + _foregroundBrushChangedSubscription?.Dispose(); + _foregroundBrushChangedSubscription = Brush.SetupBrushChanged(newValue, ref _foregroundChanged, () => ApplyColor()); void ApplyColor() { diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs index a9b8d293542e..18bc396a19d0 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs @@ -62,6 +62,8 @@ public partial class TextBox : Control, IFrameworkTemplatePoolAware private Action _selectionHighlightColorChanged; private Action _foregroundBrushChanged; + private IDisposable _selectionHighlightBrushChangedSubscription; + private IDisposable _foregroundBrushChangedSubscription; #pragma warning restore CS0067, CS0649 private ContentPresenter _header; @@ -523,7 +525,8 @@ protected override void OnFontWeightChanged(FontWeight oldValue, FontWeight newV protected override void OnForegroundColorChanged(Brush oldValue, Brush newValue) { - Brush.SetupBrushChanged(oldValue, newValue, ref _foregroundBrushChanged, () => OnForegroundColorChangedPartial(newValue)); + _foregroundBrushChangedSubscription?.Dispose(); + _foregroundBrushChangedSubscription = Brush.SetupBrushChanged(newValue, ref _foregroundBrushChanged, () => OnForegroundColorChangedPartial(newValue)); } partial void OnForegroundColorChangedPartial(Brush newValue); @@ -573,7 +576,9 @@ private void OnSelectionHighlightColorChanged(SolidColorBrush oldBrush, SolidCol { oldBrush ??= DefaultBrushes.SelectionHighlightColor; newBrush ??= DefaultBrushes.SelectionHighlightColor; - Brush.SetupBrushChanged(oldBrush, newBrush, ref _selectionHighlightColorChanged, () => OnSelectionHighlightColorChangedPartial(newBrush)); + + _selectionHighlightBrushChangedSubscription?.Dispose(); + _selectionHighlightBrushChangedSubscription = Brush.SetupBrushChanged(newBrush, ref _selectionHighlightColorChanged, () => OnSelectionHighlightColorChangedPartial(newBrush)); } partial void OnSelectionHighlightColorChangedPartial(SolidColorBrush brush); diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.Android.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.Android.cs index 4434466b2161..27eb2641984b 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.Android.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.Android.cs @@ -310,7 +310,8 @@ private void OnForegroundChanged(Brush oldValue, Brush newValue) { if (newValue is SolidColorBrush scb) { - Brush.SetupBrushChanged(oldValue, newValue, ref _foregroundChanged, () => ApplyColor()); + _foregroundBrushChangedSubscription?.Dispose(); + _foregroundBrushChangedSubscription = Brush.SetupBrushChanged(newValue, ref _foregroundChanged, () => ApplyColor()); void ApplyColor() { diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.macOS.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.macOS.cs index b2345db431dd..1d3bb5dfc92b 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.macOS.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.macOS.cs @@ -177,7 +177,8 @@ public NSRange SelectedRange public void OnForegroundChanged(Brush oldValue, Brush newValue) { - Brush.SetupBrushChanged(oldValue, newValue, ref _foregroundChanged, () => ApplyColor()); + _foregroundBrushChangedSubscription?.Dispose(); + _foregroundBrushChangedSubscription = Brush.SetupBrushChanged(newValue, ref _foregroundChanged, () => ApplyColor()); void ApplyColor() { diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.wasm.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.wasm.cs index eaae7da079b3..1d23a28fc281 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.wasm.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.wasm.cs @@ -20,6 +20,7 @@ internal partial class TextBoxView : FrameworkElement { private readonly TextBox _textBox; private Action _foregroundChanged; + private IDisposable _foregroundBrushChangedSubscription; private bool _browserContextMenuEnabled = true; private bool _isReadOnly; @@ -44,7 +45,8 @@ private void OnForegroundChanged(DependencyPropertyChangedEventArgs e) { if (e.NewValue is SolidColorBrush scb) { - Brush.SetupBrushChanged(e.OldValue as Brush, scb, ref _foregroundChanged, () => SetForeground(scb)); + _foregroundBrushChangedSubscription?.Dispose(); + _foregroundBrushChangedSubscription = Brush.SetupBrushChanged(scb, ref _foregroundChanged, () => SetForeground(scb)); } } diff --git a/src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.Android.tt b/src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.Android.tt index 21e35db05b86..085fb15c2111 100644 --- a/src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.Android.tt +++ b/src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.Android.tt @@ -181,12 +181,14 @@ namespace <#= mixin.NamespaceName #> } Action _brushChanged; + IDisposable _backgroundBrushChangedSubscription; protected virtual void OnBackgroundChanged(DependencyPropertyChangedEventArgs e) { - var oldValue = e.OldValue as Brush; var newValue = e.NewValue as Brush; - Brush.SetupBrushChanged(oldValue, newValue, ref _brushChanged, () => SetBackgroundColor(Brush.GetFallbackColor(newValue))); + + _backgroundBrushChangedSubscription?.Dispose(); + _backgroundBrushChangedSubscription = Brush.SetupBrushChanged(newValue, ref _brushChanged, () => SetBackgroundColor(Brush.GetFallbackColor(newValue))); } #endregion diff --git a/src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.iOS.tt b/src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.iOS.tt index e124cf977e9e..8eff9ce29a25 100644 --- a/src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.iOS.tt +++ b/src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.iOS.tt @@ -869,12 +869,14 @@ namespace <#= mixin.NamespaceName #> } Action _brushChanged; + IDisposable _backgroundBrushChangedSubscription; protected virtual void OnBackgroundChanged(DependencyPropertyChangedEventArgs e) { - var oldValue = e.OldValue as Brush; var newValue = e.NewValue as Brush; - Brush.SetupBrushChanged(oldValue, newValue, ref _brushChanged, () => Layer.BackgroundColor = Brush.GetFallbackColor(newValue)); + + _backgroundBrushChangedSubscription?.Dispose(); + _backgroundBrushChangedSubscription = Brush.SetupBrushChanged(newValue, ref _brushChanged, () => Layer.BackgroundColor = Brush.GetFallbackColor(newValue)); } #endregion diff --git a/src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.macOS.tt b/src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.macOS.tt index 6c0a719065c8..b360cc6f7f84 100644 --- a/src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.macOS.tt +++ b/src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.macOS.tt @@ -863,17 +863,18 @@ namespace <#= mixin.NamespaceName #> Action _brushChanged; + IDisposable _backgroundBrushChangedSubscription; protected virtual void OnBackgroundChanged(DependencyPropertyChangedEventArgs e) { - var oldValue = e.OldValue as Brush; var newValue = e.NewValue as Brush; if (newValue is not null) { WantsLayer = true; } - Brush.SetupBrushChanged(oldValue, newValue, ref _brushChanged, () => Layer.BackgroundColor = Brush.GetFallbackColor(newValue)); + _backgroundBrushChangedSubscription?.Dispose(); + _backgroundBrushChangedSubscription = Brush.SetupBrushChanged(newValue, ref _brushChanged, () => Layer.BackgroundColor = Brush.GetFallbackColor(newValue)); } #endregion diff --git a/src/Uno.UI/UI/Xaml/Media/Brush.cs b/src/Uno.UI/UI/Xaml/Media/Brush.cs index 153d45ec9d1b..29080205ad88 100644 --- a/src/Uno.UI/UI/Xaml/Media/Brush.cs +++ b/src/Uno.UI/UI/Xaml/Media/Brush.cs @@ -1,6 +1,7 @@ #nullable enable using System; +using System.Collections.Generic; using System.ComponentModel; using Microsoft.UI.Composition; using Uno.Disposables; @@ -9,6 +10,7 @@ #if HAS_UNO_WINUI using Windows.UI; +using Windows.UI.Core; #endif namespace Microsoft.UI.Xaml.Media @@ -16,13 +18,15 @@ namespace Microsoft.UI.Xaml.Media [TypeConverter(typeof(BrushConverter))] public partial class Brush : DependencyObject { - private readonly WeakEventManager _weakEventManager = new(); + private List? _invalidateRenderHandlers; - internal event Action? InvalidateRender - { - add => _weakEventManager.AddEventHandler(value); - remove => _weakEventManager.RemoveEventHandler(value); - } + internal IDisposable RegisterInvalidateRender(Action handler) + => WeakEventHelper.RegisterEvent( + _invalidateRenderHandlers ??= [], + handler, + (h, s, e) => + (h as Action)?.Invoke() + ); protected Brush() { @@ -46,30 +50,36 @@ internal static Color GetFallbackColor(Brush brush) public static implicit operator Brush(string colorCode) => SolidColorBrushHelper.Parse(colorCode); - internal static void SetupBrushChanged(Brush? oldValue, Brush? newValue, ref Action? onInvalidateRender, Action newOnInvalidateRender, bool initialInvoke = true) + internal static IDisposable? SetupBrushChanged(Brush? newValue, ref Action? onInvalidateRender, Action newOnInvalidateRender, bool initialInvoke = true) { - if (oldValue is not null && onInvalidateRender is not null) - { - oldValue.InvalidateRender -= onInvalidateRender; - } if (initialInvoke) { newOnInvalidateRender(); } + if (newValue is not null) { onInvalidateRender = newOnInvalidateRender; - newValue.InvalidateRender += onInvalidateRender; + return newValue.RegisterInvalidateRender(onInvalidateRender); } else { onInvalidateRender = null; } + + return null; } private protected void OnInvalidateRender() { - _weakEventManager.HandleEvent(nameof(InvalidateRender)); + if (_invalidateRenderHandlers is not null) + { + foreach (var action in _invalidateRenderHandlers) + { + action(this, null); + } + } + #if __SKIA__ SynchronizeCompositionBrush(); #endif diff --git a/src/Uno.UI/UI/Xaml/Shapes/Shape.cs b/src/Uno.UI/UI/Xaml/Shapes/Shape.cs index fbf6754abfd6..a4c20fc368ea 100644 --- a/src/Uno.UI/UI/Xaml/Shapes/Shape.cs +++ b/src/Uno.UI/UI/Xaml/Shapes/Shape.cs @@ -19,6 +19,8 @@ public abstract partial class Shape : FrameworkElement #if !__SKIA__ private Action _brushChanged; private Action _strokeBrushChanged; + private IDisposable _brushChangedSubscription; + private IDisposable _strokeBrushChangedSubscription; #endif /// @@ -62,7 +64,8 @@ private void OnFillChanged(Brush oldValue, Brush newValue) // In this case, we don't really want to listen to brush changes as the Brush is responsible for synchronizing its internal composition brush OnFillBrushChanged(); #else - Brush.SetupBrushChanged(oldValue, newValue, ref _brushChanged, () => OnFillBrushChanged()); + _brushChangedSubscription?.Dispose(); + _brushChangedSubscription = Brush.SetupBrushChanged(newValue, ref _brushChanged, () => OnFillBrushChanged()); #endif } @@ -98,7 +101,8 @@ private void OnStrokeChanged(Brush oldValue, Brush newValue) // In this case, we don't really want to listen to brush changes as the Brush is responsible for synchronizing its internal composition brush OnStrokeBrushChanged(); #else - Brush.SetupBrushChanged(oldValue, newValue, ref _strokeBrushChanged, () => OnStrokeBrushChanged()); + _strokeBrushChangedSubscription?.Dispose(); + _strokeBrushChangedSubscription = Brush.SetupBrushChanged(newValue, ref _strokeBrushChanged, () => OnStrokeBrushChanged()); #endif } diff --git a/src/Uno.UI/Uno.UI.netcoremobile.csproj b/src/Uno.UI/Uno.UI.netcoremobile.csproj index 69a19239075a..3bb7854e34ce 100644 --- a/src/Uno.UI/Uno.UI.netcoremobile.csproj +++ b/src/Uno.UI/Uno.UI.netcoremobile.csproj @@ -46,14 +46,6 @@ - - True - True - - - True - True - True True @@ -144,5 +136,8 @@ + + +