Include the half-sample boundary in the Box filter - #115
Merged
anthonynsimon merged 1 commit intoSep 7, 2026
Merged
Conversation
Box was the only resample filter whose window can hold zero non-zero weights: with Support 0.5 and a strict comparison, a destination centre exactly midway between two source samples scores both neighbours at zero, so the weighted sum is zero and r/sum is NaN, which converts to a transparent black pixel.
Owner
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
transform.ResizewithBoxturns pixels transparent black. On a solid opaque white 2x2 upscaledto 3x3:
Not just a toy size. A photo-shaped upscale of a fully opaque white image, 640x480 -> 960x720,
comes out with 384,000 of 691,200 pixels (55.6%) transparent black. It is reachable from the
CLI too, via
--filter box(cmd/helpers.go:144).Cause
Boxis the only filter whose window can contain zero non-zero weights. ItsFn(
transform/filters.go:50) is strictly< 0.5on both sides, andSupport: 0.5givesfilterRadius == 1when upscaling. When a destination centre lands exactly midway between twosource samples, both neighbours sit at
|normPos| == 0.5and score 0, sosum == 0andr/sumat
transform/resize.go:93is0/0= NaN.uint8(NaN)is 0.From a window-arithmetic probe, source width 2 -> destination width 3:
Sweeping a solid white source over every
(srcW, dstW)pair from 1 to 16 and counting outputbytes that are not
0xFF:35 distinct pairs break, all upscales from an even source width: 2->3, 2->5, 4->6, 6->9, 8->12,
12->13, 14->15, and so on.
The change
One operator,
<to<=. It only alters output where|normPos| == 0.5exactly - the degeneratecase - and at a midpoint both straddled samples now weigh 1, so the pixel becomes their average,
which is what a box filter straddling a sample boundary should give.
disintegration/imaging, whoseResampleFilter{Support, Kernel}shape this package'sResampleFilter{Support, Fn}follows, usesx <= 0.5for its Box (resize.go:448).The doc comment did say
x < 0.5, so this could be read as documented intent. I do not thinkit survives, because the documented behaviour has no defensible output - it divides by zero and
produces a transparent black pixel from an opaque white image. The comment reads as describing the
bug rather than sanctioning it, and I updated that line with the fix. Happy to be told otherwise.
Tests
TestResizeBoxHalfwaySampleintransform/resize_test.go, table-driven with manual pixel data andutil.RGBAImageEqual, matching the conventionCLAUDE.mddocuments. I used a solid-colourinvariant rather than a value table on purpose: the expected ratio is exactly 255 in exact
arithmetic and clamps to 255 on either side of rounding, so it will not join #72 / #108 as another
arm64 floating-point test failure.
Reverting only the operator makes all three cases fail with the visible transparent cross. The
existing
TestResizeBox- both its x2 upscale and its x0.5 downscale expectations - passesunchanged, so there is no blast radius on existing values.
Ran what
.github/workflows/check.ymlruns:make buildandmake testgreen, plusmake racegreen across all 18 packages.
gofmt -lempty andgo vet ./...clean, before and after.Precedent
Issue #37 "effect.Grayscale returns black pixels for a pure white color" and PR #45 "Fix integer
overflow on white colors" are the same class - correct numeric handling producing black pixels
from white input - and #45 was merged.
Disclosure: found and prepared with AI assistance (Claude). Every figure above is from a run on
this branch.