Avoid WriteableBitmap misuse in several places, avoid UnmanagedBlod disposal from finalizer (#14181)

* Fix TrayIconImpl not disposing WriteableBitmap when it should

* Replace WriteableBitmap with Bitmap in ColorPicker

* Replace ArrayList with PooledList to avoid extra allocations

src/Avalonia.Base/Media/Imaging/WriteableBitmap.cs

@@ -20,7 +20,7 @@ namespace Avalonia.Media.Imaging
         /// <param name="dpi">The DPI of the bitmap.</param>
         /// <param name="format">The pixel format (optional).</param>
         /// <param name="alphaFormat">The alpha format (optional).</param>
-        /// <returns>An <see cref="IWriteableBitmapImpl"/>.</returns>
+        /// <returns>An instance of the <see cref="WriteableBitmap"/> class.</returns>
         public WriteableBitmap(PixelSize size, Vector dpi, PixelFormat? format = null, AlphaFormat? alphaFormat = null) 
             : this(CreatePlatformImpl(size, dpi, format, alphaFormat))
         {

src/Avalonia.Controls.ColorPicker/Avalonia.Controls.ColorPicker.csproj

@@ -1,6 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <TargetFrameworks>net6.0;netstandard2.0</TargetFrameworks>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="..\Avalonia.Base\Metadata\NullableAttributes.cs" Link="NullableAttributes.cs" />

src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.cs

@@ -1,4 +1,6 @@
 using System;
+using System.Buffers;
+using Avalonia.Collections.Pooled;
 using Avalonia.Controls.Metadata;
 using Avalonia.Layout;
 using Avalonia.Media;
@@ -32,7 +34,7 @@ namespace Avalonia.Controls.Primitives
 
         protected bool ignorePropertyChanged = false;
 
-        private WriteableBitmap? _backgroundBitmap;
+        private Bitmap? _backgroundBitmap;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="ColorSlider"/> class.
@@ -114,7 +116,10 @@ namespace Avalonia.Controls.Primitives
 
             if (pixelWidth != 0 && pixelHeight != 0)
             {
-                ArrayList<byte> bgraPixelData = await ColorPickerHelpers.CreateComponentBitmapAsync(
+                // siteToCapacity = true, because CreateComponentBitmapAsync sets bytes via indexer over pre-allocated buffer. 
+                using var bgraPixelData = new PooledList<byte>(pixelWidth * pixelHeight * 4, ClearMode.Never, true);
+                await ColorPickerHelpers.CreateComponentBitmapAsync(
+                    bgraPixelData,
                     pixelWidth,
                     pixelHeight,
                     Orientation,
@@ -124,23 +129,8 @@ namespace Avalonia.Controls.Primitives
                     IsAlphaVisible,
                     IsPerceptive);
 
-                if (_backgroundBitmap != null)
+                _backgroundBitmap?.Dispose();
-                {
+                _backgroundBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraPixelData, pixelWidth, pixelHeight);
-                    // TODO: CURRENTLY DISABLED DUE TO INTERMITTENT CRASHES IN SKIA/RENDERER
-                    //
-                    // Re-use the existing WriteableBitmap
-                    // This assumes the height, width and byte counts are the same and must be set to null
-                    // elsewhere if that assumption is ever not true.
-                    // ColorPickerHelpers.UpdateBitmapFromPixelData(_backgroundBitmap, bgraPixelData);
-
-                    // TODO: ALSO DISABLED DISPOSE DUE TO INTERMITTENT CRASHES
-                    //_backgroundBitmap?.Dispose();
-                    _backgroundBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraPixelData, pixelWidth, pixelHeight);
-                }
-                else
-                {
-                    _backgroundBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraPixelData, pixelWidth, pixelHeight);
-                }
 
                 Background = new ImageBrush(_backgroundBitmap);
             }

src/Avalonia.Controls.ColorPicker/ColorSpectrum/ColorSpectrum.cs

@@ -6,6 +6,7 @@
 using System;
 using System.Collections.Generic;
 using System.Threading.Tasks;
+using Avalonia.Collections.Pooled;
 using Avalonia.Controls.Metadata;
 using Avalonia.Controls.Shapes;
 using Avalonia.Input;
@@ -64,19 +65,19 @@ namespace Avalonia.Controls.Primitives
         private Panel? _selectionEllipsePanel;
 
         // Put the spectrum images in a bitmap, which is then given to an ImageBrush.
-        private WriteableBitmap? _hueRedBitmap;
+        private Bitmap? _hueRedBitmap;
-        private WriteableBitmap? _hueYellowBitmap;
+        private Bitmap? _hueYellowBitmap;
-        private WriteableBitmap? _hueGreenBitmap;
+        private Bitmap? _hueGreenBitmap;
-        private WriteableBitmap? _hueCyanBitmap;
+        private Bitmap? _hueCyanBitmap;
-        private WriteableBitmap? _hueBlueBitmap;
+        private Bitmap? _hueBlueBitmap;
-        private WriteableBitmap? _huePurpleBitmap;
+        private Bitmap? _huePurpleBitmap;
 
-        private WriteableBitmap? _saturationMinimumBitmap;
+        private Bitmap? _saturationMinimumBitmap;
-        private WriteableBitmap? _saturationMaximumBitmap;
+        private Bitmap? _saturationMaximumBitmap;
 
-        private WriteableBitmap? _valueBitmap;
+        private Bitmap? _valueBitmap;
-        private WriteableBitmap? _minBitmap;
+        private Bitmap? _minBitmap;
-        private WriteableBitmap? _maxBitmap;
+        private Bitmap? _maxBitmap;
 
         // Fields used by UpdateEllipse() to ensure that it's using the data
         // associated with the last call to CreateBitmapsAndColorMap(),
@@ -1101,17 +1102,7 @@ namespace Avalonia.Controls.Primitives
             }
 
             Hsv hsv = new Hsv(hsvColor);
-
+            
-            // The middle 4 are only needed and used in the case of hue as the third dimension.
-            // Saturation and luminosity need only a min and max.
-            ArrayList<byte> bgraMinPixelData;
-            ArrayList<byte> bgraMiddle1PixelData;
-            ArrayList<byte> bgraMiddle2PixelData;
-            ArrayList<byte> bgraMiddle3PixelData;
-            ArrayList<byte> bgraMiddle4PixelData;
-            ArrayList<byte> bgraMaxPixelData;
-            List<Hsv> newHsvValues;
-
             // In Avalonia, Bounds returns the actual device-independent pixel size of a control.
             // However, this is not necessarily the size of the control rendered on a display.
             // A desktop or application scaling factor may be applied which must be accounted for here.
@@ -1121,27 +1112,20 @@ namespace Avalonia.Controls.Primitives
             int pixelDimension = (int)Math.Round(minDimension * scale);
             var pixelCount = pixelDimension * pixelDimension;
             var pixelDataSize = pixelCount * 4;
-
-            bgraMinPixelData = new ArrayList<byte>(pixelDataSize);
-            bgraMaxPixelData = new ArrayList<byte>(pixelDataSize);
-            newHsvValues = new List<Hsv>(pixelCount);
-
             // We'll only save pixel data for the middle bitmaps if our third dimension is hue.
-            if (components == ColorSpectrumComponents.ValueSaturation ||
+            var middleBitmapsSize =
-                components == ColorSpectrumComponents.SaturationValue)
+                components is ColorSpectrumComponents.ValueSaturation or ColorSpectrumComponents.SaturationValue
-            {
+                    ? pixelDataSize : 0;
-                bgraMiddle1PixelData = new ArrayList<byte>(pixelDataSize);
+
-                bgraMiddle2PixelData = new ArrayList<byte>(pixelDataSize);
+            var newHsvValues = new List<Hsv>(pixelCount);
-                bgraMiddle3PixelData = new ArrayList<byte>(pixelDataSize);
+            using var bgraMinPixelData = new PooledList<byte>(pixelDataSize, ClearMode.Never);
-                bgraMiddle4PixelData = new ArrayList<byte>(pixelDataSize);
+            using var bgraMaxPixelData = new PooledList<byte>(pixelDataSize, ClearMode.Never);
-            }
+            // The middle 4 are only needed and used in the case of hue as the third dimension.
-            else
+            // Saturation and luminosity need only a min and max.
-            {
+            using var bgraMiddle1PixelData = new PooledList<byte>(middleBitmapsSize, ClearMode.Never);
-                bgraMiddle1PixelData = new ArrayList<byte>(0);
+            using var bgraMiddle2PixelData = new PooledList<byte>(middleBitmapsSize, ClearMode.Never);
-                bgraMiddle2PixelData = new ArrayList<byte>(0);
+            using var bgraMiddle3PixelData = new PooledList<byte>(middleBitmapsSize, ClearMode.Never);
-                bgraMiddle3PixelData = new ArrayList<byte>(0);
+            using var bgraMiddle4PixelData = new PooledList<byte>(middleBitmapsSize, ClearMode.Never);
-                bgraMiddle4PixelData = new ArrayList<byte>(0);
-            }
 
             await Task.Run(() =>
             {
@@ -1187,7 +1171,7 @@ namespace Avalonia.Controls.Primitives
                 }
             });
 
-            Dispatcher.UIThread.Post(() =>
+            await Dispatcher.UIThread.InvokeAsync(() =>
             {
                 int pixelWidth = pixelDimension;
                 int pixelHeight = pixelDimension;
@@ -1201,20 +1185,29 @@ namespace Avalonia.Controls.Primitives
                 {
                     case ColorSpectrumComponents.HueValue:
                     case ColorSpectrumComponents.ValueHue:
+                        _saturationMinimumBitmap?.Dispose();
                         _saturationMinimumBitmap = _minBitmap;
+                        _saturationMaximumBitmap?.Dispose();
                         _saturationMaximumBitmap = _maxBitmap;
                         break;
                     case ColorSpectrumComponents.HueSaturation:
                     case ColorSpectrumComponents.SaturationHue:
+                        _valueBitmap?.Dispose();
                         _valueBitmap = _maxBitmap;
                         break;
                     case ColorSpectrumComponents.ValueSaturation:
                     case ColorSpectrumComponents.SaturationValue:
+                        _hueRedBitmap?.Dispose();
                         _hueRedBitmap = _minBitmap;
+                        _hueYellowBitmap?.Dispose();
                         _hueYellowBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraMiddle1PixelData, pixelWidth, pixelHeight);
+                        _hueGreenBitmap?.Dispose();
                         _hueGreenBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraMiddle2PixelData, pixelWidth, pixelHeight);
+                        _hueCyanBitmap?.Dispose();
                         _hueCyanBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraMiddle3PixelData, pixelWidth, pixelHeight);
+                        _hueBlueBitmap?.Dispose();
                         _hueBlueBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraMiddle4PixelData, pixelWidth, pixelHeight);
+                        _huePurpleBitmap?.Dispose();
                         _huePurpleBitmap = _maxBitmap;
                         break;
                 }
@@ -1249,12 +1242,12 @@ namespace Avalonia.Controls.Primitives
             double maxSaturation,
             double minValue,
             double maxValue,
-            ArrayList<byte> bgraMinPixelData,
+            PooledList<byte> bgraMinPixelData,
-            ArrayList<byte> bgraMiddle1PixelData,
+            PooledList<byte> bgraMiddle1PixelData,
-            ArrayList<byte> bgraMiddle2PixelData,
+            PooledList<byte> bgraMiddle2PixelData,
-            ArrayList<byte> bgraMiddle3PixelData,
+            PooledList<byte> bgraMiddle3PixelData,
-            ArrayList<byte> bgraMiddle4PixelData,
+            PooledList<byte> bgraMiddle4PixelData,
-            ArrayList<byte> bgraMaxPixelData,
+            PooledList<byte> bgraMaxPixelData,
             List<Hsv> newHsvValues)
         {
             double hMin = minHue;
@@ -1409,12 +1402,12 @@ namespace Avalonia.Controls.Primitives
             double maxSaturation,
             double minValue,
             double maxValue,
-            ArrayList<byte> bgraMinPixelData,
+            PooledList<byte> bgraMinPixelData,
-            ArrayList<byte> bgraMiddle1PixelData,
+            PooledList<byte> bgraMiddle1PixelData,
-            ArrayList<byte> bgraMiddle2PixelData,
+            PooledList<byte> bgraMiddle2PixelData,
-            ArrayList<byte> bgraMiddle3PixelData,
+            PooledList<byte> bgraMiddle3PixelData,
-            ArrayList<byte> bgraMiddle4PixelData,
+            PooledList<byte> bgraMiddle4PixelData,
-            ArrayList<byte> bgraMaxPixelData,
+            PooledList<byte> bgraMaxPixelData,
             List<Hsv> newHsvValues)
         {
             double hMin = minHue;

src/Avalonia.Controls.ColorPicker/Helpers/ArrayList.cs

@@ -1,71 +0,0 @@
-namespace Avalonia.Controls.Primitives
-{
-    /// <summary>
-    /// A thin wrapper over an <see cref="System.Array"/> that allows some additional list-like functionality.
-    /// </summary>
-    /// <remarks>
-    /// This is only for internal ColorPicker-related functionality and should not be used elsewhere.
-    /// It is added for performance to enjoy the simplicity of the IList.Add() method without requiring
-    /// an additional copy to turn a list into an array for bitmaps.
-    /// </remarks>
-    /// <typeparam name="T">The type of items in the array.</typeparam>
-    internal class ArrayList<T>
-    {
-        private int _nextIndex = 0;
-
-        /// <summary>
-        /// Initializes a new instance of the <see cref="ArrayList{T}"/> class.
-        /// </summary>
-        public ArrayList(int capacity)
-        {
-            Capacity = capacity;
-            Array = new T[capacity];
-        }
-
-        /// <summary>
-        /// Provides access to the underlying array by index.
-        /// This exists for simplification and the <see cref="Array"/> property
-        /// may also be used.
-        /// </summary>
-        /// <param name="i">The index of the item to get or set.</param>
-        /// <returns>The item at the given index.</returns>
-        public T this[int i]
-        {
-            get => Array[i];
-            set => Array[i] = value;
-        }
-
-        /// <summary>
-        /// Gets the underlying array.
-        /// </summary>
-        public T[] Array { get; private set; }
-
-        /// <summary>
-        /// Gets the fixed capacity/size of the array.
-        /// This must be set during construction.
-        /// </summary>
-        public int Capacity { get; private set; }
-
-        /// <summary>
-        /// Adds the given item to the array at the next available index.
-        /// WARNING: This must be used carefully and only once, in sequence.
-        /// </summary>
-        /// <param name="item">The item to add.</param>
-        public void Add(T item)
-        {
-            if (_nextIndex >= 0 &&
-                _nextIndex < Capacity)
-            {
-                Array[_nextIndex] = item;
-                _nextIndex++;
-            }
-            else
-            {
-                // If necessary an exception could be thrown here
-                // throw new IndexOutOfRangeException();
-            }
-
-            return;
-        }
-    }
-}

src/Avalonia.Controls.ColorPicker/Helpers/ColorPickerHelpers.cs

@@ -6,6 +6,7 @@
 using System;
 using System.Runtime.InteropServices;
 using System.Threading.Tasks;
+using Avalonia.Collections.Pooled;
 using Avalonia.Layout;
 using Avalonia.Media;
 using Avalonia.Media.Imaging;
@@ -34,7 +35,8 @@ namespace Avalonia.Controls.Primitives
         /// <param name="isPerceptive">Whether the slider adapts rendering to improve user-perception over exactness.
         /// This will ensure colors are always discernible.</param>
         /// <returns>A new bitmap representing a gradient of color component values.</returns>
-        public static async Task<ArrayList<byte>> CreateComponentBitmapAsync(
+        public static Task CreateComponentBitmapAsync(
+            PooledList<byte> bgraPixelData,
             int width,
             int height,
             Orientation orientation,
@@ -46,22 +48,19 @@ namespace Avalonia.Controls.Primitives
         {
             if (width == 0 || height == 0)
             {
-                return new ArrayList<byte>(0);
+                return Task.CompletedTask;
             }
 
-            var bitmap = await Task.Run<ArrayList<byte>>(() =>
+            return Task.Run(() =>
             {
                 int pixelDataIndex = 0;
                 double componentStep;
-                ArrayList<byte> bgraPixelData;
                 Color baseRgbColor = Colors.White;
                 Color rgbColor;
                 int bgraPixelDataHeight;
                 int bgraPixelDataWidth;
 
-                // Allocate the buffer
                 // BGRA formatted color components 1 byte each (4 bytes in a pixel)
-                bgraPixelData       = new ArrayList<byte>(width * height * 4);
                 bgraPixelDataHeight = height * 4;
                 bgraPixelDataWidth  = width * 4;
 
@@ -318,8 +317,6 @@ namespace Avalonia.Controls.Primitives
 
                 return bgraPixelData;
             });
-
-            return bitmap;
         }
 
         public static Hsv IncrementColorComponent(
@@ -619,45 +616,17 @@ namespace Avalonia.Controls.Primitives
         /// <param name="pixelWidth">The pixel width of the bitmap.</param>
         /// <param name="pixelHeight">The pixel height of the bitmap.</param>
         /// <returns>A new <see cref="WriteableBitmap"/>.</returns>
-        public static WriteableBitmap CreateBitmapFromPixelData(
+        public static unsafe Bitmap CreateBitmapFromPixelData(
-            ArrayList<byte> bgraPixelData,
+            PooledList<byte> bgraPixelData,
             int pixelWidth,
             int pixelHeight)
         {
-            // Standard may need to change on some devices
+            fixed (byte* array = bgraPixelData.Span)
-            Vector dpi = new Vector(96, 96);
-
-            var bitmap = new WriteableBitmap(
-                new PixelSize(pixelWidth, pixelHeight),
-                dpi,
-                PixelFormat.Bgra8888,
-                AlphaFormat.Premul);
-
-            using (var frameBuffer = bitmap.Lock())
             {
-                Marshal.Copy(bgraPixelData.Array, 0, frameBuffer.Address, bgraPixelData.Array.Length);
+                return new Bitmap(PixelFormat.Bgra8888, AlphaFormat.Premul, new IntPtr(array),
+                    new PixelSize(pixelWidth, pixelHeight),
+                    new Vector(96, 96), pixelWidth * 4);
             }
-
-            return bitmap;
-        }
-
-        /// <summary>
-        /// Updates the given <see cref="WriteableBitmap"/> with new, raw BGRA pre-multiplied alpha pixel data.
-        /// TODO: THIS METHOD IS CURRENTLY PROVIDED AS REFERENCE BUT CAUSES INTERMITTENT CRASHES IF USED.
-        /// WARNING: The bitmap's width, height and byte count MUST not have changed and MUST be enforced externally.
-        /// </summary>
-        /// <param name="bitmap">The existing <see cref="WriteableBitmap"/> to update.</param>
-        /// <param name="bgraPixelData">The bitmap (in raw BGRA pre-multiplied alpha pixels).</param>
-        public static void UpdateBitmapFromPixelData(
-            WriteableBitmap bitmap,
-            ArrayList<byte> bgraPixelData)
-        {
-            using (var frameBuffer = bitmap.Lock())
-            {
-                Marshal.Copy(bgraPixelData.Array, 0, frameBuffer.Address, bgraPixelData.Array.Length);
-            }
-
-            return;
         }
     }
 }

src/Windows/Avalonia.Win32/TrayIconImpl.cs

@@ -16,9 +16,7 @@ namespace Avalonia.Win32
 {
     internal class TrayIconImpl : ITrayIconImpl
     {
-        private static readonly Win32Icon s_emptyIcon = new(new WriteableBitmap(new PixelSize(32, 32),
+        private static readonly Win32Icon s_emptyIcon;
-            new Vector(96, 96),
-            PixelFormats.Bgra8888, AlphaFormat.Unpremul));
         private readonly int _uniqueId;
         private static int s_nextUniqueId;
         private bool _iconAdded;
@@ -29,6 +27,13 @@ namespace Avalonia.Win32
         private bool _disposedValue;
         private static readonly uint WM_TASKBARCREATED = RegisterWindowMessage("TaskbarCreated");
 
+        static TrayIconImpl()
+        {
+            using var bitmap = new WriteableBitmap(
+                new PixelSize(32, 32), new Vector(96, 96), PixelFormats.Bgra8888, AlphaFormat.Unpremul);
+            s_emptyIcon = new Win32Icon(bitmap);
+        }
+        
         public TrayIconImpl()
         {
             _exporter = new Win32NativeToManagedMenuExporter();