From de37c278c3fab7ec5ecbf2ba5270b490ca36bf3e Mon Sep 17 00:00:00 2001
From: xiaoy312 <xiaoyao312@gmail.com>
Date: Wed, 26 Jan 2022 10:31:01 -0500
Subject: [PATCH] fix(shape): Stroke update not triggering re-render

---
 .../Windows_UI_Xaml_Shapes/Shapes_Tests.cs    | 27 +++++-
 .../UITests.Shared/UITests.Shared.projitems   |  7 ++
 .../Shape_StrokeTest.xaml                     | 17 ++++
 .../Shape_StrokeTest.xaml.cs                  | 88 +++++++++++++++++++
 src/Uno.UI/UI/Xaml/Shapes/Shape.cs            |  7 +-
 5 files changed, 141 insertions(+), 5 deletions(-)
 create mode 100644 src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Shapes/Shape_StrokeTest.xaml
 create mode 100644 src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Shapes/Shape_StrokeTest.xaml.cs

diff --git a/src/SamplesApp/SamplesApp.UITests/Windows_UI_Xaml_Shapes/Shapes_Tests.cs b/src/SamplesApp/SamplesApp.UITests/Windows_UI_Xaml_Shapes/Shapes_Tests.cs
index 0a70c3a0772c..c1843afba4b7 100644
--- a/src/SamplesApp/SamplesApp.UITests/Windows_UI_Xaml_Shapes/Shapes_Tests.cs
+++ b/src/SamplesApp/SamplesApp.UITests/Windows_UI_Xaml_Shapes/Shapes_Tests.cs
@@ -201,7 +201,7 @@ public void Default_StrokeThickness()
 				else
 				{
 					var shapeContainer = _app.GetPhysicalRect($"{expectation}Grid");
-					
+
 					ImageAssert.HasColorAt(screenshot, shapeContainer.X + expectation.Offsets[0], shapeContainer.CenterY, expectation.Colors, tolerance: 15);
 					ImageAssert.HasColorAt(screenshot, shapeContainer.CenterX, shapeContainer.Y + expectation.Offsets[1], expectation.Colors, tolerance: 15);
 					ImageAssert.HasColorAt(screenshot, shapeContainer.Right + expectation.Offsets[2], shapeContainer.CenterY, expectation.Colors, tolerance: 15);
@@ -233,6 +233,29 @@ public void Setting_ImageBrush_In_Code_Behind()
 			ImageAssert.HasColorAt(screenshot, rect2.X, rect2.Y, Color.FromArgb(255, 236, 197, 175), tolerance: 5);
 		}
 
+		[Test]
+		[AutoRetry]
+		public void Shape_StrokeColor_ShouldRerenderWithChange()
+		{
+			Run("UITests.Windows_UI_Xaml_Shapes.Shape_StrokeTest");
+
+			var rect = _app.GetPhysicalRect("TestTarget").ToRectangle();
+			var center = rect.Location + new Size(rect.Size.Width / 2, rect.Size.Height / 2);
+			var initialColor = Color.Red;
+			var invertedColor = Color.FromArgb(255, 0, 255, 255); // UpdateBrushColor: flip (x => x^255) all rgb channels, except alpha
+
+			// check color before
+			using var before = TakeScreenshot("Shape_StrokeTest_Before");
+			ImageAssert.HasColorAt(before, center.X, center.Y, initialColor);
+
+			// update brush color
+			_app.FastTap("UpdateBrushColorButton");
+
+			// check color after
+			using var after = TakeScreenshot("Shape_StrokeTest_After");
+			ImageAssert.HasColorAt(after, center.X, center.Y, invertedColor);
+		}
+
 		private static string GetReddish() =>
 			AppInitializer.GetLocalPlatform() switch
 			{
@@ -244,7 +267,7 @@ private static string GetReddish() =>
 		private struct ShapeExpectation
 		{
 			public string Name  { get; set; }
-			public int[] Offsets { get; set; } 
+			public int[] Offsets { get; set; }
 			public string Colors { get; set; }
 
 			public override string ToString() => $"{Name}";
diff --git a/src/SamplesApp/UITests.Shared/UITests.Shared.projitems b/src/SamplesApp/UITests.Shared/UITests.Shared.projitems
index 8549be834fe6..419b035d1aff 100644
--- a/src/SamplesApp/UITests.Shared/UITests.Shared.projitems
+++ b/src/SamplesApp/UITests.Shared/UITests.Shared.projitems
@@ -4369,6 +4369,10 @@
       <SubType>Designer</SubType>
       <Generator>MSBuild:Compile</Generator>
     </Page>
+    <Page Include="$(MSBuildThisFileDirectory)Windows_UI_Xaml_Shapes\Shape_StrokeTest.xaml">
+      <SubType>Designer</SubType>
+      <Generator>MSBuild:Compile</Generator>
+    </Page>
     <Page Include="$(MSBuildThisFileDirectory)Windows_UI_Xaml_Shapes\StretchPage.xaml">
       <SubType>Designer</SubType>
       <Generator>MSBuild:Compile</Generator>
@@ -7126,6 +7130,9 @@
     <Compile Include="$(MSBuildThisFileDirectory)Windows_UI_Xaml_Shapes\Shapes_Default_StrokeThickness.xaml.cs">
       <DependentUpon>Shapes_Default_StrokeThickness.xaml</DependentUpon>
     </Compile>
+    <Compile Include="$(MSBuildThisFileDirectory)Windows_UI_Xaml_Shapes\Shape_StrokeTest.xaml.cs">
+      <DependentUpon>Shape_StrokeTest.xaml</DependentUpon>
+    </Compile>
     <Compile Include="$(MSBuildThisFileDirectory)Windows_UI_Xaml_Shapes\StretchPage.xaml.cs">
       <DependentUpon>StretchPage.xaml</DependentUpon>
     </Compile>
diff --git a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Shapes/Shape_StrokeTest.xaml b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Shapes/Shape_StrokeTest.xaml
new file mode 100644
index 000000000000..96fb2ff523f4
--- /dev/null
+++ b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Shapes/Shape_StrokeTest.xaml
@@ -0,0 +1,17 @@
+<Page x:Class="UITests.Windows_UI_Xaml_Shapes.Shape_StrokeTest"
+	  xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
+	  xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
+
+	<StackPanel Spacing="16">
+		<!-- note: for unknown reason, this Grid is needed for the test to fail before the fix -->
+		<Grid Height="50" Width="50">
+			<Rectangle x:Name="TestTarget"
+					   Height="40" Width="40"
+					   StrokeThickness="20" />
+		</Grid>
+
+		<Button x:Name="ChangeThemeButton" Content="ChangeTheme" Click="{x:Bind ChangeTheme}" />
+		<Button x:Name="UpdateBrushButton" Content="UpdateBrush" Click="{x:Bind UpdateBrush}" />
+		<Button x:Name="UpdateBrushColorButton" Content="UpdateBrushColor" Click="{x:Bind UpdateBrushColor}" />
+	</StackPanel>
+</Page>
diff --git a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Shapes/Shape_StrokeTest.xaml.cs b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Shapes/Shape_StrokeTest.xaml.cs
new file mode 100644
index 000000000000..adea51644c1c
--- /dev/null
+++ b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Shapes/Shape_StrokeTest.xaml.cs
@@ -0,0 +1,88 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Runtime.InteropServices.WindowsRuntime;
+using Uno.UI.Samples.Controls;
+using Windows.Foundation;
+using Windows.Foundation.Collections;
+using Windows.UI;
+using Windows.UI.ViewManagement;
+using Windows.UI.Xaml;
+using Windows.UI.Xaml.Controls;
+using Windows.UI.Xaml.Controls.Primitives;
+using Windows.UI.Xaml.Data;
+using Windows.UI.Xaml.Input;
+using Windows.UI.Xaml.Media;
+using Windows.UI.Xaml.Navigation;
+using XamlWindow = Windows.UI.Xaml.Window;
+
+namespace UITests.Windows_UI_Xaml_Shapes
+{
+	[Sample("Shapes")]
+	public sealed partial class Shape_StrokeTest : Page
+	{
+		private readonly List<SolidColorBrush> _brushes =
+			new[] { Colors.Red, Colors.Green, Colors.Blue }
+				.Select(x => new SolidColorBrush(x))
+				.ToList();
+
+		public Shape_StrokeTest()
+		{
+			this.InitializeComponent();
+			TestTarget.Stroke = _brushes[0];
+
+			// Apperently, these scenarios already work without the fix..?:
+			// - 1 OK: assign Shape.Stroke directly
+			// - 2 OK: assign Shape.Stroke.Color directly
+			// - 3 OK: updating Shape.Stroke.Color(ThemeResource) with (dark/light) theme change
+			// - 4 FAIL: assign Shape.Stroke.Color directly within a Grid?
+			// This test is aimed at the #4 scenario, and tests for its fix.
+		}
+
+		private void ChangeTheme()
+		{
+			if (XamlWindow.Current?.Content is FrameworkElement root)
+			{
+				var theme = root.ActualTheme switch
+				{
+					ElementTheme.Light => ApplicationTheme.Light,
+					ElementTheme.Dark => ApplicationTheme.Dark,
+
+					_ => GetCurrentOsTheme(),
+				};
+				root.RequestedTheme = theme == ApplicationTheme.Light ? ElementTheme.Dark : ElementTheme.Light;
+			}
+
+			ApplicationTheme GetCurrentOsTheme()
+			{
+				var settings = new UISettings();
+				var systemBackground = settings.GetColorValue(UIColorType.Background);
+				var black = Color.FromArgb(255, 0, 0, 0);
+
+				return systemBackground == black ? ApplicationTheme.Dark : ApplicationTheme.Light;
+			}
+		}
+
+		private void UpdateBrush()
+		{
+			var index = (_brushes.IndexOf(TestTarget.Stroke as SolidColorBrush) + 1) % _brushes.Count;
+			var brush = _brushes[index];
+
+			TestTarget.Stroke = brush;
+		}
+
+		private void UpdateBrushColor()
+		{
+			if (TestTarget.Stroke is SolidColorBrush stroke)
+			{
+				stroke.Color = Color.FromArgb(
+					byte.MaxValue,
+					(byte)(stroke.Color.R ^ 255),
+					(byte)(stroke.Color.G ^ 255),
+					(byte)(stroke.Color.B ^ 255)
+				);
+			}
+		}
+	}
+}
diff --git a/src/Uno.UI/UI/Xaml/Shapes/Shape.cs b/src/Uno.UI/UI/Xaml/Shapes/Shape.cs
index 313066d23f19..1ad2de528241 100644
--- a/src/Uno.UI/UI/Xaml/Shapes/Shape.cs
+++ b/src/Uno.UI/UI/Xaml/Shapes/Shape.cs
@@ -55,13 +55,13 @@ public Brush Fill
 				propertyChangedCallback: (s, e) => ((Shape)s).OnFillChanged((Brush)e.NewValue)
 #else
 				options: FrameworkPropertyMetadataOptions.ValueInheritsDataContext | FrameworkPropertyMetadataOptions.LogicalChild,
-				propertyChangedCallback: (s, e) => ((Shape)s)._brushChanged.Disposable = Brush.AssignAndObserveBrush((Brush)e.NewValue, _ => ((Shape)s).InvalidateForFillChanged(), imageBrushCallback: () => ((Shape)s).InvalidateForFillChanged())
+				propertyChangedCallback: (s, e) => ((Shape)s)._brushChanged.Disposable = Brush.AssignAndObserveBrush((Brush)e.NewValue, _ => ((Shape)s).InvalidateForBrushChanged(), imageBrushCallback: () => ((Shape)s).InvalidateForBrushChanged())
 #endif
 			)
 		);
 		#endregion
 
-		private void InvalidateForFillChanged()
+		private void InvalidateForBrushChanged()
 		{
 			// 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
@@ -107,7 +107,8 @@ public Brush Stroke
 #if LEGACY_SHAPE_MEASURE
 				propertyChangedCallback: (s, e) => ((Shape)s).OnStrokeUpdated((Brush)e.NewValue)
 #else
-				options: FrameworkPropertyMetadataOptions.AffectsArrange
+				options: FrameworkPropertyMetadataOptions.AffectsArrange,
+				propertyChangedCallback: (s, e) => ((Shape)s)._strokeBrushChanged.Disposable = Brush.AssignAndObserveBrush((Brush)e.NewValue, _ => ((Shape)s).InvalidateForBrushChanged(), imageBrushCallback: () => ((Shape)s).InvalidateForBrushChanged())
 #endif
 			)
 		);