Selecting custom color on non-100% DPI crashes settings (#247)

* Fixed an issue where we were rounding in one place and not in another.
microsoft · Jan 31, 2019 · 24b3dcb · 24b3dcb
1 parent 4b48ecf
commit 24b3dcb
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 9 deletions.
diff --git a/dev/ColorPicker/APITests/ColorPickerTests.cs b/dev/ColorPicker/APITests/ColorPickerTests.cs
@@ -7,11 +7,14 @@
 using System.Numerics;
 using System.Collections;
 using System.Linq;
+using System.Threading;
 using Windows.Foundation;
 using Windows.UI;
 using Windows.UI.Xaml;
 using Windows.UI.Xaml.Controls;
 using Windows.UI.Xaml.Controls.Primitives;
+using Windows.UI.Xaml.Shapes;
+using Windows.UI.Xaml.Media;
 using Common;
 
 #if USING_TAEF
@@ -29,14 +32,25 @@
 using ColorPicker = Microsoft.UI.Xaml.Controls.ColorPicker;
 using ColorChangedEventArgs = Microsoft.UI.Xaml.Controls.ColorChangedEventArgs;
 using ColorSpectrum = Microsoft.UI.Xaml.Controls.Primitives.ColorSpectrum;
-using XamlControlsXamlMetaDataProvider = Microsoft.UI.Xaml.XamlTypeInfo.XamlControlsXamlMetaDataProvider; 
+using XamlControlsXamlMetaDataProvider = Microsoft.UI.Xaml.XamlTypeInfo.XamlControlsXamlMetaDataProvider;
 #endif
 
 namespace Windows.UI.Xaml.Tests.MUXControls.ApiTests
 {
     [TestClass]
     public class ColorPickerTests
     {
+        [TestCleanup]
+        public void TestCleanup()
+        {
+            RunOnUIThread.Execute(() =>
+            {
+                Log.Comment("TestCleanup: Restore TestContentRoot to null");
+                // Put things back the way we found them.
+                MUXControlsTestApp.App.TestContentRoot = null;
+            });
+        }
+
         [TestMethod]
         public void ColorPickerTest()
         {
@@ -232,6 +246,21 @@ public void ValidateHueRange()
             IdleSynchronizer.Wait();
         }
 
+        [TestMethod]
+        public void ValidateFractionalWidthDoesNotCrash()
+        {
+            ColorSpectrum colorSpectrum = null;
+
+            RunOnUIThread.Execute(() =>
+            {
+                colorSpectrum = new ColorSpectrum();
+                colorSpectrum.Width = 300.75;
+                colorSpectrum.Height = 300.75;
+            });
+
+            SetAsRootAndWaitForColorSpectrumFill(colorSpectrum);
+        }
+
         // XamlControlsXamlMetaDataProvider does not exist in the OS repo,
         // so we can't execute this test as authored there.
 #if !BUILD_WINDOWS
@@ -248,5 +277,31 @@ public void VerifyColorPropertyMetadata()
             });
         }
 #endif
+
+        // This takes a FrameworkElement parameter so you can pass in either a ColorPicker or a ColorSpectrum.
+        private void SetAsRootAndWaitForColorSpectrumFill(FrameworkElement element)
+        {
+            ManualResetEvent spectrumLoadedEvent = new ManualResetEvent(false);
+
+            RunOnUIThread.Execute(() =>
+            {
+                element.Loaded += (sender, args) =>
+                {
+                    var spectrumRectangle = VisualTreeUtils.FindVisualChildByName(element, "SpectrumRectangle") as Rectangle;
+                    Verify.IsNotNull(spectrumRectangle);
+
+                    spectrumRectangle.RegisterPropertyChangedCallback(Shape.FillProperty, (o, dp) =>
+                    {
+                        spectrumLoadedEvent.Set();
+                    });
+                };
+
+                StackPanel root = new StackPanel();
+                root.Children.Add(element);
+                MUXControlsTestApp.App.TestContentRoot = root;
+            });
+
+            spectrumLoadedEvent.WaitOne();
+        }
     }
 }
diff --git a/dev/ColorPicker/ColorSpectrum.cpp b/dev/ColorPicker/ColorSpectrum.cpp
@@ -886,20 +886,20 @@ void ColorSpectrum::CreateBitmapsAndColorMap()
     shared_ptr<vector<::byte>> bgraMaxPixelData = make_shared<vector<::byte>>();
     shared_ptr<vector<Hsv>> newHsvValues = make_shared<vector<Hsv>>();
 
-    bgraMinPixelData->reserve(static_cast<size_t>(minDimension * minDimension * 4));
+    bgraMinPixelData->reserve(static_cast<size_t>(round(minDimension * minDimension * 4)));
 
     // We'll only save pixel data for the middle bitmaps if our third dimension is hue.
     if (components == winrt::ColorSpectrumComponents::ValueSaturation ||
         components == winrt::ColorSpectrumComponents::SaturationValue)
     {
-        bgraMiddle1PixelData->reserve(static_cast<size_t>(minDimension * minDimension * 4));
-        bgraMiddle2PixelData->reserve(static_cast<size_t>(minDimension * minDimension * 4));
-        bgraMiddle3PixelData->reserve(static_cast<size_t>(minDimension * minDimension * 4));
-        bgraMiddle4PixelData->reserve(static_cast<size_t>(minDimension * minDimension * 4));
+        bgraMiddle1PixelData->reserve(static_cast<size_t>(round(minDimension * minDimension * 4)));
+        bgraMiddle2PixelData->reserve(static_cast<size_t>(round(minDimension * minDimension * 4)));
+        bgraMiddle3PixelData->reserve(static_cast<size_t>(round(minDimension * minDimension * 4)));
+        bgraMiddle4PixelData->reserve(static_cast<size_t>(round(minDimension * minDimension * 4)));
     }
 
-    bgraMaxPixelData->reserve(static_cast<size_t>(minDimension * minDimension * 4));
-    newHsvValues->reserve(static_cast<size_t>(minDimension * minDimension));
+    bgraMaxPixelData->reserve(static_cast<size_t>(round(minDimension * minDimension * 4)));
+    newHsvValues->reserve(static_cast<size_t>(round(minDimension * minDimension)));
 
     winrt::WorkItemHandler workItemHandler(
         [minDimension, hsv, minHue, maxHue, minSaturation, maxSaturation, minValue, maxValue, shape, components,
@@ -1651,4 +1651,4 @@ bool ColorSpectrum::SelectionEllipseShouldBeLight()
     double bg = displayedColor.B <= 10 ? displayedColor.B / 3294.0 : pow(displayedColor.B / 269.0 + 0.0513, 2.4);
 
     return 0.2126 * rg + 0.7152 * gg + 0.0722 * bg <= 0.5;
-}
+}
diff --git a/test/MUXControlsTestApp/Utilities/VisualTreeUtils.cs b/test/MUXControlsTestApp/Utilities/VisualTreeUtils.cs
@@ -33,5 +33,32 @@ public static T FindElementOfTypeInSubtree<T>(this DependencyObject element)
 
             return null;
         }
+
+        public static DependencyObject FindVisualChildByName(FrameworkElement parent, string name)
+        {
+            if (parent.Name == name)
+            {
+                return parent;
+            }
+
+            int childrenCount = VisualTreeHelper.GetChildrenCount(parent);
+
+            for (int i = 0; i < childrenCount; i++)
+            {
+                FrameworkElement childAsFE = VisualTreeHelper.GetChild(parent, i) as FrameworkElement;
+
+                if (childAsFE != null)
+                {
+                    DependencyObject result = FindVisualChildByName(childAsFE, name);
+
+                    if (result != null)
+                    {
+                        return result;
+                    }
+                }
+            }
+
+            return null;
+        }
     }
 }