Repository navigation
fix(render): match browser box-shadow falloff and premultiplied alpha - #32
Open
leshchenko1979 wants to merge 1 commit into
Open
leshchenko1979 wants to merge 1 commit into
leshchenko1979 wants to merge 1 commit into
Conversation
Box shadows were off by up to 19 % of pixels against Chromium, for two independent reasons. The blur radius passed to boxBlurAlpha was the CSS blur value itself. A CSS blur radius b is a Gaussian with sigma = b/2, while four box passes of radius r have variance 4(r^2+r)/3, so the radius has to be derived from that equation and then calibrated: the derived value alone still left 2 % of pixels wrong at blur 48. shadowBlurRadius() now applies the calibration (k = 1.2) and matches Chromium at 0.000 % for blur 0..48. The scratch canvas padding follows the actual spread (2*radius+2) instead of a guessed blur*3. blendOver() returned straight-alpha RGB (dividing by the output alpha) but wrote into image.RGBA, which stores premultiplied samples. Every partially transparent composite was therefore invalid: rgba(255,0,0,0.8) rendered as (101,51,51) on white instead of (255,51,51), a 60 % loss in the red channel. Channels now stay premultiplied. Inset shadows were drawn by a ring loop that ramps alpha linearly from the edge, which is not the Gaussian falloff CSS specifies (1.9-11 % of pixels off, worse as blur grows). They are now the blurred complement of a hole, reusing the same calibrated blur: edge alpha lands at half, fading inward, matching Chromium within 2 px at blur 8/24/48. Also fixes a latent double release: boxBlurAlpha returns its input unchanged when the radius is 0, and the caller released it anyway, putting one buffer in the alpha pool twice so the next two acquisitions aliased. Measured against Chromium over nine shadow cases (inset and outset, blur 0..48, offset, spread, semi-transparent colour): 0.62-19.06 % of pixels differing before, 0.00 % after. The 21-case deck harness keeps 11-shadow at 0.00 % (was 2.35 %) with no other case moving. Fixes Macawls#29
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.
What
Three defects in the PNG shadow path, all measured against Chromium.
1. The blur radius was the CSS value itself. A CSS blur radius
bis a Gaussian with σ = b/2; four box passes of radiusrhave variance 4(r²+r)/3. Passingbstraight through made every shadow too tight, and even the correctly derived radius left ~2 % of pixels wrong at blur 48 — the calibration constantk = 1.2is what closes it.shadowBlurRadius()now does both steps. Scratch-canvas padding follows the real spread (2*radius+2) instead of a guessedblur*3.2.
blendOverwrote straight-alpha RGB into a premultipliedimage.RGBA. It divided by the output alpha, so every partially transparent composite was invalid:rgba(255,0,0,0.8)came out(101,51,51)on white instead of(255,51,51)— a 60 % loss in the red channel. Channels now stay premultiplied. This also affected gradient stops and any translucent border.3. Inset shadows were a linear ring ramp. Not the Gaussian CSS specifies, and it degraded with blur. They are now the blurred complement of a hole, reusing the calibrated blur: edge alpha at half, fading inward.
Plus a latent double release:
boxBlurAlphareturns its input unchanged when the radius is 0, and the caller released it anyway — one buffer entered the alpha pool twice and the next two acquisitions aliased.Measured
Nine dedicated shadow cases (inset/outset, blur 0/8/24/48, offset, spread, semi-transparent colour), pixel diff > 32 per channel against headless Chromium:
The 21-case deck harness:
11-shadow2.35 % → 0.00 %, every other case byte-identical (no collateral change).Tests
render/shadow_png_test.go— all fail onmain:TestBlendOverReturnsPremultiplied— the premultiplied contract, asserted on both a transparent and an opaque destination.TestBlendOverOpaqueSourceIsPassthroughTestShadowBlurRadiusCalibration— blur 0/2/8/24/48.TestOutsetShadowSemiTransparentColorIsNotDarkened—rgba(255,0,0,0.8)must render(255,51,51).TestInsetShadowFalloffMatchesBrowser— half-darkness distance at blur 8/24/48 within 2 px of Chromium's measured 3/9/17 px, centre stays white.TestShadowBlurZeroRepeatedRendersAreStable— catches the double release through pooled-buffer aliasing.go test ./...andgo vet ./...pass.Fixes #29