Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 57 additions & 2 deletions lint.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,63 @@ dotnet restore > $null
if ($LASTEXITCODE -ne 0) { $lintError = $true; $skipDotnetFormat = $true }

if (-not $skipDotnetFormat) {
dotnet format --verify-no-changes --no-restore
if ($LASTEXITCODE -ne 0) { $lintError = $true }
# NOTE: `dotnet format --verify-no-changes` has a known cross-platform exit-code
# bug (e.g. dotnet/sdk#41422) where it can report a non-zero exit code even when
# zero files require formatting, inconsistently between operating systems. To get
# a reliable check, format in place and use `git diff` as the source of truth.
#
# A plain `git diff` after formatting would also include any pre-existing
# uncommitted *.cs changes unrelated to formatting (e.g. a developer's
# in-progress edits), causing false failures. Stash those away first so the
# formatter runs against a clean tree and the diff reflects only its own
# changes, then restore the original working tree afterward regardless of
# outcome, leaving no side effects from running this check.
#
# `git status --porcelain` (rather than `git diff`) is used to detect
# pre-existing changes so that untracked *.cs files are also caught and
# stashed - otherwise an untracked file reformatted by `dotnet format`
# would be left modified with no diff ever having been checked against it.
# `--index` on the pop restores the original staged/unstaged split rather
# than flattening everything into the working tree.
$statusBeforeFormat = git status --porcelain -- '*.cs'
$hasPreexistingCsChanges = [bool]$statusBeforeFormat
if ($hasPreexistingCsChanges) {
git stash push --include-untracked --quiet --message "lint.ps1: pre-existing *.cs changes" -- '*.cs'
if ($LASTEXITCODE -ne 0) {
$lintError = $true
$skipDotnetFormat = $true
Write-Host "Failed to stash pre-existing *.cs changes; skipping dotnet format check."
$hasPreexistingCsChanges = $false
}
}
}

if (-not $skipDotnetFormat) {
dotnet format --no-restore
if ($LASTEXITCODE -ne 0) {
$lintError = $true
}
else {
git diff --exit-code -- '*.cs'
if ($LASTEXITCODE -ne 0) {
Comment thread
Copilot marked this conversation as resolved.
$lintError = $true
Write-Host "dotnet format made changes; run fix.ps1 locally and commit the results."
}
}

# Restore the working tree to its original state: discard any in-place
# formatting changes made purely for this check, then reapply the
# developer's original uncommitted *.cs changes (if any were stashed).
# `--index` restores the original staged/unstaged split rather than
# dropping everything into the working tree unstaged.
git checkout -- '*.cs' 2>$null
if ($hasPreexistingCsChanges) {
git stash pop --index --quiet
if ($LASTEXITCODE -ne 0) {
$lintError = $true
Write-Host "Failed to restore stashed *.cs changes; check 'git stash list' and resolve manually."
}
}
}

# [PROJECT-SPECIFIC] Add additional format verification checks here.
Expand Down
9 changes: 7 additions & 2 deletions src/DemaConsulting.CanvasNet/Canvas/Rgba32.cs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,11 @@ private static bool TryParseCore(string s, out Rgba32 result, out string? reason
}

/// <summary>Returns whether <paramref name="c"/> is a valid ASCII hexadecimal digit.</summary>
private static bool IsHexDigit(char c) =>
(c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F');
/// <remarks>
/// Delegates to <see cref="char.IsAsciiHexDigit"/> rather than a hand-written combination
/// of range comparisons - it is exactly equivalent (true for <c>'0'-'9'</c>,
/// <c>'a'-'f'</c>, and <c>'A'-'F'</c>) while avoiding a hard-to-scan, multi-operator
/// boolean expression.
/// </remarks>
private static bool IsHexDigit(char c) => char.IsAsciiHexDigit(c);
}
51 changes: 31 additions & 20 deletions src/DemaConsulting.CanvasNet/Canvas/Surface.cs
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ public void CompositeOverSpan(int y, int x, ReadOnlySpan<float> coverage, Rgba32
using var fg = new RowChannelBuffers(count);
using var work = new CompositeWorkBuffers(count);

CompositeOverSpanCore(y, x, coverage, color, bg, fg, work, count);
CompositeOverSpanCore(y, x, coverage, color, bg, fg, work);
}

/// <summary>
Expand Down Expand Up @@ -825,7 +825,7 @@ internal void CompositeOverSpan(int y, int x, ReadOnlySpan<float> coverage, Rgba
nameof(coverage), coverage.Length, "Coverage length must not exceed the workspace's capacity.");
}

CompositeOverSpanCore(y, x, coverage, color, workspace.Bg, workspace.Fg, workspace.Work, count);
CompositeOverSpanCore(y, x, coverage, color, workspace.Bg, workspace.Fg, workspace.Work);
}

/// <summary>
Expand Down Expand Up @@ -886,7 +886,7 @@ public void CompositeOverSpan(int y, int x, ReadOnlySpan<float> coverage, ReadOn
using var fg = new RowChannelBuffers(count);
using var work = new CompositeWorkBuffers(count);

CompositeOverSpanCore(y, x, coverage, colors, bg, fg, work, count);
CompositeOverSpanCore(y, x, coverage, colors, bg, fg, work);
}

/// <summary>
Expand Down Expand Up @@ -945,7 +945,7 @@ internal void CompositeOverSpan(int y, int x, ReadOnlySpan<float> coverage, Read
nameof(coverage), coverage.Length, "Coverage length must not exceed the workspace's capacity.");
}

CompositeOverSpanCore(y, x, coverage, colors, workspace.Bg, workspace.Fg, workspace.Work, count);
CompositeOverSpanCore(y, x, coverage, colors, workspace.Bg, workspace.Fg, workspace.Work);
}

/// <summary>
Expand Down Expand Up @@ -988,8 +988,9 @@ private void ValidateCompositeOverSpanArgs(int y, int x, int coverageLength)
/// </summary>
private void CompositeOverSpanCore(
int y, int x, ReadOnlySpan<float> coverage, Rgba32 color, RowChannelBuffers bg, RowChannelBuffers fg,
CompositeWorkBuffers work, int count)
CompositeWorkBuffers work)
{
var count = coverage.Length;
for (var i = 0; i < count; i++)
{
fg.RBytes[i] = color.R;
Expand All @@ -999,19 +1000,20 @@ private void CompositeOverSpanCore(
fg.ABytes[i] = (byte)Math.Clamp(MathF.Round(scaledAlpha, MidpointRounding.AwayFromZero), 0f, 255f);
}

CompositeOverSpanCoreShared(y, x, coverage, bg, fg, work, count);
CompositeOverSpanCoreShared(y, x, coverage, bg, fg, work);
}

/// <summary>
/// Per-pixel-color counterpart of <see cref="CompositeOverSpanCore(int, int, ReadOnlySpan{float}, Rgba32, RowChannelBuffers, RowChannelBuffers, CompositeWorkBuffers, int)"/>:
/// Per-pixel-color counterpart of <see cref="CompositeOverSpanCore(int, int, ReadOnlySpan{float}, Rgba32, RowChannelBuffers, RowChannelBuffers, CompositeWorkBuffers)"/>:
/// populates <paramref name="fg"/> from one <see cref="Rgba32"/> per pixel in
/// <paramref name="colors"/>, rather than broadcasting a single constant color, then
/// shares the exact same <see cref="CompositeOverSpanCoreShared"/> blend body.
/// </summary>
private void CompositeOverSpanCore(
int y, int x, ReadOnlySpan<float> coverage, ReadOnlySpan<Rgba32> colors, RowChannelBuffers bg, RowChannelBuffers fg,
CompositeWorkBuffers work, int count)
CompositeWorkBuffers work)
{
var count = coverage.Length;
for (var i = 0; i < count; i++)
{
var color = colors[i];
Expand All @@ -1022,7 +1024,7 @@ private void CompositeOverSpanCore(
fg.ABytes[i] = (byte)Math.Clamp(MathF.Round(scaledAlpha, MidpointRounding.AwayFromZero), 0f, 255f);
}

CompositeOverSpanCoreShared(y, x, coverage, bg, fg, work, count);
CompositeOverSpanCoreShared(y, x, coverage, bg, fg, work);
}

/// <summary>
Expand All @@ -1032,12 +1034,16 @@ private void CompositeOverSpanCore(
/// <see cref="CompositeOverRow"/> pipeline, and writes the result back into the surface
/// via <see cref="ReinterleaveRow"/>. <paramref name="fg"/> must already be populated (its
/// byte channels only - this method widens it to float itself) by the caller before this
/// method is invoked.
/// method is invoked. <paramref name="coverage"/>'s length is always the row's active
/// pixel count - it is not passed separately since every caller derives it from the very
/// same <paramref name="coverage"/> span before renting <paramref name="bg"/>/<paramref name="fg"/>/
/// <paramref name="work"/> to that exact size.
/// </summary>
private void CompositeOverSpanCoreShared(
int y, int x, ReadOnlySpan<float> coverage, RowChannelBuffers bg, RowChannelBuffers fg,
CompositeWorkBuffers work, int count)
CompositeWorkBuffers work)
{
var count = coverage.Length;
var bgRow = GetRowSpanBytes(y).Slice(x * BytesPerPixel, count * BytesPerPixel);

DeinterleaveRow(bgRow, bg, count);
Expand Down Expand Up @@ -1197,9 +1203,9 @@ private static void CompositeOverRow(RowChannelBuffers bg, RowChannelBuffers fg,
TensorPrimitives.Multiply(bgAn, oneMinusFgAn, outAn);
TensorPrimitives.Add(outAn, fgAn, outAn);

CompositeOverChannel(bg.RFloat, fg.RFloat, bgAn, fgAn, oneMinusFgAn, outAn, work.Term, work.OutR, count);
CompositeOverChannel(bg.GFloat, fg.GFloat, bgAn, fgAn, oneMinusFgAn, outAn, work.Term, work.OutG, count);
CompositeOverChannel(bg.BFloat, fg.BFloat, bgAn, fgAn, oneMinusFgAn, outAn, work.Term, work.OutB, count);
CompositeOverChannel(bg.RFloat, fg.RFloat, work, work.OutR, count);
CompositeOverChannel(bg.GFloat, fg.GFloat, work, work.OutG, count);
CompositeOverChannel(bg.BFloat, fg.BFloat, work, work.OutB, count);

// Only now scale the normalized outA up to [0, 255] for narrowing - every channel above
// divided by the still-normalized outAn.
Expand All @@ -1208,17 +1214,22 @@ private static void CompositeOverRow(RowChannelBuffers bg, RowChannelBuffers fg,

/// <summary>
/// Computes <c>(fgC * fgA + bgC * bgA * (1 - fgA)) / outA</c> for one <c>[0, 255]</c>-
/// scaled color channel of a whole row, where the alpha terms (<paramref name="fgAn"/>,
/// <paramref name="oneMinusFgAn"/>, <paramref name="outAn"/>) are <c>[0, 1]</c>-scaled.
/// scaled color channel of a whole row, reading the alpha terms (<c>BgAn</c>/<c>FgAn</c>/
/// <c>OneMinusFgA</c>/<c>OutA</c>, all <c>[0, 1]</c>-scaled) and the <c>Term</c> scratch
/// buffer directly from <paramref name="work"/> - the same
/// <see cref="CompositeWorkBuffers"/> instance <see cref="CompositeOverRow"/> already
/// populated them into, rather than re-passing each of its fields as its own parameter.
/// </summary>
private static void CompositeOverChannel(
float[] bgC, float[] fgC, Span<float> bgAn, Span<float> fgAn, Span<float> oneMinusFgAn, Span<float> outAn,
float[] termScratch, float[] outC, int count)
private static void CompositeOverChannel(float[] bgC, float[] fgC, CompositeWorkBuffers work, float[] outC, int count)
{
var bgAn = work.BgAn.AsSpan(0, count);
var fgAn = work.FgAn.AsSpan(0, count);
var oneMinusFgAn = work.OneMinusFgA.AsSpan(0, count);
var outAn = work.OutA.AsSpan(0, count);
var bgCSpan = bgC.AsSpan(0, count);
var fgCSpan = fgC.AsSpan(0, count);
var outCSpan = outC.AsSpan(0, count);
var termSpan = termScratch.AsSpan(0, count);
var termSpan = work.Term.AsSpan(0, count);

// outC = fgC * fgA
TensorPrimitives.Multiply(fgCSpan, fgAn, outCSpan);
Expand Down
85 changes: 64 additions & 21 deletions src/DemaConsulting.CanvasNet/Codecs/GifCodec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@
/// Console.WriteLine($"{surface.Width}x{surface.Height}");
/// </code>
/// </example>
public static Surface Load(Stream stream)

Check warning on line 160 in src/DemaConsulting.CanvasNet/Codecs/GifCodec.cs

View workflow job for this annotation

GitHub Actions / Build / Build macos-latest

Refactor this method to reduce its Cognitive Complexity from 43 to the 15 allowed.
{
ArgumentNullException.ThrowIfNull(stream);

Expand Down Expand Up @@ -290,27 +290,16 @@
}

result = new Surface(canvasWidth, canvasHeight);
for (var row = 0; row < imgHeight; row++)
{
var rowSpan = result.GetRowSpan(top + row);
var rowOffset = row * imgWidth;
for (var col = 0; col < imgWidth; col++)
{
var index = indices[rowOffset + col];
if (index >= activeColorTable.Length)
{
throw new InvalidDataException(
$"GIF pixel index {index} is out of range for its color table " +
$"({activeColorTable.Length} entries).");
}

var color = activeColorTable[index];
var alpha = (byte)(pendingTransparencyFlag && index == pendingTransparentIndex
? 0
: 255);
rowSpan[left + col] = new Rgba32(color.R, color.G, color.B, alpha);
}
}
BlitIndexedFrame(
result,
indices,
activeColorTable,
left,
top,
imgWidth,
imgHeight,
pendingTransparencyFlag,
pendingTransparentIndex);
}

pendingTransparencyFlag = false;
Expand Down Expand Up @@ -365,6 +354,60 @@
return Load(stream);
}

/// <summary>
/// Resolves one frame's decoded palette indices to RGBA pixels via
/// <paramref name="colorTable"/>, writing them into <paramref name="destination"/> at the
/// frame's declared placement, honoring the frame's transparent color index if any.
/// </summary>
/// <remarks>
/// Isolated from <see cref="Load(Stream)"/> as its own self-contained blit step - resolving
/// an indexed frame's pixels is a distinct, independently testable operation from the
/// surrounding GIF block/stream parsing that produces its inputs.
/// </remarks>
/// <param name="destination">The surface to write pixels into. Must already be sized to contain the frame's placement.</param>
/// <param name="indices">The frame's decoded palette indices, one per pixel, in row-major order.</param>
/// <param name="colorTable">The color table (local or global) to resolve each index against.</param>
/// <param name="left">The frame's left placement offset within <paramref name="destination"/>.</param>
/// <param name="top">The frame's top placement offset within <paramref name="destination"/>.</param>
/// <param name="width">The frame's width, in pixels.</param>
/// <param name="height">The frame's height, in pixels.</param>
/// <param name="transparencyFlag">Whether <paramref name="transparentIndex"/> should be rendered fully transparent.</param>
/// <param name="transparentIndex">The palette index treated as transparent when <paramref name="transparencyFlag"/> is set.</param>
/// <exception cref="InvalidDataException">
/// Thrown when a decoded index has no corresponding entry in <paramref name="colorTable"/>.
/// </exception>
private static void BlitIndexedFrame(
Surface destination,
byte[] indices,
Rgba32[] colorTable,
int left,
int top,
int width,
int height,
bool transparencyFlag,
byte transparentIndex)
{
for (var row = 0; row < height; row++)
{
var rowSpan = destination.GetRowSpan(top + row);
var rowOffset = row * width;
for (var col = 0; col < width; col++)
{
var index = indices[rowOffset + col];
if (index >= colorTable.Length)
{
throw new InvalidDataException(
$"GIF pixel index {index} is out of range for its color table " +
$"({colorTable.Length} entries).");
}

var color = colorTable[index];
var alpha = (byte)(transparencyFlag && index == transparentIndex ? 0 : 255);
rowSpan[left + col] = new Rgba32(color.R, color.G, color.B, alpha);
}
}
}

/// <summary>
/// Reads a GIF file's Logical Screen Descriptor and reports its declared dimensions,
/// without reading any color table, block, or pixel data.
Expand Down
Loading
Loading