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

perf: Avoid storing DefaultValue in DPDetails #18001

Merged
merged 24 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from 21 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
5 changes: 5 additions & 0 deletions build/PackageDiffIgnore.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1874,6 +1874,8 @@
</Fields>

<Properties>
<Member fullName="Windows.UI.Xaml.DependencyProperty Windows.UI.Xaml.FrameworkElement::StretchAffectsMeasureProperty()" reason="Not in WinUI. Made a regular property" />
<Member fullName="Microsoft.UI.Xaml.DependencyProperty Microsoft.UI.Xaml.FrameworkElement::StretchAffectsMeasureProperty()" reason="Not in WinUI. Made a regular property" />
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
</Properties>

<Methods>
Expand All @@ -1890,6 +1892,9 @@
<Member fullName="Microsoft.UI.Xaml.IXamlServiceProvider Uno.UI.Helpers.MarkupHelper.CreateParserContext(System.Object target, Microsoft.UI.Xaml.Markup.ProvideValueTargetProperty property)" reason="Not in WinUI; refactored out" />
<Member fullName="Windows.UI.Xaml.IXamlServiceProvider Uno.UI.Helpers.MarkupHelper.CreateParserContext(System.Object target, Windows.UI.Xaml.Markup.ProvideValueTargetProperty property)" reason="Not in WinUI; refactored out" />

<Member fullName="Microsoft.UI.Xaml.DependencyProperty Microsoft.UI.Xaml.FrameworkElement.get_StretchAffectsMeasureProperty()" reason="Not in WinUI. Made a regular property" />
<Member fullName="Windows.UI.Xaml.DependencyProperty Windows.UI.Xaml.FrameworkElement.get_StretchAffectsMeasureProperty()" reason="Not in WinUI. Made a regular property" />

</Methods>
</IgnoreSet>
<![CDATA[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,16 @@ private static void OnSampleContentChanged(object dependencyObject, DependencyPr
}

#if XAMARIN
protected override void OnDataContextChanged()
internal override bool GetDefaultValue2(DependencyProperty property, out object defaultValue)
MartinZikmund marked this conversation as resolved.
Show resolved Hide resolved
{
base.OnDataContextChanged();
if (property == ContentProperty)
{
// Workaround to #10396: The DataContext of ContentTemplate should be ContentControl.DataContext if ContentControl.Content is not set.
defaultValue = DataContext;
return true;
}

// Workaround to #10396: The DataContext of ContentTemplate should be ContentControl.DataContext if ContentControl.Content is not set.
this.SetValue(ContentProperty, DataContext, DependencyPropertyValuePrecedences.DefaultValue);
return base.GetDefaultValue2(property, out defaultValue);
}
#endif
}
Expand Down
14 changes: 0 additions & 14 deletions src/Uno.UI.RuntimeTests/Helpers/StyleHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,27 +93,13 @@ public static IDisposable UseFluentStyles()

// Force default brushes to be reloaded
DefaultBrushes.ResetDefaultThemeBrushes();
ResetIslandRootForeground();

return new DisposableAction(() =>
{
resources.MergedDictionaries.Remove(xcr);
DefaultBrushes.ResetDefaultThemeBrushes();
ResetIslandRootForeground();
});
#endif
}

#if !WINAPPSDK
private static void ResetIslandRootForeground()
{
if (Uno.UI.Xaml.Core.CoreServices.Instance.InitializationType == Xaml.Core.InitializationType.IslandsOnly &&
VisualTreeUtils.FindVisualChildByType<Control>(TestServices.WindowHelper.XamlRoot.VisualTree.RootElement) is { } control)
{
// Ensure the root element's Foreground is set correctly
control.SetValue(Control.ForegroundProperty, DefaultBrushes.TextForegroundBrush, DependencyPropertyValuePrecedences.DefaultValue);
}
}
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,7 @@ public void When_GetDefaultValue()
var defaultValueTest = new DefaultValueTest();
Assert.AreEqual(expected, defaultValueTest.GetPrecedenceSpecificValue(DefaultValueTest.TestValueProperty, DependencyPropertyValuePrecedences.DefaultValue));
Assert.AreEqual(expected, defaultValueTest.GetValue(DefaultValueTest.TestValueProperty));
Assert.AreEqual(expected, (defaultValueTest as IDependencyObjectStoreProvider).Store.GetPropertyDetails(DefaultValueTest.TestValueProperty).GetDefaultValue());
Assert.AreEqual(expected, defaultValueTest.TestValue);
}

[TestMethod]
public void When_SetDefaultValue()
{
var expected = 24;
var defaultValueTest = new DefaultValueTest();
((IDependencyObjectStoreProvider)defaultValueTest).Store.GetPropertyDetails(DefaultValueTest.TestValueProperty).SetDefaultValue(expected);
Assert.AreEqual(expected, defaultValueTest.GetPrecedenceSpecificValue(DefaultValueTest.TestValueProperty, DependencyPropertyValuePrecedences.DefaultValue));
Assert.AreEqual(expected, defaultValueTest.GetValue(DefaultValueTest.TestValueProperty));
Assert.AreEqual(expected, ((IDependencyObjectStoreProvider)defaultValueTest).Store.GetPropertyDetails(DefaultValueTest.TestValueProperty).GetDefaultValue());
Assert.AreEqual(expected, (defaultValueTest as IDependencyObjectStoreProvider).Store.GetDefaultValue(DefaultValueTest.TestValueProperty));
Assert.AreEqual(expected, defaultValueTest.TestValue);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -538,16 +538,10 @@ WeakReference Build()
var dc = new object();
SUT.DataContext = dc;

var originalBrush = SUT.Foreground as Brush;

SUT.SetValue(ContentControl.ForegroundProperty, null);

Assert.IsNull(originalBrush.DataContext);

SUT.ClearValue(ContentControl.ForegroundProperty);

Assert.AreEqual(dc, originalBrush.DataContext);

SUT.DataContext = null;

return new(dc);
Expand Down
2 changes: 1 addition & 1 deletion src/Uno.UI/DataBinding/BindingExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ private void ApplyFallbackValue(bool useTypeDefaultValue = true)
else if (useTypeDefaultValue && TargetPropertyDetails != null)
{
var viewTarget = _view.Target;
SetTargetValue(TargetPropertyDetails.Property.GetDefaultValue(viewTarget as UIElement, viewTarget?.GetType()));
SetTargetValue(TargetPropertyDetails.Property.GetDefaultValue(viewTarget as DependencyObject, viewTarget?.GetType()));
}
}

Expand Down
58 changes: 58 additions & 0 deletions src/Uno.UI/Helpers/Boxes.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
using System;
using Microsoft.UI.Xaml;
using Microsoft.UI.Xaml.Media;
using Uno.UI.Xaml;

namespace Uno.UI.Helpers;


internal static class Boxes
{
public static class BooleanBoxes
{
public static readonly object BoxedTrue = true;
public static readonly object BoxedFalse = false;
}

public static class VerticalAlignmentBoxes
{
public static readonly VerticalAlignment Top = VerticalAlignment.Top;
public static readonly VerticalAlignment Bottom = VerticalAlignment.Bottom;
public static readonly VerticalAlignment Stretch = VerticalAlignment.Stretch;
public static readonly VerticalAlignment Center = VerticalAlignment.Center;
}

public static class NullableDoubleBoxes
{
public static readonly double? Zero = 0.0d;
public static readonly double? One = 1.0d;
}

public static class StretchBoxes
{
public static readonly Stretch None = Stretch.None;
public static readonly Stretch Fill = Stretch.Fill;
public static readonly Stretch Uniform = Stretch.Uniform;
public static readonly Stretch UniformToFill = Stretch.UniformToFill;
}

public static object Box(bool value) => value ? BooleanBoxes.BoxedTrue : BooleanBoxes.BoxedFalse;

public static object Box(VerticalAlignment value) => value switch
{
VerticalAlignment.Top => VerticalAlignmentBoxes.Top,
VerticalAlignment.Bottom => VerticalAlignmentBoxes.Bottom,
VerticalAlignment.Stretch => VerticalAlignmentBoxes.Stretch,
VerticalAlignment.Center => VerticalAlignmentBoxes.Center,
_ => value,
};

public static object Box(Stretch value) => value switch
{
Stretch.None => StretchBoxes.None,
Stretch.Fill => StretchBoxes.Fill,
Stretch.Uniform => StretchBoxes.Uniform,
Stretch.UniformToFill => StretchBoxes.UniformToFill,
_ => value,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public TextReadingOrder TextReadingOrder
}

public static DependencyProperty TextReadingOrderProperty { get; } =
DependencyProperty.Register(nameof(TextReadingOrder), typeof(TextReadingOrder), typeof(NumberBox), new FrameworkPropertyMetadata(null));
DependencyProperty.Register(nameof(TextReadingOrder), typeof(TextReadingOrder), typeof(NumberBox), new FrameworkPropertyMetadata(TextReadingOrder.Default));

public bool PreventKeyboardDisplayOnProgrammaticFocus
{
Expand All @@ -168,7 +168,7 @@ public bool PreventKeyboardDisplayOnProgrammaticFocus
}

public static DependencyProperty PreventKeyboardDisplayOnProgrammaticFocusProperty { get; } =
DependencyProperty.Register(nameof(PreventKeyboardDisplayOnProgrammaticFocus), typeof(bool), typeof(NumberBox), new FrameworkPropertyMetadata(null));
DependencyProperty.Register(nameof(PreventKeyboardDisplayOnProgrammaticFocus), typeof(bool), typeof(NumberBox), new FrameworkPropertyMetadata(false));

public new object Description
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public ContentPresenter()
#if !UNO_HAS_BORDER_VISUAL
_borderRenderer = new BorderLayerRenderer(this);
#endif
SetDefaultForeground(ForegroundProperty);
UpdateLastUsedTheme();

InitializePlatform();
}
Expand Down Expand Up @@ -1234,7 +1234,7 @@ protected override void OnBackgroundChanged(DependencyPropertyChangedEventArgs e
internal override void UpdateThemeBindings(ResourceUpdateReason updateReason)
{
base.UpdateThemeBindings(updateReason);
SetDefaultForeground(ForegroundProperty);
UpdateLastUsedTheme();
}

#if __ANDROID__
Expand Down
9 changes: 0 additions & 9 deletions src/Uno.UI/UI/Xaml/Controls/Control/Control.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ public partial class Control : FrameworkElement

private void InitializeControl()
{
SetDefaultForeground(ForegroundProperty);
SubscribeToOverridenRoutedEvents();
OnIsFocusableChanged();

Expand All @@ -59,14 +58,6 @@ private void InitializeControl()

internal override bool IsEnabledOverride() => IsEnabled && base.IsEnabledOverride();

internal override void UpdateThemeBindings(Data.ResourceUpdateReason updateReason)
{
base.UpdateThemeBindings(updateReason);

//override the default value from dependency property based on application theme
SetDefaultForeground(ForegroundProperty);
}

private protected override Type GetDefaultStyleKey() => DefaultStyleKey as Type;

protected override void OnBackgroundChanged(DependencyPropertyChangedEventArgs e)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ private static void OnFocusedElementChanged(DependencyObject dependencyObject, D

if (args.NewValue is FrameworkElement element)
{
element.EnsureFocusVisualBrushDefaults();
element.SizeChanged += focusVisual.FocusedElementSizeChanged;
#if !UNO_HAS_ENHANCED_LIFECYCLE
element.LayoutUpdated += focusVisual.FocusedElementLayoutUpdated;
Expand Down
6 changes: 5 additions & 1 deletion src/Uno.UI/UI/Xaml/Controls/Icons/BitmapIcon.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace Microsoft.UI.Xaml.Controls;
/// <summary>
/// Represents an icon that uses a bitmap as its content.
/// </summary>
public partial class BitmapIcon : IconElement
public partial class BitmapIcon : IconElement, IThemeChangeAware
{
private readonly Image _image;

Expand Down Expand Up @@ -78,4 +78,8 @@ private void UpdateImageMonochromeColor()
}
#endif
}

// The way this works in WinUI is by the MarkInheritedPropertyDirty call in CFrameworkElement::NotifyThemeChangedForInheritedProperties
// There is a special handling for Foreground specifically there.
void IThemeChangeAware.OnThemeChanged() => UpdateImageMonochromeColor();
}
12 changes: 11 additions & 1 deletion src/Uno.UI/UI/Xaml/Controls/Icons/FontIcon.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.UI.Xaml.Controls;
/// <summary>
/// Represents an icon that uses a glyph from the specified font.
/// </summary>
public partial class FontIcon : IconElement
public partial class FontIcon : IconElement, IThemeChangeAware
{
private readonly TextBlock _textBlock;

Expand Down Expand Up @@ -220,4 +220,14 @@ private protected override void OnForegroundChanged(DependencyPropertyChangedEve
_textBlock.Foreground = (Brush)e.NewValue;
}
}

// The way this works in WinUI is by the MarkInheritedPropertyDirty call in CFrameworkElement::NotifyThemeChangedForInheritedProperties
// There is a special handling for Foreground specifically there.
void IThemeChangeAware.OnThemeChanged()
{
if (_textBlock is not null)
{
_textBlock.Foreground = Foreground;
}
}
}
4 changes: 2 additions & 2 deletions src/Uno.UI/UI/Xaml/Controls/Icons/IconElement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public partial class IconElement : FrameworkElement

public IconElement()
{
SetDefaultForeground(ForegroundProperty);
UpdateLastUsedTheme();
}

/// <summary>
Expand Down Expand Up @@ -95,7 +95,7 @@ internal override void UpdateThemeBindings(ResourceUpdateReason updateReason)
{
base.UpdateThemeBindings(updateReason);

SetDefaultForeground(ForegroundProperty);
UpdateLastUsedTheme();
}

[MemberNotNull(nameof(_rootGrid))]
Expand Down
12 changes: 11 additions & 1 deletion src/Uno.UI/UI/Xaml/Controls/Icons/PathIcon.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace Microsoft.UI.Xaml.Controls;
/// <summary>
/// Represents an icon that uses a vector path as its content.
/// </summary>
public partial class PathIcon : IconElement
public partial class PathIcon : IconElement, IThemeChangeAware
{
private readonly Path _path;

Expand Down Expand Up @@ -60,4 +60,14 @@ private protected override void OnForegroundChanged(DependencyPropertyChangedEve
_path.Fill = (Brush)e.NewValue;
}
}

// The way this works in WinUI is by the MarkInheritedPropertyDirty call in CFrameworkElement::NotifyThemeChangedForInheritedProperties
// There is a special handling for Foreground specifically there.
void IThemeChangeAware.OnThemeChanged()
{
if (_path is not null)
{
_path.Fill = Foreground;
}
}
}
12 changes: 11 additions & 1 deletion src/Uno.UI/UI/Xaml/Controls/Icons/SymbolIcon.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.UI.Xaml.Controls;
/// <summary>
/// Represents an icon that uses a glyph from the Segoe MDL2 Assets font as its content.
/// </summary>
public sealed partial class SymbolIcon : IconElement
public sealed partial class SymbolIcon : IconElement, IThemeChangeAware
{
private double _fontSize = 20.0;

Expand Down Expand Up @@ -103,4 +103,14 @@ private protected override void OnForegroundChanged(DependencyPropertyChangedEve
_textBlock.Foreground = (Brush)e.NewValue;
}
}

// The way this works in WinUI is by the MarkInheritedPropertyDirty call in CFrameworkElement::NotifyThemeChangedForInheritedProperties
// There is a special handling for Foreground specifically there.
void IThemeChangeAware.OnThemeChanged()
{
if (_textBlock is not null)
{
_textBlock.Foreground = Foreground;
}
}
}
2 changes: 1 addition & 1 deletion src/Uno.UI/UI/Xaml/Controls/Popup/Popup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ internal override bool GetDefaultValue2(DependencyProperty property, out object
{
if (property == IsLightDismissEnabledProperty)
{
defaultValue = FeatureConfiguration.Popup.EnableLightDismissByDefault;
defaultValue = Uno.UI.Helpers.Boxes.Box(FeatureConfiguration.Popup.EnableLightDismissByDefault);
return true;
}
return base.GetDefaultValue2(property, out defaultValue);
Expand Down
10 changes: 7 additions & 3 deletions src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
namespace Microsoft.UI.Xaml.Controls
{
[ContentProperty(Name = nameof(Inlines))]
public partial class TextBlock : DependencyObject
public partial class TextBlock : DependencyObject, IThemeChangeAware
{
private InlineCollection _inlines;
private string _inlinesText; // Text derived from the content of Inlines
Expand Down Expand Up @@ -74,7 +74,7 @@ private Range Selection
public TextBlock()
{
IFrameworkElementHelper.Initialize(this);
SetDefaultForeground(ForegroundProperty);
UpdateLastUsedTheme();

_hyperlinks.CollectionChanged += HyperlinksOnCollectionChanged;

Expand Down Expand Up @@ -1230,7 +1230,7 @@ internal override void UpdateThemeBindings(Data.ResourceUpdateReason updateReaso
{
base.UpdateThemeBindings(updateReason);

SetDefaultForeground(ForegroundProperty);
UpdateLastUsedTheme();

if (_inlines is not null)
{
Expand Down Expand Up @@ -1265,5 +1265,9 @@ public Range((int start, int end) tuple) : this(tuple.start, tuple.end)

[SuppressMessage("CodeQuality", "IDE0051:Remove unused private members", Justification = "Used only by some platforms")]
partial void UpdateIsTextTrimmed();

// The way this works in WinUI is by the MarkInheritedPropertyDirty call in CFrameworkElement::NotifyThemeChangedForInheritedProperties
// There is a special handling for Foreground specifically there.
void IThemeChangeAware.OnThemeChanged() => OnForegroundChanged();
}
}
Loading
Loading