fix(RadialProgress): premultiplied composite to remove AA fringe (#36)#37
Merged
Conversation
The previous composite multiplied the layer color by coverage and then used mix(base, layer, layer.a) -- which effectively multiplies the layer RGB by coverage a second time, darkening and desaturating AA edges by ~20% at 50% coverage. Reported in #36 with reproduction screenshots. Switch to a premultiplied-alpha composite throughout: fillPM = vec4(fillCol.rgb * fillCol.a, fillCol.a) trackPM = vec4(trackCol.rgb * trackCol.a, trackCol.a) layer = trackPM*trackCoverage + fillPM*fillCoverage (already premul) out.rgb = base.rgb * (1 - layer.a) + layer.rgb (premul "over") This matches the renderer's blend func (gl.ONE, gl.ONE_MINUS_SRC_ALPHA), so the output is already in the form the framebuffer expects. The fix preserves the original behavior of partial-alpha track colors -- which worked under the old code only because the double-multiplication coincidentally offset the missing input premultiplication. Canvas2D backend is unaffected: it uses ctx.stroke() with strokeStyle and relies on the browser's source-over compositor, which is already correct. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Closes #36.
Summary
The current composite multiplies `layer.rgb` by AA coverage and then uses `mix(base.rgb, layer.rgb, layer.a)` to blend. `mix` expands to `a*(1-t) + b*t`, so when `b` is already coverage-scaled and `t == coverage`, the layer RGB gets multiplied by coverage a second time. The result is a ~20% darker, desaturated band at the stroke's AA edges — visible as a gray-blue fringe in the reporter's screenshot.
Fix
Composite in premultiplied-alpha space throughout:
```glsl
vec4 fillPM = vec4(fillCol.rgb * fillCol.a, fillCol.a);
vec4 trackPM = vec4(trackCol.rgb * trackCol.a, trackCol.a);
layer = trackPM * trackCoverage + fillPM * fillCoverage; // already premul
// Premul "over"
out.rgb = base.rgb * (1.0 - layer.a) + layer.rgb;
```
This matches the renderer's blend func (`gl.ONE, gl.ONE_MINUS_SRC_ALPHA`) at WebGlRenderer.ts:180, so the fragment's output is already in the form the framebuffer expects.
Why this also fixes the (latent) track-color case
The old code happened to render partial-alpha track colors at the right intensity, but only because the double-multiplication offset the missing input premultiplication — two bugs cancelling. Premultiplying both inputs and using a true over-operator makes both cases right for the right reason.
Canvas2D
Unaffected: the canvas backend strokes via `ctx.stroke()` with a `strokeStyle` and relies on the browser's source-over compositor, which is already correct.
Verification
Credit
@rwaltenberg diagnosed the cause and provided the exact patch in #36 — adopting it verbatim.
🤖 Generated with Claude Code