diff --git a/.cspell.yaml b/.cspell.yaml index bcb385a5..5da09b73 100644 --- a/.cspell.yaml +++ b/.cspell.yaml @@ -56,6 +56,7 @@ words: - FCHECK - FileAssert - fileassert + - finalizer - blittable - colorspace - formedness diff --git a/README.md b/README.md index a30d360c..bdd9e62f 100644 --- a/README.md +++ b/README.md @@ -72,23 +72,23 @@ using DemaConsulting.CanvasNet.Canvas; using DemaConsulting.CanvasNet.Codecs; using System.IO; -var surface = new Surface(4, 4); -surface[1, 1] = new Rgba32(255, 0, 0, 255); // set a red, opaque pixel -var cropped = surface.Crop(0, 0, 2, 2); // independent 2x2 copy +using var surface = new Surface(4, 4); +surface[1, 1] = new Rgba32(255, 0, 0, 255); // set a red, opaque pixel +using var cropped = surface.Crop(0, 0, 2, 2); // independent 2x2 copy BmpCodec.Save(surface, "surface.bmp"); // save as a 32-bit BMP file -var reloaded = BmpCodec.Load("surface.bmp"); // load it back +using var reloaded = BmpCodec.Load("surface.bmp"); // load it back PngCodec.Save(surface, "surface.png"); // save as an RGBA PNG file -var reloadedPng = PngCodec.Load("surface.png"); // load it back +using var reloadedPng = PngCodec.Load("surface.png"); // load it back TiffCodec.Save(surface, "surface.tiff"); // save as an RGBA TIFF file -var reloadedTiff = TiffCodec.Load("surface.tiff"); // load it back +using var reloadedTiff = TiffCodec.Load("surface.tiff"); // load it back JpegCodec.Save(surface, "surface.jpg", 90); // save as a baseline JPEG file -var reloadedJpeg = JpegCodec.Load("surface.jpg"); // load it back +using var reloadedJpeg = JpegCodec.Load("surface.jpg"); // load it back -var rasterized = SvgCodec.Load("icon.svg", 256, 256); // decode/rasterize an SVG into a 256x256 surface +using var rasterized = SvgCodec.Load("icon.svg", 256, 256); // decode/rasterize an SVG into a 256x256 surface // Triage an untrusted file's header before decoding pixel data: var info = PngCodec.GetInfo("untrusted.png"); // reads only the header, never decodes IDAT @@ -99,7 +99,7 @@ if (info.Width > Surface.MaxDimension throw new InvalidDataException("Image dimensions exceed the supported maximum."); } -var safeSurface = PngCodec.Load("untrusted.png"); // safe to decode fully +using var safeSurface = PngCodec.Load("untrusted.png"); // safe to decode fully ``` Filling a vector path onto a surface: @@ -110,7 +110,7 @@ using DemaConsulting.CanvasNet.Drawing; using DemaConsulting.CanvasNet.Geometry; using System.Numerics; -var canvas = new Surface(64, 64); +using var canvas = new Surface(64, 64); var triangle = new PathBuilder() .MoveTo(new Vector2(8, 56)) .LineTo(new Vector2(56, 56)) @@ -129,7 +129,7 @@ using DemaConsulting.CanvasNet.Drawing; using DemaConsulting.CanvasNet.Geometry; using System.Numerics; -var canvas = new Surface(64, 64); +using var canvas = new Surface(64, 64); var polyline = new PathBuilder() .MoveTo(new Vector2(8, 48)) .LineTo(new Vector2(32, 16)) @@ -154,7 +154,7 @@ using DemaConsulting.CanvasNet.Drawing; using DemaConsulting.CanvasNet.Geometry; using System.Numerics; -var canvas = new Surface(64, 64); +using var canvas = new Surface(64, 64); var rectangle = new PathBuilder() .MoveTo(new Vector2(4, 4)) .LineTo(new Vector2(60, 4)) @@ -221,7 +221,7 @@ var glyphOutline = font.GetGlyphOutline(glyphIndex); // raw font-design-unit coo var scale = 48f / font.UnitsPerEm; var canvasOutline = TransformGlyph(glyphOutline, scale, baselineY: 56f); -var surface = new Surface(64, 64); +using var surface = new Surface(64, 64); PathFiller.Fill(surface, canvasOutline, new Rgba32(20, 120, 255, 255)); ``` diff --git a/docs/design/canvas-net/canvas/surface.md b/docs/design/canvas-net/canvas/surface.md index 09ecb023..96ade165 100644 --- a/docs/design/canvas-net/canvas/surface.md +++ b/docs/design/canvas-net/canvas/surface.md @@ -28,6 +28,7 @@ section below). | `_buffer` | `byte[]` | Contiguous, row-major pixel storage, sized `Height * _strideBytes`. | | `_strideBytes` | `int` | The physical byte size of one row, including trailing padding. | | `BytesPerPixel` | `const int` | The number of bytes per pixel (always 4: R, G, B, A). | +| `_disposed` | `bool` | Set once by `Dispose()`; guarded members then throw. | `MaxDimension` is `public` (not merely internal) so that callers can compare a probed image's declared dimensions — for example, a codec's `GetInfo(Stream)`/`GetInfo(string)` result — against @@ -208,6 +209,7 @@ Results are converted back to bytes with round-half-away-from-zero, clamped to ` - `ArgumentNullException` — when `foreground` is `null` - `ArgumentException` — when `foreground.Width != Width` or `foreground.Height != Height` +- `ObjectDisposedException` — when this surface, or `foreground`, has been disposed #### CompositeOver(Rgba32 color) @@ -328,6 +330,48 @@ no conditional-select primitive. Since `netstandard2.0` support has been dropped single, unconditional code path shared by `net8.0`, `net9.0`, and `net10.0` — no `#if` target-framework gating is required. +#### Dispose() + +Releases the resources held by this `Surface` and implements `IDisposable`. This method is +idempotent: a second (or subsequent) call has no additional effect, guarded by the `_disposed` +flag. This differs from the existing `RowChannelBuffers`/`CompositeWorkBuffers` internal helper +types elsewhere in this same file, which rent-and-return `ArrayPool` arrays but do **not** +guard `Dispose()` with a flag - callers must not call their `Dispose()` more than once, since a +second call would return the same rented arrays a second time. `Surface.Dispose()` intentionally +does not follow that shape, since `IDisposable.Dispose()` is conventionally expected to tolerate +repeated calls. + +In this release, `_buffer` is a plain managed `byte[]`, not rented from an `ArrayPool`, so +there is nothing for `Dispose()` to actually release yet — it exists purely to establish the +disposal contract _before_ the pixel-storage strategy changes, so that a future release can back +`_buffer` with a pooled array (returning it to the pool inside `Dispose()`) without another +breaking _API surface_ change. That claim is limited to the API surface: a future pooled +implementation must still honor the span-lifetime rule below as a design constraint, since it +cannot retroactively invalidate a span a caller obtained before `Dispose()` was called — pooling +does not get to relax that rule, only to make violating it (a stale span read/write after the +array is re-rented to another `Surface`) more expensive than it is today. Because the only +backing storage today is managed memory the garbage collector already reclaims safely on its +own, `Surface` deliberately declares no finalizer: forgetting to call `Dispose()` only forgoes a +(currently nonexistent) prompt release — it can never leak an unmanaged or pooled resource. + +After `Dispose()` has been called, every other public member that touches the pixel buffer +(the indexer, `GetRowSpanBytes`, `GetRowSpan`, `Crop`, `PremultiplyAlpha`, `UnpremultiplyAlpha`, +`Clear`, both `CompositeOver` overloads, and both public `CompositeOverSpan` overloads) throws +`ObjectDisposedException` via an `ObjectDisposedException.ThrowIf(_disposed, this)` guard as the +first statement in the member. The two `internal` workspace-reusing `CompositeOverSpan` overloads +used by `ScanlineRasterizer` carry the same guard, so a disposed surface cannot be mutated through +either the public or the internal fill path. + +**Span lifetime**: `GetRowSpanBytes`/`GetRowSpan` return a span that aliases `_buffer` directly, +and the disposal guard only runs when those methods are _called_ - it cannot revoke a span a +caller already obtained and is still holding. A caller that retains such a span across a call to +`Dispose()` and continues reading or writing through it gets undefined behavior: harmless today +(the backing array is merely unreachable through `Surface` itself), but under a future pooled-array +implementation the same array could already be rented out to, and actively used by, a different +`Surface`, so the stale-span access would corrupt unrelated pixel data. Callers must treat a span +returned by either method as invalid once `Dispose()` has been called on the same surface, and must +not retain one across a `Dispose()` call. + ### Error Handling All validation is performed at the start of the constructor, indexer, `GetRowSpanBytes`, `Crop`, @@ -338,6 +382,12 @@ entry and the resulting exception propagates directly to the caller uncaught. Th internal state to roll back because invalid arguments are rejected before any field is read or written, and before the destination surface is mutated in `Crop`. +Additionally, once `Dispose()` has been called, every public member that touches the pixel +buffer — the indexer (get and set), `GetRowSpanBytes`, `GetRowSpan`, `Crop`, `PremultiplyAlpha`, +`UnpremultiplyAlpha`, `Clear`, `CompositeOver(Surface)`, `CompositeOver(Rgba32)`, and both public +`CompositeOverSpan` overloads — throws `ObjectDisposedException` via a guard at entry, before any +of the member's own argument validation runs. + ### Dependencies `Surface` has one NuGet package dependency, `System.Numerics.Tensors` (pinned to a version diff --git a/docs/design/canvas-net/rendering/canvas.md b/docs/design/canvas-net/rendering/canvas.md index 2a330899..d20c6398 100644 --- a/docs/design/canvas-net/rendering/canvas.md +++ b/docs/design/canvas-net/rendering/canvas.md @@ -34,6 +34,15 @@ gradient-paint `FillPath` overload likewise skips `Gradient.WithTransform` and p `Gradient` reference straight through. This preserves the exact bit pattern of pre-existing renders when callers adopt the wrapper. +#### Surface Ownership + +`Canvas` holds a non-owning reference to the `Canvas.Surface` passed to its constructor: it never +calls `Surface.Dispose()` and does not itself implement `IDisposable`. The caller that +constructed the `Surface` remains solely responsible for disposing it, once both the `Canvas` and +the `Surface` are no longer needed. This is a deliberate design choice — `Canvas` is a thin, +transform-aware wrapper around a `Surface` a caller already owns, not an owner of that surface's +lifetime. + #### Validation - The constructor throws `ArgumentNullException` on a null `Surface`. diff --git a/docs/reqstream/canvas-net.yaml b/docs/reqstream/canvas-net.yaml index 11253e91..8445be78 100644 --- a/docs/reqstream/canvas-net.yaml +++ b/docs/reqstream/canvas-net.yaml @@ -158,6 +158,20 @@ sections: - CanvasNet_SystemIntegration_ClearSurfaceThenReadPixel_ReturnsExpectedColor - CanvasNet_SystemIntegration_CanvasClearThenReadWrappedSurfacePixel_ReturnsExpectedColor + - id: CanvasNet-Lib-CanvasDisposal + title: >- + The library shall provide explicit resource-lifetime management for a pixel buffer, + logically releasing it on demand and rejecting further use of a released buffer. + tags: [system] + justification: | + Reserving the option to back the pixel buffer with a pooled (ArrayPool-rented) array in + a future release - without another breaking API change - requires an explicit disposal + contract to exist on the buffer type from the start. + children: + - CanvasNet-Canvas-Disposal + tests: + - CanvasNet_SystemIntegration_DisposeSurfaceThenUseIt_ThrowsObjectDisposedException + - id: CanvasNet-Lib-CanvasMaxDimensionPublic title: >- The library shall expose the maximum permitted pixel-buffer dimension as a public diff --git a/docs/reqstream/canvas-net/canvas.yaml b/docs/reqstream/canvas-net/canvas.yaml index 9548604f..5e8105e9 100644 --- a/docs/reqstream/canvas-net/canvas.yaml +++ b/docs/reqstream/canvas-net/canvas.yaml @@ -395,3 +395,33 @@ sections: tests: - Rgba32_Parse_6HexUppercase_ReturnsExpectedRgbaWithAlpha255 - Rgba32_TryParse_ValidInput_ReturnsTrueAndSetsResult + + - id: CanvasNet-Canvas-Disposal + title: >- + The Canvas subsystem shall provide explicit resource-lifetime management for its + pixel buffer, logically releasing it on demand and rejecting further use once + released. + justification: | + Reserving the option to back the pixel buffer with a pooled (ArrayPool-rented) + array in a future release - without another breaking API change - requires an + explicit disposal contract to exist on the buffer type from the start. + children: + - CanvasNet-Canvas-Surface-Disposal + tests: + - Surface_Dispose_CalledOnce_DoesNotThrow + - Surface_Dispose_CalledTwice_DoesNotThrow + - Surface_IndexerGet_AfterDispose_ThrowsObjectDisposedException + - Surface_IndexerSet_AfterDispose_ThrowsObjectDisposedException + - Surface_GetRowSpanBytes_AfterDispose_ThrowsObjectDisposedException + - Surface_GetRowSpan_AfterDispose_ThrowsObjectDisposedException + - Surface_Crop_AfterDispose_ThrowsObjectDisposedException + - Surface_PremultiplyAlpha_AfterDispose_ThrowsObjectDisposedException + - Surface_UnpremultiplyAlpha_AfterDispose_ThrowsObjectDisposedException + - Surface_CompositeOverSurface_AfterDispose_ThrowsObjectDisposedException + - Surface_CompositeOverSurface_ForegroundDisposed_ThrowsObjectDisposedException + - Surface_CompositeOverColor_AfterDispose_ThrowsObjectDisposedException + - Surface_CompositeOverSpan_AfterDispose_ThrowsObjectDisposedException + - Surface_CompositeOverSpanPerPixelColors_AfterDispose_ThrowsObjectDisposedException + - Surface_CompositeOverSpanWithWorkspace_AfterDispose_ThrowsObjectDisposedException + - Surface_CompositeOverSpanWithWorkspacePerPixelColors_AfterDispose_ThrowsObjectDisposedException + - Surface_Clear_AfterDispose_ThrowsObjectDisposedException diff --git a/docs/reqstream/canvas-net/canvas/surface.yaml b/docs/reqstream/canvas-net/canvas/surface.yaml index 7ba04660..c2cd5ec6 100644 --- a/docs/reqstream/canvas-net/canvas/surface.yaml +++ b/docs/reqstream/canvas-net/canvas/surface.yaml @@ -351,3 +351,35 @@ sections: - Surface_CompositeOverSpan_PerPixelColors_ZeroCoverage_LeavesBackgroundUnchanged - Surface_CompositeOverSpan_PerPixelColors_LengthMismatch_ThrowsArgumentException - Surface_CompositeOverSpanWithWorkspace_PerPixelColors_MatchesPublicOverload + + - id: CanvasNet-Canvas-Surface-Disposal + title: >- + The Surface class shall implement IDisposable, logically releasing its + pixel-buffer on Dispose so that it can no longer be used, tolerating a second + call to Dispose without throwing, and rejecting any further use of a disposed + instance's buffer-touching members with ObjectDisposedException. + justification: | + Surface currently owns a plain managed byte array, but a future non-breaking + change may back it with an ArrayPool-rented array to reduce allocation pressure + in hot rendering/codec paths. Adding the IDisposable contract now - even before + any pooling is implemented - lets that optimization land later without another + breaking API change, and gives callers a single, consistent lifetime-management + pattern regardless of the internal storage strategy in effect at any given time. + tests: + - Surface_Dispose_CalledOnce_DoesNotThrow + - Surface_Dispose_CalledTwice_DoesNotThrow + - Surface_IndexerGet_AfterDispose_ThrowsObjectDisposedException + - Surface_IndexerSet_AfterDispose_ThrowsObjectDisposedException + - Surface_GetRowSpanBytes_AfterDispose_ThrowsObjectDisposedException + - Surface_GetRowSpan_AfterDispose_ThrowsObjectDisposedException + - Surface_Crop_AfterDispose_ThrowsObjectDisposedException + - Surface_PremultiplyAlpha_AfterDispose_ThrowsObjectDisposedException + - Surface_UnpremultiplyAlpha_AfterDispose_ThrowsObjectDisposedException + - Surface_CompositeOverSurface_AfterDispose_ThrowsObjectDisposedException + - Surface_CompositeOverSurface_ForegroundDisposed_ThrowsObjectDisposedException + - Surface_CompositeOverColor_AfterDispose_ThrowsObjectDisposedException + - Surface_CompositeOverSpan_AfterDispose_ThrowsObjectDisposedException + - Surface_CompositeOverSpanPerPixelColors_AfterDispose_ThrowsObjectDisposedException + - Surface_CompositeOverSpanWithWorkspace_AfterDispose_ThrowsObjectDisposedException + - Surface_CompositeOverSpanWithWorkspacePerPixelColors_AfterDispose_ThrowsObjectDisposedException + - Surface_Clear_AfterDispose_ThrowsObjectDisposedException diff --git a/docs/sysml2/model/canvas-net/canvas/surface.sysml b/docs/sysml2/model/canvas-net/canvas/surface.sysml index a3fb5487..7f3f8b8e 100644 --- a/docs/sysml2/model/canvas-net/canvas/surface.sysml +++ b/docs/sysml2/model/canvas-net/canvas/surface.sysml @@ -1,6 +1,6 @@ package CanvasNet { part def Surface { - doc /* Mutable, in-memory 32-bit RGBA pixel buffer providing span-based row access, independent-copy cropping, and vectorized bulk pixel operations (alpha premultiplication and Porter-Duff "over" compositing). */ + doc /* Mutable, in-memory 32-bit RGBA pixel buffer providing span-based row access, independent-copy cropping, and vectorized bulk pixel operations (alpha premultiplication and Porter-Duff "over" compositing). Implements IDisposable to establish the disposal contract for its pixel buffer ahead of a future ArrayPool-backed storage change. */ comment sourceRef /* Source: src/DemaConsulting.CanvasNet/Canvas/Surface.cs, src/DemaConsulting.CanvasNet/Canvas/Rgba32.cs */ comment testRef /* Test: test/DemaConsulting.CanvasNet.Tests/Canvas/SurfaceTests.cs */ diff --git a/docs/user_guide/introduction.md b/docs/user_guide/introduction.md index 48901a9d..468cf5f0 100644 --- a/docs/user_guide/introduction.md +++ b/docs/user_guide/introduction.md @@ -49,7 +49,7 @@ dotnet add package DemaConsulting.CanvasNet using DemaConsulting.CanvasNet.Canvas; using DemaConsulting.CanvasNet.Codecs; -var surface = new Surface(4, 4); +using var surface = new Surface(4, 4); surface[0, 0] = new Rgba32(255, 0, 0, 255); // opaque red pixel Console.WriteLine(surface[0, 0].R); // Output: 255 ``` @@ -83,6 +83,28 @@ is fully transparent (all channels zero) until pixels are explicitly set. - `ArgumentOutOfRangeException`: Thrown when `width` or `height` is less than or equal to zero, or greater than 8192. +#### Disposal + +`Surface` implements `IDisposable`. In this release its pixel buffer is a plain managed array, so +`Dispose()` has nothing to actually release yet — it exists to establish the disposal contract +ahead of a future release that may back the buffer with a pooled array, without another breaking +API change. Call `Dispose()` (or wrap construction in a `using`/`using var` statement) once a +`Surface` is no longer needed; calling `Dispose()` more than once is safe and has no additional +effect. + +```csharp +public void Dispose() +``` + +After `Dispose()` has been called, every other public member that touches the pixel buffer (the +indexer, `GetRowSpanBytes`, `GetRowSpan`, `Crop`, `PremultiplyAlpha`, `UnpremultiplyAlpha`, +`Clear`, `CompositeOver`, and `CompositeOverSpan`) throws `ObjectDisposedException`. + +**Exceptions:** + +- `ObjectDisposedException`: Thrown by any other public buffer-touching member once this + `Surface` has been disposed. + #### Surface Properties ##### Width @@ -1170,7 +1192,7 @@ explicitly-supplied value - including the all-zero matrix - is preserved exactly using DemaConsulting.CanvasNet.Canvas; using DemaConsulting.CanvasNet.Codecs; -var surface = new Surface(4, 4); +using var surface = new Surface(4, 4); surface[1, 1] = new Rgba32(255, 0, 0, 255); // opaque red pixel var pixel = surface[1, 1]; Console.WriteLine(pixel.R); // Output: 255 @@ -1182,10 +1204,10 @@ Console.WriteLine(pixel.R); // Output: 255 using DemaConsulting.CanvasNet.Canvas; using DemaConsulting.CanvasNet.Codecs; -var surface = new Surface(4, 4); +using var surface = new Surface(4, 4); surface[1, 1] = new Rgba32(0, 255, 0, 255); // opaque green pixel -var cropped = surface.Crop(1, 1, 2, 2); +using var cropped = surface.Crop(1, 1, 2, 2); Console.WriteLine(cropped.Width); // Output: 2 Console.WriteLine(cropped.Height); // Output: 2 Console.WriteLine(cropped[0, 0].G); // Output: 255 @@ -1197,17 +1219,17 @@ Console.WriteLine(cropped[0, 0].G); // Output: 255 using DemaConsulting.CanvasNet.Canvas; using DemaConsulting.CanvasNet.Codecs; -var surface = new Surface(2, 2); +using var surface = new Surface(2, 2); surface[0, 0] = new Rgba32(255, 0, 0, 128); // semi-transparent red pixel // Save with alpha preserved (Bit32, the default) BmpCodec.Save(surface, "surface.bmp"); -var loaded = BmpCodec.Load("surface.bmp"); +using var loaded = BmpCodec.Load("surface.bmp"); Console.WriteLine(loaded[0, 0].A); // Output: 128 // Save without alpha (Bit24) - reloading always yields opaque pixels BmpCodec.Save(surface, "surface24.bmp", BmpBitDepth.Bit24); -var loaded24 = BmpCodec.Load("surface24.bmp"); +using var loaded24 = BmpCodec.Load("surface24.bmp"); Console.WriteLine(loaded24[0, 0].A); // Output: 255 ``` @@ -1217,17 +1239,17 @@ Console.WriteLine(loaded24[0, 0].A); // Output: 255 using DemaConsulting.CanvasNet.Canvas; using DemaConsulting.CanvasNet.Codecs; -var surface = new Surface(2, 2); +using var surface = new Surface(2, 2); surface[0, 0] = new Rgba32(0, 255, 0, 128); // semi-transparent green pixel // Save with alpha preserved (Rgba, the default) PngCodec.Save(surface, "surface.png"); -var loaded = PngCodec.Load("surface.png"); +using var loaded = PngCodec.Load("surface.png"); Console.WriteLine(loaded[0, 0].A); // Output: 128 // Save without alpha (Rgb) - reloading always yields opaque pixels PngCodec.Save(surface, "surface-rgb.png", PngColorType.Rgb); -var loadedRgb = PngCodec.Load("surface-rgb.png"); +using var loadedRgb = PngCodec.Load("surface-rgb.png"); Console.WriteLine(loadedRgb[0, 0].A); // Output: 255 ``` @@ -1237,17 +1259,17 @@ Console.WriteLine(loadedRgb[0, 0].A); // Output: 255 using DemaConsulting.CanvasNet.Canvas; using DemaConsulting.CanvasNet.Codecs; -var surface = new Surface(2, 2); +using var surface = new Surface(2, 2); surface[0, 0] = new Rgba32(0, 0, 255, 128); // semi-transparent blue pixel // Save uncompressed (the default) TiffCodec.Save(surface, "surface.tiff"); -var loaded = TiffCodec.Load("surface.tiff"); +using var loaded = TiffCodec.Load("surface.tiff"); Console.WriteLine(loaded[0, 0].A); // Output: 128 // Save with LZW compression and an automatic horizontal-differencing predictor TiffCodec.Save(surface, "surface-lzw.tiff", TiffCompression.Lzw); -var loadedLzw = TiffCodec.Load("surface-lzw.tiff"); +using var loadedLzw = TiffCodec.Load("surface-lzw.tiff"); Console.WriteLine(loadedLzw[0, 0].B); // Output: 255 ``` @@ -1257,12 +1279,12 @@ Console.WriteLine(loadedLzw[0, 0].B); // Output: 255 using DemaConsulting.CanvasNet.Canvas; using DemaConsulting.CanvasNet.Codecs; -var surface = new Surface(16, 16); +using var surface = new Surface(16, 16); surface[0, 0] = new Rgba32(255, 128, 0, 255); // opaque orange pixel // Save at quality 90 (the default) JpegCodec.Save(surface, "surface.jpg", 90); -var loaded = JpegCodec.Load("surface.jpg"); +using var loaded = JpegCodec.Load("surface.jpg"); Console.WriteLine(loaded[0, 0].A); // Output: 255 // JPEG is lossy, so compare color channels with a tolerance rather than exact equality @@ -1274,7 +1296,7 @@ Console.WriteLine(Math.Abs(loaded[0, 0].R - surface[0, 0].R) <= 15); // Output: ```csharp using DemaConsulting.CanvasNet.Canvas; -var background = new Surface(1, 1); +using var background = new Surface(1, 1); background[0, 0] = new Rgba32(0, 255, 0, 255); // opaque green background // Composite a semi-transparent red overlay over the background, in place @@ -1307,7 +1329,7 @@ if (info.Width > Surface.MaxDimension || info.Height > Surface.MaxDimension || p } // Only decode pixel data once the header has been judged safe. -var surface = PngCodec.Load("untrusted.png"); +using var surface = PngCodec.Load("untrusted.png"); Console.WriteLine($"{surface.Width}x{surface.Height}, alpha: {info.HasAlpha}"); ``` @@ -1319,7 +1341,7 @@ using DemaConsulting.CanvasNet.Drawing; using DemaConsulting.CanvasNet.Geometry; using System.Numerics; -var canvas = new Surface(64, 64); +using var canvas = new Surface(64, 64); var triangle = new PathBuilder() .MoveTo(new Vector2(8, 56)) .LineTo(new Vector2(56, 56)) @@ -1340,7 +1362,7 @@ using DemaConsulting.CanvasNet.Drawing; using DemaConsulting.CanvasNet.Geometry; using System.Numerics; -var canvas = new Surface(64, 64); +using var canvas = new Surface(64, 64); var polyline = new PathBuilder() .MoveTo(new Vector2(8, 48)) .LineTo(new Vector2(32, 16)) @@ -1366,7 +1388,7 @@ using DemaConsulting.CanvasNet.Drawing; using DemaConsulting.CanvasNet.Geometry; using System.Numerics; -var canvas = new Surface(64, 64); +using var canvas = new Surface(64, 64); var square = new PathBuilder() .MoveTo(new Vector2(4, 4)) .LineTo(new Vector2(60, 4)) @@ -1437,7 +1459,7 @@ var glyphOutline = font.GetGlyphOutline(glyphIndex); var scale = 48f / font.UnitsPerEm; var canvasOutline = TransformGlyph(glyphOutline, scale, baselineY: 56f); -var surface = new Surface(64, 64); +using var surface = new Surface(64, 64); PathFiller.Fill(surface, canvasOutline, new Rgba32(20, 120, 255, 255)); Console.WriteLine(font.GetAdvanceWidth(glyphIndex)); ``` @@ -1463,7 +1485,7 @@ Console.WriteLine($"{info.Width}x{info.Height}"); // Output: 100x100 // Rasterize the document into a caller-chosen 64x64 surface (SvgCodec is decode-only). using var loadStream = new MemoryStream(Encoding.UTF8.GetBytes(svg)); -var surfaceSvg = SvgCodec.Load(loadStream, 64, 64); +using var surfaceSvg = SvgCodec.Load(loadStream, 64, 64); Console.WriteLine(surfaceSvg[32, 32].A); // Output: 255 (well inside the filled rectangle) ``` @@ -1479,7 +1501,7 @@ using DemaConsulting.CanvasNet.Canvas; using DemaConsulting.CanvasNet.Rendering; using System.Numerics; -var surface = new Surface(200, 200); +using var surface = new Surface(200, 200); var canvas = new Canvas(surface); canvas.Save(); diff --git a/docs/verification/canvas-net.md b/docs/verification/canvas-net.md index 1d83a1e6..00a4ade1 100644 --- a/docs/verification/canvas-net.md +++ b/docs/verification/canvas-net.md @@ -282,6 +282,17 @@ data was set — exactly matches the clear color, confirming that a regression i `Canvas.Clear` -> `Surface.Clear` passthrough would be caught at the system level even if it were otherwise masked by `Rendering` unit-level test isolation. +### Integration: Dispose Surface Then Use It Throws ObjectDisposedException + +**Test**: `CanvasNet_SystemIntegration_DisposeSurfaceThenUseIt_ThrowsObjectDisposedException` + +Exercises end-to-end system behavior for the `Surface` unit's disposal contract: constructs a +`Surface` through the public API, sets a pixel, then calls `Dispose()` through the public API. +Asserts calling `Dispose()` a second time remains safe (idempotent), and that a representative +sample of public buffer-touching members (the indexer, `GetRowSpan`, `Clear`, `Crop`) all reject +further use with `ObjectDisposedException`, confirming the system-level disposal guarantee - not +merely the individual unit-level guard on any single member. + ### Integration: TIFF GetInfo on Non-Seekable Stream Returns Expected Info **Test**: `CanvasNet_SystemIntegration_TiffGetInfoOnNonSeekableStream_ReturnsExpectedInfo` diff --git a/docs/verification/canvas-net/canvas/surface.md b/docs/verification/canvas-net/canvas/surface.md index 36a68f0b..dccd5001 100644 --- a/docs/verification/canvas-net/canvas/surface.md +++ b/docs/verification/canvas-net/canvas/surface.md @@ -359,6 +359,36 @@ applied at full coverage, that zero coverage leaves the background unchanged, th `colors`/`coverage` length mismatch throws `ArgumentException`, and that the internal workspace-reusing per-pixel-color overload matches the public overload's output exactly. +#### CanvasNet-Canvas-Surface-Disposal: Dispose Is Idempotent and Blocks Further Use + +**Tests**: `Surface_Dispose_CalledOnce_DoesNotThrow`, `Surface_Dispose_CalledTwice_DoesNotThrow`, +`Surface_IndexerGet_AfterDispose_ThrowsObjectDisposedException`, +`Surface_IndexerSet_AfterDispose_ThrowsObjectDisposedException`, +`Surface_GetRowSpanBytes_AfterDispose_ThrowsObjectDisposedException`, +`Surface_GetRowSpan_AfterDispose_ThrowsObjectDisposedException`, +`Surface_Crop_AfterDispose_ThrowsObjectDisposedException`, +`Surface_PremultiplyAlpha_AfterDispose_ThrowsObjectDisposedException`, +`Surface_UnpremultiplyAlpha_AfterDispose_ThrowsObjectDisposedException`, +`Surface_CompositeOverSurface_AfterDispose_ThrowsObjectDisposedException`, +`Surface_CompositeOverSurface_ForegroundDisposed_ThrowsObjectDisposedException`, +`Surface_CompositeOverColor_AfterDispose_ThrowsObjectDisposedException`, +`Surface_CompositeOverSpan_AfterDispose_ThrowsObjectDisposedException`, +`Surface_CompositeOverSpanPerPixelColors_AfterDispose_ThrowsObjectDisposedException`, +`Surface_CompositeOverSpanWithWorkspace_AfterDispose_ThrowsObjectDisposedException`, +`Surface_CompositeOverSpanWithWorkspacePerPixelColors_AfterDispose_ThrowsObjectDisposedException`, +`Surface_Clear_AfterDispose_ThrowsObjectDisposedException` + +Confirms that calling `Dispose()` once does not throw, and that calling it a second time is also +safe (idempotent). Separately confirms that once a `Surface` has been disposed, every public +buffer-touching member — the indexer (both get and set), `GetRowSpanBytes`, `GetRowSpan`, `Crop`, +`PremultiplyAlpha`, `UnpremultiplyAlpha`, `Clear`, both `CompositeOver` overloads, and both public +`CompositeOverSpan` overloads — rejects further use by throwing `ObjectDisposedException`. The +same guard is also confirmed on the two `internal` workspace-reusing `CompositeOverSpan` +overloads used by `ScanlineRasterizer`'s fill path, so a disposed surface cannot be mutated +through either the public or the internal fill entry point. +`CompositeOver(Surface)` additionally rejects a disposed `foreground` argument, even when this +surface itself has not been disposed. + #### Rgba32 Sanity Checks (no requirement link) **Tests**: `Rgba32_FieldAssignment_StoresChannelValues`, `Rgba32_Equals_SameChannelValues_ReturnsTrue` diff --git a/docs/verification/canvas-net/rendering/canvas.md b/docs/verification/canvas-net/rendering/canvas.md index d59845aa..b0a9bc1d 100644 --- a/docs/verification/canvas-net/rendering/canvas.md +++ b/docs/verification/canvas-net/rendering/canvas.md @@ -22,6 +22,11 @@ The `Canvas` unit is verified by `test/DemaConsulting.CanvasNet.Tests/Rendering/ verifies pixels shift by the translation applied. - **Validation**: null-surface, null-path, null-gradient, null-style, and invalid fill-rule tests cover the validation contract. +- **Surface ownership (no requirement link)**: + `Canvas_Surface_DisposedExternally_CanvasDoesNotThrowFromAccessingSurfaceProperty` is a + design-intent regression test confirming `Canvas` holds a non-owning reference to its wrapped + `Surface` — it disposes the `Surface` externally, then confirms accessing `Canvas.Surface` + itself does not throw, proving `Canvas` adds no ownership check of its own. ### Traceability diff --git a/src/DemaConsulting.CanvasNet/Canvas/Surface.cs b/src/DemaConsulting.CanvasNet/Canvas/Surface.cs index cd23a1d6..ce9b2186 100644 --- a/src/DemaConsulting.CanvasNet/Canvas/Surface.cs +++ b/src/DemaConsulting.CanvasNet/Canvas/Surface.cs @@ -22,8 +22,20 @@ namespace DemaConsulting.CanvasNet.Canvas; /// always return exactly Width-length data - the padding bytes are never observable /// through any public member. /// +/// Surface implements so that its pixel-storage strategy can +/// be changed in a future release (for example, to an -rented +/// array) without another breaking API change. In this release the internal buffer is a +/// plain managed array, not rented from a pool, so +/// has nothing to actually release yet; it exists purely to establish the disposal contract +/// early. Because the only backing storage is managed memory that the garbage collector +/// already reclaims safely on its own, Surface deliberately declares no finalizer - +/// forgetting to call only forgoes the (currently nonexistent) prompt +/// release, it can never leak an unmanaged or pooled resource. Calling +/// is idempotent (safe to call more than once), and every other public member that touches +/// the pixel buffer throws once the instance has been +/// disposed. /// -public sealed class Surface +public sealed class Surface : IDisposable { /// /// The number of bytes used to store a single pixel (one byte per RGBA channel). @@ -84,6 +96,13 @@ public sealed class Surface /// private readonly byte[] _buffer; + /// + /// Set once has been called; every other public member that touches + /// checks this flag first via 's + /// ThrowIf helper and rejects further use once it is . + /// + private bool _disposed; + /// /// Initializes a new instance of the class with the specified /// dimensions, fully transparent (all pixel bytes zero). @@ -155,6 +174,28 @@ public Surface(int width, int height) _buffer = new byte[height * _strideBytes]; } + /// + /// Releases the resources held by this . + /// + /// + /// Idempotent: calling this method more than once has no additional effect. In this + /// release is a plain managed array, not rented from a pool, so + /// there is nothing to actually release yet - this method exists to establish the + /// disposal contract before a future release backs with an + /// -rented array (at which point this method would return that + /// array to the pool here). After this method has been called, every other public member + /// that touches the pixel buffer throws . + /// + public void Dispose() + { + if (_disposed) + { + return; + } + + _disposed = true; + } + /// /// Gets the width of the surface, in pixels. /// @@ -175,6 +216,7 @@ public Surface(int width, int height) /// Thrown when is outside [0, Width) or is /// outside [0, Height). /// + /// Thrown when this surface has been disposed. /// /// This indexer is a convenience for single-pixel access and is slower than working /// directly with or , because each @@ -186,6 +228,8 @@ public Surface(int width, int height) { get { + ObjectDisposedException.ThrowIf(_disposed, this); + var row = GetRowSpan(y); if (x < 0 || x >= Width) { @@ -196,6 +240,8 @@ public Surface(int width, int height) } set { + ObjectDisposedException.ThrowIf(_disposed, this); + var row = GetRowSpan(y); if (x < 0 || x >= Width) { @@ -218,6 +264,7 @@ public Surface(int width, int height) /// /// Thrown when is outside [0, Height). /// + /// Thrown when this surface has been disposed. /// /// The returned span aliases this surface's internal buffer directly - no data is copied, /// so writes through the span are immediately visible through the indexer and vice versa. @@ -225,9 +272,22 @@ public Surface(int width, int height) /// internal row padding (see the class-level remarks): any padding bytes physically /// stored beyond the row's Width * 4 pixel bytes are never included in, or /// observable through, the returned span. + /// + /// Span lifetime: the returned span is only valid until this surface is + /// disposed. A caller that retains the span and continues to read or write through it + /// after calling gets undefined behavior - today that means + /// reading or writing the backing array after it has merely been marked unavailable + /// (harmless but unsupported); under a future pooled-array implementation, the same + /// array could already have been rented out to, and be actively used by, a different + /// , so the same stale-span access would corrupt unrelated pixel + /// data. Callers must not retain a span returned by this method past a call to + /// on the same surface. + /// /// public Span GetRowSpanBytes(int y) { + ObjectDisposedException.ThrowIf(_disposed, this); + if (y < 0 || y >= Height) { throw new ArgumentOutOfRangeException(nameof(y), y, "Y must be within the surface height."); @@ -248,14 +308,23 @@ public Span GetRowSpanBytes(int y) /// /// Thrown when is outside [0, Height). /// + /// Thrown when this surface has been disposed. /// /// Uses 's span-reinterpretation cast to reinterpret the raw /// byte row returned by as values /// without copying any data, so writes through the returned span are immediately visible /// through the indexer and , and vice versa. + /// + /// Span lifetime: see - the same rule applies + /// here, since the returned span aliases the same underlying storage. Callers must + /// not retain a span returned by this method past a call to on + /// the same surface. + /// /// public Span GetRowSpan(int y) { + ObjectDisposedException.ThrowIf(_disposed, this); + return MemoryMarshal.Cast(GetRowSpanBytes(y)); } @@ -297,6 +366,7 @@ public Span GetRowSpan(int y) /// less than or equal to zero, x + width exceeds , or /// y + height exceeds . /// + /// Thrown when this surface has been disposed. /// /// The returned surface owns an entirely separate buffer: subsequent writes to either /// surface never affect the other. Each row of the sub-region is copied in a single @@ -316,6 +386,8 @@ public Span GetRowSpan(int y) /// public Surface Crop(int x, int y, int width, int height) { + ObjectDisposedException.ThrowIf(_disposed, this); + // Validate each argument individually so that callers get a precise parameter name // identifying exactly which value was invalid if (x < 0) @@ -381,6 +453,7 @@ public Surface Crop(int x, int y, int width, int height) /// [0, 255]; the alpha channel itself is unchanged. Every possible input byte /// pattern is valid, so this method never throws. /// + /// Thrown when this surface has been disposed. /// /// /// var surface = new Surface(1, 1); @@ -391,6 +464,8 @@ public Surface Crop(int x, int y, int width, int height) /// public void PremultiplyAlpha() { + ObjectDisposedException.ThrowIf(_disposed, this); + var pixelsPerRow = _strideBytes / BytesPerPixel; using var row = new RowChannelBuffers(pixelsPerRow); @@ -423,6 +498,7 @@ public void PremultiplyAlpha() /// itself is unchanged. Every possible input byte pattern is valid, so this method never /// throws. /// + /// Thrown when this surface has been disposed. /// /// /// var surface = new Surface(1, 1); @@ -433,6 +509,8 @@ public void PremultiplyAlpha() /// public void UnpremultiplyAlpha() { + ObjectDisposedException.ThrowIf(_disposed, this); + var pixelsPerRow = _strideBytes / BytesPerPixel; using var row = new RowChannelBuffers(pixelsPerRow); @@ -470,6 +548,9 @@ public void UnpremultiplyAlpha() /// Thrown when 's or /// does not match this surface's. /// + /// + /// Thrown when this surface, or , has been disposed. + /// /// /// Both this surface and are assumed to hold straight /// (unassociated) alpha on input, and the result is also straight alpha - callers do not @@ -482,8 +563,12 @@ public void UnpremultiplyAlpha() /// public void CompositeOver(Surface foreground) { + ObjectDisposedException.ThrowIf(_disposed, this); + ArgumentNullException.ThrowIfNull(foreground); + ObjectDisposedException.ThrowIf(foreground._disposed, foreground); + if (foreground.Width != Width || foreground.Height != Height) { throw new ArgumentException( @@ -524,6 +609,7 @@ public void CompositeOver(Surface foreground) /// into this surface in place. /// /// The constant foreground color to composite over every pixel. + /// Thrown when this surface has been disposed. /// /// Uses the same formula and rounding rule as , with /// acting as the foreground at every pixel. Every possible @@ -539,6 +625,8 @@ public void CompositeOver(Surface foreground) /// public void CompositeOver(Rgba32 color) { + ObjectDisposedException.ThrowIf(_disposed, this); + // Build a single-pixel-wide "foreground surface" whose one row is broadcast as the // constant foreground for every row of this surface - this reuses exactly the same // per-row compositing formula as CompositeOver(Surface) without duplicating it, @@ -591,8 +679,9 @@ public void CompositeOver(Rgba32 color) /// constant foreground row) and then bulk-copies that one pattern row into every row of /// the surface via , so the fill cost is one vectorized memory /// copy per row rather than a per-pixel store. Every possible value - /// is valid, so this method never throws. + /// is valid, so this method never throws for that reason. /// + /// Thrown when this surface has been disposed. /// /// /// var surface = new Surface(4, 4); @@ -601,6 +690,8 @@ public void CompositeOver(Rgba32 color) /// public void Clear(Rgba32 color) { + ObjectDisposedException.ThrowIf(_disposed, this); + var pixelsPerRow = _strideBytes / BytesPerPixel; using var pattern = new RowChannelBuffers(pixelsPerRow); @@ -643,6 +734,7 @@ public void Clear(Rgba32 color) /// Thrown when is outside [0, Height), or when /// or x + coverage.Length is outside [0, Width]. /// + /// Thrown when this surface has been disposed. /// /// This is the first partial-row compositing primitive in : every /// other CompositeOver overload always processes a full row (or the whole @@ -658,6 +750,8 @@ public void Clear(Rgba32 color) /// public void CompositeOverSpan(int y, int x, ReadOnlySpan coverage, Rgba32 color) { + ObjectDisposedException.ThrowIf(_disposed, this); + // The public entry point always rents its own scratch buffers for the single call - see // CompositeOverSpan(int, int, ReadOnlySpan, Rgba32, CompositeSpanWorkspace) for the // amortized-workspace overload used by hot loops such as @@ -704,6 +798,7 @@ public void CompositeOverSpan(int y, int x, ReadOnlySpan coverage, Rgba32 /// , or when /// coverage.Length exceeds 's capacity. /// + /// Thrown when this surface has been disposed. /// /// This overload is internal because it exposes an implementation-detail allocation /// strategy (buffer reuse), not new externally observable behavior: for a given @@ -713,6 +808,8 @@ public void CompositeOverSpan(int y, int x, ReadOnlySpan coverage, Rgba32 /// internal void CompositeOverSpan(int y, int x, ReadOnlySpan coverage, Rgba32 color, CompositeSpanWorkspace workspace) { + ObjectDisposedException.ThrowIf(_disposed, this); + ValidateCompositeOverSpanArgs(y, x, coverage.Length); ArgumentNullException.ThrowIfNull(workspace); @@ -756,6 +853,7 @@ internal void CompositeOverSpan(int y, int x, ReadOnlySpan coverage, Rgba /// Thrown when 's length does not equal 's /// length. /// + /// Thrown when this surface has been disposed. /// /// This overload exists for per-pixel-color callers - such as /// 's gradient fill @@ -768,6 +866,8 @@ internal void CompositeOverSpan(int y, int x, ReadOnlySpan coverage, Rgba /// public void CompositeOverSpan(int y, int x, ReadOnlySpan coverage, ReadOnlySpan colors) { + ObjectDisposedException.ThrowIf(_disposed, this); + ValidateCompositeOverSpanArgs(y, x, coverage.Length); if (colors.Length != coverage.Length) @@ -813,6 +913,7 @@ public void CompositeOverSpan(int y, int x, ReadOnlySpan coverage, ReadOn /// Thrown when 's length does not equal 's /// length. /// + /// Thrown when this surface has been disposed. /// /// Internal for the same reason as : /// it exposes an implementation-detail allocation strategy, not new externally observable @@ -821,6 +922,8 @@ public void CompositeOverSpan(int y, int x, ReadOnlySpan coverage, ReadOn /// internal void CompositeOverSpan(int y, int x, ReadOnlySpan coverage, ReadOnlySpan colors, CompositeSpanWorkspace workspace) { + ObjectDisposedException.ThrowIf(_disposed, this); + ValidateCompositeOverSpanArgs(y, x, coverage.Length); ArgumentNullException.ThrowIfNull(workspace); diff --git a/src/DemaConsulting.CanvasNet/Rendering/Canvas.cs b/src/DemaConsulting.CanvasNet/Rendering/Canvas.cs index a57befc8..78fe25c3 100644 --- a/src/DemaConsulting.CanvasNet/Rendering/Canvas.cs +++ b/src/DemaConsulting.CanvasNet/Rendering/Canvas.cs @@ -38,6 +38,13 @@ public sealed class Canvas private Matrix3x2 _current = Matrix3x2.Identity; /// The underlying pixel surface every fill/stroke on this Canvas writes to. + /// + /// Canvas holds a non-owning reference to this : it never + /// calls and does not itself implement + /// . The caller that constructed the passed + /// to the constructor remains solely responsible for disposing it once both this + /// Canvas and the are no longer needed. + /// public Surface Surface { get; } /// The current composed affine transform. @@ -47,7 +54,13 @@ public sealed class Canvas /// Initializes a new wrapping . Starts /// with as the current transform and an empty stack. /// - /// The pixel surface to render into. Must not be . + /// + /// The pixel surface to render into. Must not be . Ownership of + /// is not transferred to this Canvas: this constructor + /// stores only a reference to it, and the caller remains responsible for calling + /// on it once both this Canvas and the + /// are no longer needed. + /// /// Thrown when is . public Canvas(Surface surface) { diff --git a/test/DemaConsulting.CanvasNet.Tests/Canvas/SurfaceTests.cs b/test/DemaConsulting.CanvasNet.Tests/Canvas/SurfaceTests.cs index 7946076b..d8c6e1fa 100644 --- a/test/DemaConsulting.CanvasNet.Tests/Canvas/SurfaceTests.cs +++ b/test/DemaConsulting.CanvasNet.Tests/Canvas/SurfaceTests.cs @@ -1546,4 +1546,261 @@ public void Surface_CompositeOverSpanWithWorkspace_PerPixelColors_MatchesPublicO } } + /// + /// Proves that calling Dispose once on a surface does not throw. + /// + [Fact] + public void Surface_Dispose_CalledOnce_DoesNotThrow() + { + // Arrange + var surface = new Surface(2, 2); + + // Act & Assert + var exception = Record.Exception(surface.Dispose); + Assert.Null(exception); + } + + /// + /// Proves that calling Dispose a second time on an already-disposed surface does not + /// throw (Dispose is idempotent). + /// + [Fact] + public void Surface_Dispose_CalledTwice_DoesNotThrow() + { + // Arrange + var surface = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + var exception = Record.Exception(surface.Dispose); + Assert.Null(exception); + } + + /// + /// Proves that reading the indexer on a disposed surface throws ObjectDisposedException. + /// + [Fact] + public void Surface_IndexerGet_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + Assert.Throws(() => surface[0, 0]); + } + + /// + /// Proves that writing the indexer on a disposed surface throws ObjectDisposedException. + /// + [Fact] + public void Surface_IndexerSet_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + Assert.Throws(() => surface[0, 0] = new Rgba32(1, 2, 3, 4)); + } + + /// + /// Proves that calling GetRowSpanBytes on a disposed surface throws + /// ObjectDisposedException. + /// + [Fact] + public void Surface_GetRowSpanBytes_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + Assert.Throws(() => surface.GetRowSpanBytes(0)); + } + + /// + /// Proves that calling GetRowSpan on a disposed surface throws ObjectDisposedException. + /// + [Fact] + public void Surface_GetRowSpan_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + Assert.Throws(() => surface.GetRowSpan(0)); + } + + /// + /// Proves that calling Crop on a disposed surface throws ObjectDisposedException. + /// + [Fact] + public void Surface_Crop_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + Assert.Throws(() => surface.Crop(0, 0, 1, 1)); + } + + /// + /// Proves that calling PremultiplyAlpha on a disposed surface throws + /// ObjectDisposedException. + /// + [Fact] + public void Surface_PremultiplyAlpha_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + Assert.Throws(surface.PremultiplyAlpha); + } + + /// + /// Proves that calling UnpremultiplyAlpha on a disposed surface throws + /// ObjectDisposedException. + /// + [Fact] + public void Surface_UnpremultiplyAlpha_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + Assert.Throws(surface.UnpremultiplyAlpha); + } + + /// + /// Proves that calling CompositeOver(Surface) on a disposed surface throws + /// ObjectDisposedException. + /// + [Fact] + public void Surface_CompositeOverSurface_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + var foreground = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + Assert.Throws(() => surface.CompositeOver(foreground)); + } + + /// + /// Proves that calling CompositeOver(Surface) with a disposed foreground surface throws + /// ObjectDisposedException, even when this surface itself has not been disposed. + /// + [Fact] + public void Surface_CompositeOverSurface_ForegroundDisposed_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + var foreground = new Surface(2, 2); + foreground.Dispose(); + + // Act & Assert + Assert.Throws(() => surface.CompositeOver(foreground)); + } + + /// + /// Proves that calling CompositeOver(Rgba32) on a disposed surface throws + /// ObjectDisposedException. + /// + [Fact] + public void Surface_CompositeOverColor_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + Assert.Throws(() => surface.CompositeOver(new Rgba32(1, 2, 3, 4))); + } + + /// + /// Proves that calling the constant-color CompositeOverSpan overload on a disposed + /// surface throws ObjectDisposedException. + /// + [Fact] + public void Surface_CompositeOverSpan_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + Assert.Throws( + () => surface.CompositeOverSpan(0, 0, [1f], new Rgba32(1, 2, 3, 4))); + } + + /// + /// Proves that calling the per-pixel-color CompositeOverSpan overload on a disposed + /// surface throws ObjectDisposedException. + /// + [Fact] + public void Surface_CompositeOverSpanPerPixelColors_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + Assert.Throws( + () => surface.CompositeOverSpan(0, 0, [1f], (ReadOnlySpan)[new Rgba32(1, 2, 3, 4)])); + } + + /// + /// Proves that calling Clear on a disposed surface throws ObjectDisposedException. + /// + [Fact] + public void Surface_Clear_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + surface.Dispose(); + + // Act & Assert + Assert.Throws(() => surface.Clear(new Rgba32(1, 2, 3, 4))); + } + + /// + /// Proves that calling the internal workspace-reusing constant-color CompositeOverSpan + /// overload on a disposed surface throws ObjectDisposedException. + /// + [Fact] + public void Surface_CompositeOverSpanWithWorkspace_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + using var workspace = new Surface.CompositeSpanWorkspace(1); + surface.Dispose(); + + // Act & Assert + Assert.Throws( + () => surface.CompositeOverSpan(0, 0, [1f], new Rgba32(1, 2, 3, 4), workspace)); + } + + /// + /// Proves that calling the internal workspace-reusing per-pixel-color CompositeOverSpan + /// overload on a disposed surface throws ObjectDisposedException. + /// + [Fact] + public void Surface_CompositeOverSpanWithWorkspacePerPixelColors_AfterDispose_ThrowsObjectDisposedException() + { + // Arrange + var surface = new Surface(2, 2); + using var workspace = new Surface.CompositeSpanWorkspace(1); + surface.Dispose(); + + // Act & Assert + Assert.Throws( + () => surface.CompositeOverSpan(0, 0, [1f], (ReadOnlySpan)[new Rgba32(1, 2, 3, 4)], workspace)); + } } diff --git a/test/DemaConsulting.CanvasNet.Tests/CanvasNetTests.cs b/test/DemaConsulting.CanvasNet.Tests/CanvasNetTests.cs index 1b42e29c..d08782fa 100644 --- a/test/DemaConsulting.CanvasNet.Tests/CanvasNetTests.cs +++ b/test/DemaConsulting.CanvasNet.Tests/CanvasNetTests.cs @@ -121,6 +121,35 @@ public void CanvasNet_SystemIntegration_CanvasClearThenReadWrappedSurfacePixel_R Assert.Equal(clearColor, canvas.Surface[0, 0]); } + /// + /// Proves that the system's disposal contract is enforced end to end through the public + /// API: disposing a Surface is idempotent, and subsequently rejects further use through a + /// representative sample of buffer-touching public members with ObjectDisposedException. + /// The complete member-by-member matrix is covered by the unit tests in SurfaceTests. + /// + [Fact] + public void CanvasNet_SystemIntegration_DisposeSurfaceThenUseIt_ThrowsObjectDisposedException() + { + // Arrange: construct a surface through the public API and set a pixel so there is + // something meaningful for the guarded members below to have acted on, had they not + // thrown first + var surface = new Surface(2, 2); + surface[0, 0] = new Rgba32(1, 2, 3, 4); + + // Act: dispose the surface through the public API + surface.Dispose(); + + // Assert: disposing a second time remains safe (idempotent) + var secondDispose = Record.Exception(surface.Dispose); + Assert.Null(secondDispose); + + // Assert: every public buffer-touching member now rejects further use + Assert.Throws(() => surface[0, 0]); + Assert.Throws(() => surface.GetRowSpan(0)); + Assert.Throws(() => surface.Clear(new Rgba32(5, 6, 7, 8))); + Assert.Throws(() => surface.Crop(0, 0, 1, 1)); + } + /// /// Proves that the system can save a Surface to BMP and load it back through the public /// API, preserving pixel values end to end. diff --git a/test/DemaConsulting.CanvasNet.Tests/Rendering/CanvasTests.cs b/test/DemaConsulting.CanvasNet.Tests/Rendering/CanvasTests.cs index 5c6dd8af..bbbe8430 100644 --- a/test/DemaConsulting.CanvasNet.Tests/Rendering/CanvasTests.cs +++ b/test/DemaConsulting.CanvasNet.Tests/Rendering/CanvasTests.cs @@ -348,4 +348,23 @@ public void Canvas_FillPath_InvalidFillRule_ThrowsArgumentOutOfRangeException() Assert.Throws(() => canvas.FillPath(Triangle(), new Rgba32(0, 0, 0, 255), (FillRule)999)); } + /// + /// Design-intent regression test: Canvas does not own or dispose the Surface it wraps, so + /// disposing that Surface externally must not cause Canvas itself to throw when its + /// Surface property is subsequently accessed - Canvas adds no ownership check of its own. + /// (Calling a buffer-touching member on the returned, already-disposed Surface would + /// legitimately throw ObjectDisposedException from Surface - that is not exercised here.) + /// + [Fact] + public void Canvas_Surface_DisposedExternally_CanvasDoesNotThrowFromAccessingSurfaceProperty() + { + var surface = NewSurface(); + var canvas = new RenderCanvas(surface); + + surface.Dispose(); + + var exception = Record.Exception(() => _ = canvas.Surface); + Assert.Null(exception); + Assert.Same(surface, canvas.Surface); + } }