Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] perf: Brush change subscription #12595

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 33 additions & 43 deletions src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.cs
Original file line number Diff line number Diff line change
@@ -1,27 +1,17 @@
#pragma warning disable CS0109

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Collections.Specialized;
using System.Linq;
using Uno;
using Uno.Disposables;
using Uno.Extensions;
using Uno.Foundation.Logging;
using Windows.UI.Text;
using Windows.UI.Xaml.Automation.Peers;
using Windows.UI.Xaml.Documents;
using Windows.UI.Xaml;
using Uno.UI.DataBinding;
using System;
using Uno.UI;
using System.Collections;
using Windows.UI.Xaml.Input;
using Windows.UI.Xaml.Markup;
using Windows.UI.Xaml.Media;
using Windows.UI.Text;
using Windows.Foundation;
using Windows.UI.Input;
using Windows.UI.Xaml.Input;
using Windows.UI.Xaml.Automation.Peers;
using Uno;
using Uno.Foundation.Logging;

using RadialGradientBrush = Microsoft.UI.Xaml.Media.RadialGradientBrush;

#if XAMARIN_IOS
Expand Down Expand Up @@ -413,41 +403,41 @@ Brush Foreground

private void OnForegroundChanged()
{
void refreshForeground()
_foregroundChanged.Disposable = null;

if (Foreground?.SupportsAssignAndObserveBrush ?? false)
{
// The try-catch here is primarily for the benefit of Android. This callback is raised when (say) the brush color changes,
// which may happen when the system theme changes from light to dark. For app-level resources, a large number of views may
// be subscribed to changes on the brush, including potentially some that have been removed from the visual tree, collected
// on the native side, but not yet collected on the managed side (for Xamarin targets).
_foregroundChanged.Disposable = Foreground.SubscribeToChanges(OnForegroundBrushChanged);
}

OnForegroundBrushChanged();
}

// On Android, in practice this could result in ObjectDisposedExceptions when calling RequestLayout(). The try/catch is to
// ensure that callbacks are correctly raised for remaining views referencing the brush which *are* still live in the visual tree.
private void OnForegroundBrushChanged()
{
// The try-catch here is primarily for the benefit of Android. This callback is raised when (say) the brush color changes,
// which may happen when the system theme changes from light to dark. For app-level resources, a large number of views may
// be subscribed to changes on the brush, including potentially some that have been removed from the visual tree, collected
// on the native side, but not yet collected on the managed side (for Xamarin targets).

// On Android, in practice this could result in ObjectDisposedExceptions when calling RequestLayout(). The try/catch is to
// ensure that callbacks are correctly raised for remaining views referencing the brush which *are* still live in the visual tree.
#if !HAS_EXPENSIVE_TRYFINALLY
try
try
#endif
{
OnForegroundChangedPartial();
InvalidateTextBlock();
}
{
OnForegroundChangedPartial();
InvalidateTextBlock();
}
#if !HAS_EXPENSIVE_TRYFINALLY
catch (Exception e)
catch (Exception e)
{
if (this.Log().IsEnabled(LogLevel.Debug))
{
if (this.Log().IsEnabled(LogLevel.Debug))
{
this.Log().LogDebug($"Failed to invalidate for brush changed: {e}");
}
this.Log().LogDebug($"Failed to invalidate for brush changed: {e}");
}
#endif
}

_foregroundChanged.Disposable = null;

if (Foreground?.SupportsAssignAndObserveBrush ?? false)
{
_foregroundChanged.Disposable = Brush.AssignAndObserveBrush(Foreground, c => refreshForeground(), refreshForeground);
}

refreshForeground();
#endif
}

partial void OnForegroundChangedPartial();
Expand Down
15 changes: 8 additions & 7 deletions src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@
using Uno.Extensions;
using Uno.UI.Common;
using Uno.UI.DataBinding;
using Uno.UI.Xaml.Input;
using Windows.Foundation;
using Windows.System;
using Windows.UI.Core;
using Windows.UI.Text;
using Windows.UI.Xaml.Automation;
using Windows.UI.Xaml.Automation.Peers;
using Windows.UI.Xaml.Data;
using Windows.UI.Xaml.Input;
Expand Down Expand Up @@ -421,12 +419,13 @@ protected override void OnForegroundColorChanged(Brush oldValue, Brush newValue)
_foregroundBrushSubscription.Disposable = null;
if (newValue is SolidColorBrush brush)
{
OnForegroundColorChangedPartial(brush);
_foregroundBrushSubscription.Disposable =
Brush.AssignAndObserveBrush(brush, c => OnForegroundColorChangedPartial(brush));
OnForegroundBrushChanged();
_foregroundBrushSubscription.Disposable = brush.SubscribeToChanges(OnForegroundBrushChanged);
}
}

private void OnForegroundBrushChanged() => OnForegroundColorChangedPartial(Foreground);

partial void OnForegroundColorChangedPartial(Brush newValue);

#region PlaceholderText DependencyProperty
Expand Down Expand Up @@ -475,15 +474,17 @@ private void OnSelectionHighlightColorChanged(SolidColorBrush brush)
_selectionHighlightColorSubscription.Disposable = null;
if (brush is not null)
{
OnSelectionHighlightColorChangedPartial(brush);
_selectionHighlightColorSubscription.Disposable = Brush.AssignAndObserveBrush(brush, c => OnSelectionHighlightColorChangedPartial(brush));
OnSelectionHighlightColorBrushChanged();
_selectionHighlightColorSubscription.Disposable = brush.SubscribeToChanges(OnSelectionHighlightColorBrushChanged);
}
else
{
OnSelectionHighlightColorChangedPartial(DefaultBrushes.SelectionHighlightColor);
}
}

private void OnSelectionHighlightColorBrushChanged() => OnSelectionHighlightColorChangedPartial(SelectionHighlightColor);

partial void OnSelectionHighlightColorChangedPartial(SolidColorBrush brush);

#endregion
Expand Down
59 changes: 55 additions & 4 deletions src/Uno.UI/UI/Xaml/Media/Brush.cs
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
using System;
using System.Collections.Generic;
using System.Text;
using System.ComponentModel;
using System.Diagnostics.Contracts;
using Windows.UI.Xaml.Controls;
using Windows.UI;
using Uno.Collections;
using Uno.UI.DataBinding;
using Uno.UI.Xaml;
using Uno.UI.Xaml.Media;

namespace Windows.UI.Xaml.Media
{
[TypeConverter(typeof(BrushConverter))]
public partial class Brush : DependencyObject
{
private ImmutableList<BrushChangedCallback> _brushChangedCallbacks = ImmutableList<BrushChangedCallback>.Empty;

public Brush()
{
InitializeBinder();
Expand All @@ -34,6 +35,7 @@ public double Opacity

protected virtual void OnOpacityChanged(double oldValue, double newValue)
{
RaiseBrushChanged();
}

#endregion
Expand All @@ -60,6 +62,7 @@ public Transform RelativeTransform

protected virtual void OnRelativeTransformChanged(Transform oldValue, Transform newValue)
{
RaiseBrushChanged();
}

private protected Color GetColorWithOpacity(Color referenceColor)
Expand Down Expand Up @@ -97,5 +100,53 @@ internal static bool TryGetColorWithOpacity(Brush brush, out Color color)
// TODO: Refactor brush handling to a cleaner unified approach - https://github.com/unoplatform/uno/issues/5192
internal bool SupportsAssignAndObserveBrush => true;
#endif

private protected void RaiseBrushChanged()
{
var currentCallbacks = _brushChangedCallbacks.Data;
for (var callbackIndex = 0; callbackIndex < currentCallbacks.Length; callbackIndex++)
{
var callback = currentCallbacks[callbackIndex];
callback.Invoke();
}
}

internal IDisposable SubscribeToChanges(BrushChangedCallback onChanged)
{
var weakOnChangedReference = WeakReferencePool.RentWeakReference(null, onChanged);

BrushChangedCallback weakCallback = () => (!weakOnChangedReference.IsDisposed ?
weakOnChangedReference.Target as BrushChangedCallback : null)?.Invoke();

_brushChangedCallbacks = _brushChangedCallbacks.Add(weakCallback);

return new BrushChangedDisposable(this, weakCallback, weakOnChangedReference);
}

/// <summary>
/// Disposable that removes brush changed callback.
/// </summary>
internal struct BrushChangedDisposable : IDisposable
{
private readonly ManagedWeakReference _brushWeakReference;
private readonly ManagedWeakReference _onChangedWeakReference;
private readonly BrushChangedCallback _callbackWeak;

public BrushChangedDisposable(Brush brush, BrushChangedCallback callbackWeak, ManagedWeakReference onChangedWeakReference)
{
_brushWeakReference = WeakReferencePool.RentWeakReference(brush, brush);
_callbackWeak = callbackWeak;
_onChangedWeakReference = onChangedWeakReference;
}

public void Dispose()
{
if (!_brushWeakReference.IsDisposed && _brushWeakReference.Target is Brush brush)
{
brush._brushChangedCallbacks = brush._brushChangedCallbacks.Remove(_callbackWeak);
_onChangedWeakReference.Dispose();
}
}
}
}
}
6 changes: 1 addition & 5 deletions src/Uno.UI/UI/Xaml/Media/Brush.skia.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,7 @@ void UpdateColorWhenAnyChanged(DependencyObject source, params DependencyPropert

if (brush is SolidColorBrush colorBrush)
{
UpdateColorWhenAnyChanged(colorBrush, new[]
{
SolidColorBrush.ColorProperty,
SolidColorBrush.OpacityProperty
});

}
else if (brush is GradientBrush gradientBrush)
{
Expand Down
3 changes: 3 additions & 0 deletions src/Uno.UI/UI/Xaml/Media/BrushChangedCallback.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace Uno.UI.Xaml.Media;

internal delegate void BrushChangedCallback();
24 changes: 24 additions & 0 deletions src/Uno.UI/UI/Xaml/Media/BrushChangedDisposable.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
using System;
using Uno.UI.DataBinding;

namespace Windows.UI.Xaml.Media;

/// <summary>
/// Disposable that returns a <see cref="ManagedWeakReference"/> to the pool.
/// </summary>
internal struct BrushChangedDisposable : IDisposable
{
private readonly Brush _brush;
private readonly WeakReferenceReturnDisposable _callbackDisposable;

public BrushChangedDisposable(Brush brush, WeakReferenceReturnDisposable callbackDisposable)
{
_brush = brush;
_callbackDisposable = callbackDisposable;
}

public void Dispose()
{
_callbackDisposable.Dispose();
}
}
2 changes: 2 additions & 0 deletions src/Uno.UI/UI/Xaml/Media/SolidColorBrush.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ private void UpdateColorWithOpacity(Windows.UI.Color newColor)
partial void OnColorChanged(Windows.UI.Color oldValue, Windows.UI.Color newValue)
{
UpdateColorWithOpacity(newValue);

RaiseBrushChanged();
}

protected override void OnOpacityChanged(double oldValue, double newValue)
Expand Down