ColorPicker issues in WASM

[jsinsel-kahua]

Current behavior

Add a <ColorPicker> to any sample project. The following occurs in WASM:

1. The color spectrum does not display even though you can click on it.
2. The track bar is not visible even though you can interact with it if you know where to click.
3. When used in a popup, opening the popup after the first time will not let you interact with the control.

Expected behavior

Should be able to see the color spectrum and track bar. Should be able to interact with it even when shown multiple times in a popup.

How to reproduce it (as minimally and precisely as possible)

Attaching a sample project.
App1.Shared.zip

Workaround

Environment

Nuget Package:

• Uno.UI / Uno.UI.WebAssembly / Uno.UI.Skia
• Uno.WinUI / Uno.WinUI.WebAssembly / Uno.WinUI.Skia
• Uno.SourceGenerationTasks
• Uno.UI.RemoteControl / Uno.WinUI.RemoteControl
• Other:

Nuget Package Version(s):

Affected platform(s):

• iOS
• Android
• WebAssembly
• WebAssembly renderers for Xamarin.Forms
• macOS
• Skia
  • WPF
  • GTK (Linux)
  • Tizen
• Windows
• Build tasks
• Solution Templates

IDE:

• Visual Studio 2017 (version: )
• Visual Studio 2019 (version: )
• Visual Studio for Mac (version: )
• Rider Windows (version: )
• Rider macOS (version: )
• Visual Studio Code (version: )

Relevant plugins:

• Resharper (version: )

Anything else we need to know?

[francoistanguay]

@robloo Any of this rings a bell? Should it work? Not yet implemented?

[robloo]

@francoistanguay @jsinsel The ColorPicker was only tested to work with Android and iOS at the time. There is a known issue in Uno where image source invalidation doesn't work (at least this is the best theory). If you set an ImageBrush's ImageSource to a new WritableBitmap and then that WritableBitmap is updated asyncronously later, updates will be ignored and won't ever display. This means the color spectrum is never visible in some cases. For some reason correctly handling this source invalidation was only implemented on a few platforms. ImageBrush/ImageSource likely needs a big refactoring on all platforms as this code is inconsistent and quite messy.

The issue with the slider backgrounds not being visible may be similar. Although, I've seen evidence there has been regressions with controls derived from Slider. No idea what may be going on with Popup. Personally, I've never worked with the WASM head.

Some key pieces of code to look at:

Code where the WriteableBitmap representing the ColorSpectrum is is created. This is done asynchronously and may occur after the WriteableBitmap was already added to an ImageBrush/ImageSource.

uno/src/Uno.UI/Microsoft/UI/Xaml/Controls/ColorPicker/ColorSpectrum.cs

Lines 1158 to 1166 in e1f887c

	WriteableBitmap minBitmap = ColorHelpers.CreateBitmapFromPixelData(pixelWidth, pixelHeight, bgraMinPixelData);
	WriteableBitmap maxBitmap = ColorHelpers.CreateBitmapFromPixelData(pixelWidth, pixelHeight, bgraMaxPixelData);
	switch (components2)
	{
	case ColorSpectrumComponents.HueValue:
	case ColorSpectrumComponents.ValueHue:
	strongThis.m_saturationMinimumBitmap = minBitmap;
	strongThis.m_saturationMaximumBitmap = maxBitmap;

Code where the WriteableBitmap is set as the ImageSource of an ImageBrush and then the ImageBrush is set as the Fill of the spectrum Rectangle itself.

uno/src/Uno.UI/Microsoft/UI/Xaml/Controls/ColorPicker/ColorSpectrum.cs

Lines 1601 to 1609 in e1f887c

	ImageBrush spectrumBrush = new ImageBrush();
	ImageBrush spectrumOverlayBrush = new ImageBrush();
	spectrumBrush.ImageSource = m_saturationMinimumBitmap;
	spectrumOverlayBrush.ImageSource = m_saturationMaximumBitmap;
	spectrumOverlayRectangle.Opacity = Hsv.GetSaturation(hsvColor);
	spectrumOverlayEllipse.Opacity = Hsv.GetSaturation(hsvColor);
	spectrumRectangle.Fill = spectrumBrush;
	spectrumEllipse.Fill = spectrumBrush;

I do not understand fully how changes are propagated by Uno itself. However, WriteableBitmap, Image, ImageBrush and ImageSource are all involved.

Edit: I should add: a new ImageBrush is created and set as Fill each time UpdateBitmapSources() is called so I might be wrong about this being an issue with update propagation as described above. It could also be related to the Shape.Fill property. Anyway, it's been several months since I looked at this and only ported the code to C# so I shouldn't say much about things I don't fully understand. If there are any specific questions related to ColorPicker and debugging this I will be happy to help though.

[Xiaoy312]

fixme:

• ColorSpectrum (the rainbow part) not showing any color

  wasm: ColorHelpers::CreateBitmapFromPixelData using .WriteAsync() in a non-async method
  skia: Brush::AsssignAndObserveImageBrush missing implementation for ImageBrush

• Luminosity Slider (horizontal slider: black ~ {color}) not showing

  wasm: Brush::AsssignAndObserveImageBrush not observing on GradientBrush.GradientStops and their (Color+Offset) changes

• Irresponsive after being reloaded

  event subscribed OnApplyTemplate and unsubscribed from Unloaded, but not when (re-)Loaded

other issues noticed:

• ColorSpectrum dragging tooltip (nearest known color name) is missing
• Luminosity Slider tooltip is literally "Value %1!u!" when it is supposed to be $"Value {luminosity/255} ({nearest_known_color})"
• LinearGradientBrush doesnt use the correct offsets/plane: expecting left to right, got top-left to bottom-right

[robloo]

@Xiaoy312

1. I've noticed some regressions in sliders in newer versions of Uno, they don't seem to work at all if you derive from them new controls.

2. Image invalidation is still an issue and WASM may still not fully support WriteableBitmap. This affects the Spectrum and Sliders both.

3. On other platforms, such as Android, interaction with the spectrum has also regressed. It is usually not possible to update the color by pressing on the spectrum.

4. Luminosity Slider tooltip is literally "Value %1!u!"

   This was a known issue with the string formatting called out in the original PR: feat(ColorPicker): Add ColorPicker, ColorPickerSlider & ColorSpectrum #3955 (Question 1 at the end of the issue description). @MartinZikmund since then found a helper method that seems to automatically do the conversion. I can't find that comment at the moment to know which method he was talking about specifically but it should be a simple code update. We don't want to modify the WinUI strings themselves or its just going to break in the future. Edit: I found the conversation WinUI InfoBar #4427 (comment)

5. ColorSpectrum dragging tooltip (nearest known color name) is missing

   Again, this was called out in the original PR. It is using ColorHelper.ToDisplayName() which isn't implemented in Uno. The algorithm to implement it is unknown and non-public. There is conditional code built into the upstream WinUI source to disable this. I used this functionality to disable it since Uno does not support it:

   uno/src/Uno.UI/Microsoft/UI/Xaml/Controls/ColorPicker/ColorSpectrum.cs

   Line 173 in a2cfd23

   if (DownlevelHelper.ToDisplayNameExists())

   .

   uno/src/Uno.UI/Helpers/WinUI/DownlevelHelper.cs

   Line 8 in a2cfd23

   public static bool ToDisplayNameExists()

   Additionally, this functionality has been removed in WInUI 3 releases so I expect Uno should keep it disabled indefinitely. Announced for 0.8 here: Announcing: WinUI 3 - Windows App SDK 0.8! 🎉✨ microsoft/microsoft-ui-xaml#5260

I have given up fixing all the related, secondary issues and unsupported features in Uno to get the ColorPicker to work correctly on all platforms. However, I still think the ported code is good and you won't find any issues there (it was well tested on UWP after the C++ -> C# conversion, and then well tested again on Android at the time). If you need help with the ColorPicker code specifically though I will support there. For example, I can take responsibility for number 4 but it will be a while before I get around to it.

[Xiaoy312]

it is working now \o/

edit: the spectrum looks better than that, the gif compression ruined it...

[robloo]

@Xiaoy312 Awesome!

Note that there is a bug in the gradient (you might have fixed that already in the PR as you were adjusting code in that area). However, that is the same bug that existed in iOS.

The slider background gradient origins are somehow incorrect. There is a bug in iOS here it looks like as the gradient should not have a diagonal slant to it -- should be perfectly uniform horizontally.

#3955 (comment)

Edit: Added Graphics

Actual

Expected