Skip to content

Commit 5146817

Browse files
author
Wayfarer
committed
DirectBitmap: UnsafeBitmap introduced & memory fixes
The `Bitmap` property has been replaced by `UnsafeBitmap` to make direct memory binding explicit. All returns now provide a copy (`new Bitmap(UnsafeBitmap)`) to avoid memory leaks and side effects. Dispose logic and resource management have been improved. Color conversion in `ColorHsv` is more robust (clamping instead of exceptions). Methods return null if errors occur. SIMD and pixel operations have been refactored. `Pixel32` is now immutable (`readonly`). Tests and helper methods consistently use `UnsafeBitmap`. The changes increase security and efficiency in the imaging framework.
1 parent ce64ea1 commit 5146817

15 files changed

Lines changed: 136 additions & 113 deletions

ImageCompare/AnalysisProcessing.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ internal static Bitmap DifferenceImage(Bitmap first, Bitmap second, Color highli
258258
}
259259
});
260260

261-
return dbmCanvas.Bitmap;
261+
return new Bitmap(dbmCanvas.UnsafeBitmap);
262262
}
263263
}
264264
}

Imaging/Cif.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ public Color GetColor(int id)
333333
dbm.SetPixel(coordinate.X, coordinate.Y, key);
334334
}
335335

336-
return dbm.Bitmap;
336+
return new Bitmap(dbm.UnsafeBitmap);
337337
}
338338

339339
/// <summary>

Imaging/CifProcessing.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ internal static Dictionary<Color, SortedSet<int>> ConvertToCifFromBitmap(Bitmap
7373
dbm.SetArea(ids, color);
7474
}
7575

76-
return dbm.Bitmap;
76+
return new Bitmap(dbm.UnsafeBitmap);
7777
}
7878

7979
/// <summary>

Imaging/ColorHsv.cs

Lines changed: 23 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public ColorHsv(byte r, byte g, byte b, byte a)
117117
}
118118

119119
/// <summary>
120-
/// Froms the RGB.
120+
/// Convert from RGB to HSV.
121121
/// </summary>
122122
/// <param name="r">The r.</param>
123123
/// <param name="g">The g.</param>
@@ -126,16 +126,20 @@ public ColorHsv(byte r, byte g, byte b, byte a)
126126
/// <returns>Color HSV object.</returns>
127127
public static ColorHsv FromRgb(int r, int g, int b, int a = 255)
128128
{
129-
ValidateRgb(r, g, b, a);
129+
// Robustness: Clamp instead of throwing
130+
r = Math.Max(0, Math.Min(255, r));
131+
g = Math.Max(0, Math.Min(255, g));
132+
b = Math.Max(0, Math.Min(255, b));
133+
a = Math.Max(0, Math.Min(255, a));
130134

131135
var hsv = new ColorHsv { R = r, G = g, B = b, A = a };
132-
133136
hsv.ToHsv();
134137
return hsv;
135138
}
136139

137140
/// <summary>
138-
/// Froms the HSV.
141+
/// Creates a ColorHsv from HSV values.
142+
/// Automatically wraps Hue and clamps S/V/A.
139143
/// </summary>
140144
/// <param name="h">The h.</param>
141145
/// <param name="s">The s.</param>
@@ -144,10 +148,22 @@ public static ColorHsv FromRgb(int r, int g, int b, int a = 255)
144148
/// <returns>Color HSV object.</returns>
145149
public static ColorHsv FromHsv(double h, double s, double v, int a = 255)
146150
{
147-
ValidateHsv(h, s, v, a);
151+
// Robustness: Handle "fuzzy" floating point math
148152

149-
var hsv = new ColorHsv { H = h, S = s, V = v, A = a };
153+
// 1. Wrap Hue (e.g. 361 becomes 1, -10 becomes 350)
154+
// This handles cases where the mouse rotation calc goes slightly over 360
155+
h = h % 360.0;
156+
if (h < 0) h += 360.0;
157+
158+
// 2. Clamp Saturation and Value to 0.0 - 1.0
159+
// Barycentric triangle math often returns -0.00001 or 1.00001 at the edges
160+
s = Math.Max(0.0, Math.Min(1.0, s));
161+
v = Math.Max(0.0, Math.Min(1.0, v));
150162

163+
// 3. Clamp Alpha
164+
a = Math.Max(0, Math.Min(255, a));
165+
166+
var hsv = new ColorHsv { H = h, S = s, V = v, A = a };
151167
hsv.ToRgb();
152168
return hsv;
153169
}
@@ -261,7 +277,7 @@ public int CompareTo(ColorHsv? other)
261277
/// <returns>
262278
/// The result of the operator.
263279
/// </returns>
264-
public static bool operator ==(ColorHsv a, ColorHsv b) => Equals(a, b);
280+
public static bool operator ==(ColorHsv? a, ColorHsv b) => Equals(a, b);
265281

266282
/// <summary>
267283
/// Implements the operator !=.
@@ -391,29 +407,5 @@ private static void ValidateRgb(int r, int g, int b, int a)
391407
if (r is < 0 or > 255 || g is < 0 or > 255 || b is < 0 or > 255 || a is < 0 or > 255)
392408
throw new ArgumentOutOfRangeException("RGB values must be 0–255");
393409
}
394-
395-
/// <summary>
396-
/// Validates the HSV.
397-
/// </summary>
398-
/// <param name="h">The h.</param>
399-
/// <param name="s">The s.</param>
400-
/// <param name="v">The v.</param>
401-
/// <param name="a">a.</param>
402-
/// <exception cref="System.ArgumentOutOfRangeException">
403-
/// Hue must be 0–360
404-
/// or
405-
/// Saturation must be 0–1
406-
/// or
407-
/// Value must be 0–1
408-
/// or
409-
/// Alpha must be 0–255
410-
/// </exception>
411-
private static void ValidateHsv(double h, double s, double v, int a)
412-
{
413-
if (h is < 0 or > 360) throw new ArgumentOutOfRangeException("Hue must be 0–360");
414-
if (s is < 0 or > 1) throw new ArgumentOutOfRangeException("Saturation must be 0–1");
415-
if (v is < 0 or > 1) throw new ArgumentOutOfRangeException("Value must be 0–1");
416-
if (a is < 0 or > 255) throw new ArgumentOutOfRangeException("Alpha must be 0–255");
417-
}
418410
}
419411
}

Imaging/DirectBitmap.cs

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public DirectBitmap(Image image)
8484
}
8585
}
8686

87-
Bitmap = new Bitmap(
87+
UnsafeBitmap = new Bitmap(
8888
Width,
8989
Height,
9090
Width * Marshal.SizeOf<Pixel32>(),
@@ -120,7 +120,7 @@ public DirectBitmap(string filePath)
120120
Height = image.Height;
121121
Initiate();
122122

123-
using var graphics = Graphics.FromImage(Bitmap);
123+
using var graphics = Graphics.FromImage(UnsafeBitmap);
124124
graphics.DrawImage(image, new Rectangle(0, 0, Width, Height), 0, 0, Width, Height, GraphicsUnit.Pixel);
125125
}
126126
catch (Exception ex)
@@ -134,11 +134,14 @@ public DirectBitmap(string filePath)
134134

135135
/// <summary>
136136
/// Gets the bitmap.
137+
/// Be careful, we pass a reference that never gets copied, so any changes to this Bitmap will affect the DirectBitmap and vice versa.
138+
/// GcHandle is pinned, so the memory address of the pixel data will not change, allowing for direct manipulation of the bitmap's pixels.
139+
/// This memory is not managed by the .NET runtime, so it is crucial to ensure that it is properly released to avoid memory leaks. Always call Dispose() when done with the DirectBitmap to free the pinned handle and associated resources.
137140
/// </summary>
138141
/// <value>
139142
/// The bitmap.
140143
/// </value>
141-
public Bitmap Bitmap { get; private set; }
144+
public Bitmap UnsafeBitmap { get; private set; }
142145

143146
/// <summary>
144147
/// Gets a value indicating whether this <see cref="DirectBitmap" /> is disposed.
@@ -154,7 +157,12 @@ public DirectBitmap(string filePath)
154157
/// <inheritdoc />
155158
public int Width { get; }
156159

157-
/// <inheritdoc />
160+
/// <summary>
161+
/// Gets or sets the bits handle.
162+
/// </summary>
163+
/// <value>
164+
/// The bits handle.
165+
/// </value>
158166
private GCHandle BitsHandle { get; set; }
159167

160168
/// <inheritdoc />
@@ -206,7 +214,7 @@ private void Initiate(Color color = default)
206214
BitsHandle = GCHandle.Alloc(Bits, GCHandleType.Pinned);
207215

208216
// Create Bitmap from pinned Pixel32 array
209-
Bitmap = new Bitmap(
217+
UnsafeBitmap = new Bitmap(
210218
Width,
211219
Height,
212220
Width * Marshal.SizeOf<Pixel32>(), // stride in bytes
@@ -230,7 +238,8 @@ public static DirectBitmap GetInstance(Bitmap btm)
230238

231239
// Lock source bits
232240
var rect = new Rectangle(0, 0, btm.Width, btm.Height);
233-
var srcData = btm.LockBits(rect, System.Drawing.Imaging.ImageLockMode.ReadOnly, PixelFormat.Format32bppArgb);
241+
var srcData = btm.LockBits(rect, System.Drawing.Imaging.ImageLockMode.ReadOnly,
242+
PixelFormat.Format32bppArgb);
234243

235244
// We can copy directly into our Bits array since it's pinned
236245
unsafe
@@ -318,7 +327,7 @@ public void DrawRectangle(int x1, int y2, int width, int height, Color color)
318327

319328
for (var x = x1; x < x1 + width && x < Width; x++)
320329
{
321-
Bits[rowStart + x] = colorPixel;
330+
Bits[rowStart + x] = colorPixel;
322331
}
323332
}
324333
}
@@ -505,7 +514,7 @@ public Span<Color> GetColors()
505514
/// <returns>BitmapImage image data</returns>
506515
public BitmapImage ToBitmapImage()
507516
{
508-
return Bitmap.ToBitmapImage();
517+
return UnsafeBitmap.ToBitmapImage();
509518
}
510519

511520
/// <summary>
@@ -583,21 +592,18 @@ private static IEnumerable<PixelData> Convert(IEnumerable<(int x, int y, Color c
583592
/// </param>
584593
private void Dispose(bool disposing)
585594
{
586-
if (Disposed)
587-
{
588-
return;
589-
}
595+
if (Disposed) return;
590596

591597
if (disposing)
592598
{
593-
// free managed resources
594-
Bitmap?.Dispose();
599+
// Managed resources (objects that implement IDisposable)
600+
UnsafeBitmap?.Dispose();
601+
}
595602

596-
// Free the GCHandle if it is allocated
597-
if (BitsHandle.IsAllocated)
598-
{
599-
BitsHandle.Free();
600-
}
603+
// Unmanaged resources/Handles (Free these always)
604+
if (BitsHandle.IsAllocated)
605+
{
606+
BitsHandle.Free();
601607
}
602608

603609
Disposed = true;

Imaging/DirectBitmapCore.cs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ internal static unsafe void SetPixelsAdaptive(
115115
/// <param name="width">The width of the image.</param>
116116
/// <param name="height">The height of the image.</param>
117117
/// <param name="pixels">Enumerable of (x, y, Pixel32) tuples to set.</param>
118-
internal static void SetPixelsSimd(Pixel32[] bits, int width, int height, IEnumerable<(int x, int y, Pixel32 color)> pixels)
118+
internal static void SetPixelsSimd(Pixel32[] bits, int width, int height,
119+
IEnumerable<(int x, int y, Pixel32 color)> pixels)
119120
{
120121
if (bits == null || pixels == null) return;
121122

@@ -262,8 +263,20 @@ internal static unsafe void BlendInt(Pixel32[] dstBits, uint[] src)
262263
uint s = *sPtr;
263264

264265
uint sa = s >> 24;
265-
if (sa == 0) { dPtr++; sPtr++; continue; }
266-
if (sa == 255) { *dPtr = s; dPtr++; sPtr++; continue; }
266+
if (sa == 0)
267+
{
268+
dPtr++;
269+
sPtr++;
270+
continue;
271+
}
272+
273+
if (sa == 255)
274+
{
275+
*dPtr = s;
276+
dPtr++;
277+
sPtr++;
278+
continue;
279+
}
267280

268281
uint d = *dPtr;
269282

@@ -293,4 +306,3 @@ internal static unsafe void BlendInt(Pixel32[] dstBits, uint[] src)
293306
}
294307
}
295308
}
296-

Imaging/DirectBitmapImage.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ public void SetPixel(int x, int y, Color color)
165165
/// Fills the bitmap with a uniform color using SIMD.
166166
/// </summary>
167167
/// <param name="color">The color to fill with.</param>
168-
public unsafe void FillSimd(Color color)
168+
public void FillSimd(Color color)
169169
{
170170
var packed = (uint)(color.A << 24 | color.R << 16 | color.G << 8 | color.B);
171171

@@ -268,11 +268,11 @@ public void UpdateBitmapFromBits()
268268
unsafe
269269
{
270270
var dst = (Pixel32*)_bitmap.BackBuffer; // treat back buffer as Pixel32[]
271-
var span = Bits.AsSpan(); // Pixel32[]
271+
var span = Bits.AsSpan(); // Pixel32[]
272272

273273
for (int i = 0; i < span.Length; i++)
274274
{
275-
dst[i] = span[i]; // direct struct copy
275+
dst[i] = span[i]; // direct struct copy
276276
}
277277
}
278278

0 commit comments

Comments
 (0)