Skip to content

Accept padding that leaves one pixel of content on odd and tiny sizes [patch] - #141

Open
matt-edmondson wants to merge 1 commit into
mainfrom
fix/122-padding-bound
Open

matt-edmondson wants to merge 1 commit into
mainfrom
fix/122-padding-bound

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #122

What changed

Arguments.Validate compared the padding against Size / 2, and integer division rounds an odd size down. As a result:

  • --size 1 failed with "Padding must be less than half the size" even at the default padding of 0.
  • --size 3 --padding 1 was rejected, although the pipeline's own clamp (EffectivePadding, (size - 1) / 2) allows it and leaves a 1-pixel image.

Validate now rejects only Padding * 2 >= Size, which is the same bound EffectivePadding uses. The error message is unchanged: it is now exactly true ("less than half the size", with no rounding). The --padding help text and the README's option and exit-code tables now state the bound as padding * 2 < size.

Tests (ArgumentsTests)

  • ValidateAcceptsTheSmallestSizesThePipelineHandles: Size = 1, Padding = 0 and Size = 3, Padding = 1 are both valid.
  • ValidateRejectsPaddingThatLeavesNoContentOnASmallSize: Size = 4, Padding = 2 is rejected with the padding message.
  • ValidateUsesIntegerDivisionForOddSizes pinned the old rounding, so it is replaced by ValidateAcceptsPaddingThatLeavesOnePixelOnAnOddSize: on 33 pixels, 16 is now accepted and 17 rejected.

With the Arguments.cs change reverted, the three new cases fail. With the change, every ArgumentsTests case passes, along with the rest of the suite except OutputMatchesTheGoldMaster. Those 8 cases fail identically on main in this environment, because the checkout has the gold-master PNGs only as Git LFS pointer stubs.

Note on CI

The scheduled CI run on main is red as well (run 37857512628). Locally, restore on main fails with NU1902/NU1903 "Warning As Error" for SixLabors.ImageSharp 3.1.12 (GHSA-j3p4-wp97-rph4 and others). The failure isn't caused by this PR, and this PR doesn't widen its scope to bump ImageSharp.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Rt1SUkG3bsTZEWbmGirSx3


Generated by Claude Code

… [patch]

Validate compared Padding against Size / 2, which rounds an odd size down. --size 1 failed
even at the default padding of 0, and --size 3 --padding 1 was refused although the pipeline's
own clamp, EffectivePadding's (size - 1) / 2, allows it. Validate now rejects only
Padding * 2 >= Size, the same bound.

Fixes #122

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Rt1SUkG3bsTZEWbmGirSx3

Copy link
Copy Markdown
Contributor Author

The ci / .NET / Test on {ubuntu,windows,macos}-latest jobs fail during restore, before any test runs. The cause is NU1902/NU1903 "Warning As Error" for SixLabors.ImageSharp 3.1.12 (GHSA-gwg2-r3hj-4w44, GHSA-j3p4-wp97-rph4, GHSA-j9gm-c75j-xc9q, GHSA-jjfr-hcj7-qf5w, GHSA-wmxv-xphr-5c9g).

This failure isn't caused by this PR. The scheduled CI run on main (37857512628) fails the same way, and no open PR fixes it yet. The fix is to bump SixLabors.ImageSharp in Directory.Packages.props to the first release that patches those advisories. That belongs in its own PR. With NuGet audit disabled locally, this branch's tests pass, except for gold-master cases that also fail on main here because the PNGs arrive as LFS pointer stubs.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants