Skip to content

Commit 473b21b

Browse files
committed
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
1 parent 33b1bfb commit 473b21b

File tree

7 files changed

+78
-191
lines changed

7 files changed

+78
-191
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public class WriteableBitmap : Bitmap
2020
/// <param name="dpi">The DPI of the bitmap.</param>
2121
/// <param name="format">The pixel format (optional).</param>
2222
/// <param name="alphaFormat">The alpha format (optional).</param>
23-
/// <returns>An <see cref="IWriteableBitmapImpl"/>.</returns>
23+
/// <returns>An instance of the <see cref="WriteableBitmap"/> class.</returns>
2424
public WriteableBitmap(PixelSize size, Vector dpi, PixelFormat? format = null, AlphaFormat? alphaFormat = null)
2525
: this(CreatePlatformImpl(size, dpi, format, alphaFormat))
2626
{

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
33
<TargetFrameworks>net6.0;netstandard2.0</TargetFrameworks>
4+
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
45
</PropertyGroup>
56
<ItemGroup>
67
<Compile Include="..\Avalonia.Base\Metadata\NullableAttributes.cs" Link="NullableAttributes.cs" />

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

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
using System;
2+
using System.Buffers;
3+
using Avalonia.Collections.Pooled;
24
using Avalonia.Controls.Metadata;
35
using Avalonia.Layout;
46
using Avalonia.Media;
@@ -32,7 +34,7 @@ public partial class ColorSlider : Slider
3234

3335
protected bool ignorePropertyChanged = false;
3436

35-
private WriteableBitmap? _backgroundBitmap;
37+
private Bitmap? _backgroundBitmap;
3638

3739
/// <summary>
3840
/// Initializes a new instance of the <see cref="ColorSlider"/> class.
@@ -114,7 +116,10 @@ private async void UpdateBackground()
114116

115117
if (pixelWidth != 0 && pixelHeight != 0)
116118
{
117-
ArrayList<byte> bgraPixelData = await ColorPickerHelpers.CreateComponentBitmapAsync(
119+
// siteToCapacity = true, because CreateComponentBitmapAsync sets bytes via indexer over pre-allocated buffer.
120+
using var bgraPixelData = new PooledList<byte>(pixelWidth * pixelHeight * 4, ClearMode.Never, true);
121+
await ColorPickerHelpers.CreateComponentBitmapAsync(
122+
bgraPixelData,
118123
pixelWidth,
119124
pixelHeight,
120125
Orientation,
@@ -124,23 +129,8 @@ private async void UpdateBackground()
124129
IsAlphaVisible,
125130
IsPerceptive);
126131

127-
if (_backgroundBitmap != null)
128-
{
129-
// TODO: CURRENTLY DISABLED DUE TO INTERMITTENT CRASHES IN SKIA/RENDERER
130-
//
131-
// Re-use the existing WriteableBitmap
132-
// This assumes the height, width and byte counts are the same and must be set to null
133-
// elsewhere if that assumption is ever not true.
134-
// ColorPickerHelpers.UpdateBitmapFromPixelData(_backgroundBitmap, bgraPixelData);
135-
136-
// TODO: ALSO DISABLED DISPOSE DUE TO INTERMITTENT CRASHES
137-
//_backgroundBitmap?.Dispose();
138-
_backgroundBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraPixelData, pixelWidth, pixelHeight);
139-
}
140-
else
141-
{
142-
_backgroundBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraPixelData, pixelWidth, pixelHeight);
143-
}
132+
_backgroundBitmap?.Dispose();
133+
_backgroundBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraPixelData, pixelWidth, pixelHeight);
144134

145135
Background = new ImageBrush(_backgroundBitmap);
146136
}

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

Lines changed: 48 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System;
77
using System.Collections.Generic;
88
using System.Threading.Tasks;
9+
using Avalonia.Collections.Pooled;
910
using Avalonia.Controls.Metadata;
1011
using Avalonia.Controls.Shapes;
1112
using Avalonia.Input;
@@ -64,19 +65,19 @@ public partial class ColorSpectrum : TemplatedControl
6465
private Panel? _selectionEllipsePanel;
6566

6667
// Put the spectrum images in a bitmap, which is then given to an ImageBrush.
67-
private WriteableBitmap? _hueRedBitmap;
68-
private WriteableBitmap? _hueYellowBitmap;
69-
private WriteableBitmap? _hueGreenBitmap;
70-
private WriteableBitmap? _hueCyanBitmap;
71-
private WriteableBitmap? _hueBlueBitmap;
72-
private WriteableBitmap? _huePurpleBitmap;
68+
private Bitmap? _hueRedBitmap;
69+
private Bitmap? _hueYellowBitmap;
70+
private Bitmap? _hueGreenBitmap;
71+
private Bitmap? _hueCyanBitmap;
72+
private Bitmap? _hueBlueBitmap;
73+
private Bitmap? _huePurpleBitmap;
7374

74-
private WriteableBitmap? _saturationMinimumBitmap;
75-
private WriteableBitmap? _saturationMaximumBitmap;
75+
private Bitmap? _saturationMinimumBitmap;
76+
private Bitmap? _saturationMaximumBitmap;
7677

77-
private WriteableBitmap? _valueBitmap;
78-
private WriteableBitmap? _minBitmap;
79-
private WriteableBitmap? _maxBitmap;
78+
private Bitmap? _valueBitmap;
79+
private Bitmap? _minBitmap;
80+
private Bitmap? _maxBitmap;
8081

8182
// Fields used by UpdateEllipse() to ensure that it's using the data
8283
// associated with the last call to CreateBitmapsAndColorMap(),
@@ -1101,17 +1102,7 @@ private async void CreateBitmapsAndColorMap()
11011102
}
11021103

11031104
Hsv hsv = new Hsv(hsvColor);
1104-
1105-
// The middle 4 are only needed and used in the case of hue as the third dimension.
1106-
// Saturation and luminosity need only a min and max.
1107-
ArrayList<byte> bgraMinPixelData;
1108-
ArrayList<byte> bgraMiddle1PixelData;
1109-
ArrayList<byte> bgraMiddle2PixelData;
1110-
ArrayList<byte> bgraMiddle3PixelData;
1111-
ArrayList<byte> bgraMiddle4PixelData;
1112-
ArrayList<byte> bgraMaxPixelData;
1113-
List<Hsv> newHsvValues;
1114-
1105+
11151106
// In Avalonia, Bounds returns the actual device-independent pixel size of a control.
11161107
// However, this is not necessarily the size of the control rendered on a display.
11171108
// A desktop or application scaling factor may be applied which must be accounted for here.
@@ -1121,27 +1112,20 @@ private async void CreateBitmapsAndColorMap()
11211112
int pixelDimension = (int)Math.Round(minDimension * scale);
11221113
var pixelCount = pixelDimension * pixelDimension;
11231114
var pixelDataSize = pixelCount * 4;
1124-
1125-
bgraMinPixelData = new ArrayList<byte>(pixelDataSize);
1126-
bgraMaxPixelData = new ArrayList<byte>(pixelDataSize);
1127-
newHsvValues = new List<Hsv>(pixelCount);
1128-
11291115
// We'll only save pixel data for the middle bitmaps if our third dimension is hue.
1130-
if (components == ColorSpectrumComponents.ValueSaturation ||
1131-
components == ColorSpectrumComponents.SaturationValue)
1132-
{
1133-
bgraMiddle1PixelData = new ArrayList<byte>(pixelDataSize);
1134-
bgraMiddle2PixelData = new ArrayList<byte>(pixelDataSize);
1135-
bgraMiddle3PixelData = new ArrayList<byte>(pixelDataSize);
1136-
bgraMiddle4PixelData = new ArrayList<byte>(pixelDataSize);
1137-
}
1138-
else
1139-
{
1140-
bgraMiddle1PixelData = new ArrayList<byte>(0);
1141-
bgraMiddle2PixelData = new ArrayList<byte>(0);
1142-
bgraMiddle3PixelData = new ArrayList<byte>(0);
1143-
bgraMiddle4PixelData = new ArrayList<byte>(0);
1144-
}
1116+
var middleBitmapsSize =
1117+
components is ColorSpectrumComponents.ValueSaturation or ColorSpectrumComponents.SaturationValue
1118+
? pixelDataSize : 0;
1119+
1120+
var newHsvValues = new List<Hsv>(pixelCount);
1121+
using var bgraMinPixelData = new PooledList<byte>(pixelDataSize, ClearMode.Never);
1122+
using var bgraMaxPixelData = new PooledList<byte>(pixelDataSize, ClearMode.Never);
1123+
// The middle 4 are only needed and used in the case of hue as the third dimension.
1124+
// Saturation and luminosity need only a min and max.
1125+
using var bgraMiddle1PixelData = new PooledList<byte>(middleBitmapsSize, ClearMode.Never);
1126+
using var bgraMiddle2PixelData = new PooledList<byte>(middleBitmapsSize, ClearMode.Never);
1127+
using var bgraMiddle3PixelData = new PooledList<byte>(middleBitmapsSize, ClearMode.Never);
1128+
using var bgraMiddle4PixelData = new PooledList<byte>(middleBitmapsSize, ClearMode.Never);
11451129

11461130
await Task.Run(() =>
11471131
{
@@ -1187,7 +1171,7 @@ await Task.Run(() =>
11871171
}
11881172
});
11891173

1190-
Dispatcher.UIThread.Post(() =>
1174+
await Dispatcher.UIThread.InvokeAsync(() =>
11911175
{
11921176
int pixelWidth = pixelDimension;
11931177
int pixelHeight = pixelDimension;
@@ -1201,20 +1185,29 @@ await Task.Run(() =>
12011185
{
12021186
case ColorSpectrumComponents.HueValue:
12031187
case ColorSpectrumComponents.ValueHue:
1188+
_saturationMinimumBitmap?.Dispose();
12041189
_saturationMinimumBitmap = _minBitmap;
1190+
_saturationMaximumBitmap?.Dispose();
12051191
_saturationMaximumBitmap = _maxBitmap;
12061192
break;
12071193
case ColorSpectrumComponents.HueSaturation:
12081194
case ColorSpectrumComponents.SaturationHue:
1195+
_valueBitmap?.Dispose();
12091196
_valueBitmap = _maxBitmap;
12101197
break;
12111198
case ColorSpectrumComponents.ValueSaturation:
12121199
case ColorSpectrumComponents.SaturationValue:
1200+
_hueRedBitmap?.Dispose();
12131201
_hueRedBitmap = _minBitmap;
1202+
_hueYellowBitmap?.Dispose();
12141203
_hueYellowBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraMiddle1PixelData, pixelWidth, pixelHeight);
1204+
_hueGreenBitmap?.Dispose();
12151205
_hueGreenBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraMiddle2PixelData, pixelWidth, pixelHeight);
1206+
_hueCyanBitmap?.Dispose();
12161207
_hueCyanBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraMiddle3PixelData, pixelWidth, pixelHeight);
1208+
_hueBlueBitmap?.Dispose();
12171209
_hueBlueBitmap = ColorPickerHelpers.CreateBitmapFromPixelData(bgraMiddle4PixelData, pixelWidth, pixelHeight);
1210+
_huePurpleBitmap?.Dispose();
12181211
_huePurpleBitmap = _maxBitmap;
12191212
break;
12201213
}
@@ -1249,12 +1242,12 @@ private static void FillPixelForBox(
12491242
double maxSaturation,
12501243
double minValue,
12511244
double maxValue,
1252-
ArrayList<byte> bgraMinPixelData,
1253-
ArrayList<byte> bgraMiddle1PixelData,
1254-
ArrayList<byte> bgraMiddle2PixelData,
1255-
ArrayList<byte> bgraMiddle3PixelData,
1256-
ArrayList<byte> bgraMiddle4PixelData,
1257-
ArrayList<byte> bgraMaxPixelData,
1245+
PooledList<byte> bgraMinPixelData,
1246+
PooledList<byte> bgraMiddle1PixelData,
1247+
PooledList<byte> bgraMiddle2PixelData,
1248+
PooledList<byte> bgraMiddle3PixelData,
1249+
PooledList<byte> bgraMiddle4PixelData,
1250+
PooledList<byte> bgraMaxPixelData,
12581251
List<Hsv> newHsvValues)
12591252
{
12601253
double hMin = minHue;
@@ -1409,12 +1402,12 @@ private void FillPixelForRing(
14091402
double maxSaturation,
14101403
double minValue,
14111404
double maxValue,
1412-
ArrayList<byte> bgraMinPixelData,
1413-
ArrayList<byte> bgraMiddle1PixelData,
1414-
ArrayList<byte> bgraMiddle2PixelData,
1415-
ArrayList<byte> bgraMiddle3PixelData,
1416-
ArrayList<byte> bgraMiddle4PixelData,
1417-
ArrayList<byte> bgraMaxPixelData,
1405+
PooledList<byte> bgraMinPixelData,
1406+
PooledList<byte> bgraMiddle1PixelData,
1407+
PooledList<byte> bgraMiddle2PixelData,
1408+
PooledList<byte> bgraMiddle3PixelData,
1409+
PooledList<byte> bgraMiddle4PixelData,
1410+
PooledList<byte> bgraMaxPixelData,
14181411
List<Hsv> newHsvValues)
14191412
{
14201413
double hMin = minHue;

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

Lines changed: 0 additions & 71 deletions
This file was deleted.

0 commit comments

Comments
 (0)