Skip to content

Commit 5cb5384

Browse files
committed
fix(vanilla): composite canvas scatter points individually
Canvas-mode scatter batched every point sharing a (fill, alpha) into one beginPath() and painted the lot with a single fill() at globalAlpha. Circles overlapping inside one path are unioned by the fill rule, so the whole cluster was faded ONCE instead of each dot compositing in turn. A six-deep stack at opacity 0.35 landed on 0.65 luminance instead of 0.12. Dense translucent scatters therefore rendered visibly washed out, while their exports looked correct -- exports below VECTOR_EXPORT_MAX_POINTS re-render through real SVG circles, which never had the bug. Above that cap the raster-marks export path shares the canvas renderer, so it was affected too. Fills, strokes and exit ghosts are now issued per point. This is not a correctness-for-speed trade: per-circle measured faster at every count tested in headless Chromium (~1.5x at 4k points, ~3.7x at 50k). One enormous multi-subpath tessellation costs more than thousands of tiny independent fills. Redundant fillStyle/strokeStyle/lineWidth/globalAlpha writes are elided across runs of identically-styled points, and paint order stays mark order to match SVG. Regression coverage is an e2e pixel test rather than a unit test: a recording 2D-context stub cannot see this, since both call sequences are legal and only the rasterized output differs. The new invariants spec stacks six translucent dots and asserts the composited luminance matches the closed-form value, which fails at ~159 with the batching restored. - packages/vanilla/src/scatter-canvas/renderer.ts: per-point draw calls - e2e/invariants/canvas-alpha-parity.spec.ts: pixel-level parity gate - examples/src/testing/canvas-alpha-parity.stories.tsx: its harness - renderer.test.ts: assert per-point calls + state-write elision - docs + gallery story: drop the now-false "batching" explanation, keep the still-real fill-pass-then-stroke-pass paint-order note
1 parent 640def6 commit 5cb5384

6 files changed

Lines changed: 399 additions & 72 deletions

File tree

docs/spec-reference.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -795,11 +795,11 @@ Everything below is a deliberate trade, not a limitation to work around:
795795
- **Gradient fills flatten to their first stop on screen.** Exports render the
796796
true gradient, because they materialize a full SVG (see below).
797797
- **Overlapping dots can show a stroke over a neighbour's fill.** The canvas
798-
paints every fill and then every stroke, two batched draw calls rather than
799-
two per point, which is what keeps thousands of dots cheap. SVG paints each
800-
dot's fill and stroke together. Sparse clouds are indistinguishable; tight
801-
clusters show it. Lowering `mark.opacity` for dense scatters — worth doing
802-
regardless, to keep overplotting readable — makes it disappear.
798+
paints all fills in one pass and all strokes in a second, so a later dot's
799+
stroke can sit over an earlier dot's fill. SVG paints each dot's fill and
800+
stroke together. Sparse clouds are indistinguishable; tight clusters show it.
801+
Lowering `mark.opacity` for dense scatters — worth doing regardless, to keep
802+
overplotting readable — makes it disappear.
803803
- **Per-mark keyboard focus and edit-mode point selection are unavailable.**
804804
There are no per-point elements to focus. Annotation, chrome and legend
805805
editing still work, and the screen-reader data table is unaffected.
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/**
2+
* Canvas and SVG must composite translucent overlapping marks identically.
3+
*
4+
* Red-locked bug: the canvas renderer batched all points sharing a
5+
* (fill, alpha) into one `beginPath()` and painted them with a single `fill()`
6+
* at `globalAlpha`. Circles overlapping inside one path get unioned by the fill
7+
* rule, so the stack was faded ONCE -- a dense cluster came out at the alpha of
8+
* a single dot. SVG composites each `<circle>` separately and builds toward
9+
* opaque. Dense scatters rendered washed out on screen while their exports
10+
* (which re-render through real SVG) looked right.
11+
*
12+
* This cannot be caught by a stubbed 2D context: both call sequences are legal
13+
* and the difference only exists in rasterized output. Hence a real browser.
14+
*
15+
* The story stacks `STACK_DEPTH` identical points at one coordinate, so the
16+
* correct composite is closed-form rather than a screenshot baseline:
17+
* over white, luminance = 255 * (1 - opacity)^depth
18+
* With opacity 0.35 and depth 6 that is ~19/255. The batched bug produced
19+
* ~166/255 -- a gap no antialiasing tolerance can blur away.
20+
*/
21+
22+
import { expect, test } from '@playwright/test';
23+
24+
const OPACITY = 0.35;
25+
const STACK_DEPTH = 6;
26+
27+
/** Ideal src-over result for N stacked black dots at OPACITY on white. */
28+
const EXPECTED = 255 * (1 - OPACITY) ** STACK_DEPTH;
29+
30+
/**
31+
* Generous: covers antialiasing, DPR downsampling and the canvas layer's
32+
* 8-bit rounding. Still ~7x tighter than the distance to the buggy value, so
33+
* the test discriminates the defect without policing sub-percent drift.
34+
*/
35+
const TOLERANCE = 20;
36+
37+
test('canvas composites stacked translucent points like SVG does', async ({ page }) => {
38+
await page.goto('/?mode=preview&story=testing--canvas-alpha-parity--stacked-translucent-points');
39+
40+
// `createChart` promotes the mount node itself to `.oc-root`; the chart is not
41+
// nested in a further wrapper.
42+
await page.waitForSelector('#parity-svg.oc-root svg.oc-chart');
43+
await page.waitForSelector('#parity-canvas canvas.oc-mark-canvas');
44+
await page.evaluate(() => document.fonts.ready);
45+
// The canvas layer paints on a rAF tick; give it a couple of frames to land.
46+
await page.waitForTimeout(200);
47+
48+
const sample = await page.evaluate(() => {
49+
const markCanvas = document.querySelector(
50+
'#parity-canvas canvas.oc-mark-canvas',
51+
) as HTMLCanvasElement | null;
52+
if (!markCanvas) return { error: 'no mark canvas' };
53+
54+
const ctx = markCanvas.getContext('2d');
55+
if (!ctx) return { error: '2d context unavailable' };
56+
57+
const { width, height } = markCanvas;
58+
const data = ctx.getImageData(0, 0, width, height).data;
59+
60+
/**
61+
* Composite a pixel over white, so the reading is comparable regardless of
62+
* whether the layer painted an opaque background or left it transparent.
63+
*/
64+
const lumAt = (i: number): number => {
65+
const a = data[i + 3] / 255;
66+
return data[i] * a + 255 * (1 - a);
67+
};
68+
69+
// Locate the stack rather than trusting a computed coordinate.
70+
//
71+
// Deliberate: on the canvas side `.oc-marks` is an EMPTY svg group (the dots
72+
// live on the canvas), so its bounding box collapses to zero at the
73+
// container origin -- sampling there reads unpainted background and the test
74+
// would "fail" on the wrong pixel. The stack is the darkest thing drawn, so
75+
// searching for it is both simpler and self-validating.
76+
let darkest = Number.POSITIVE_INFINITY;
77+
let paintedPixels = 0;
78+
for (let i = 0; i < data.length; i += 4) {
79+
if (data[i + 3] > 2) paintedPixels++;
80+
const lum = lumAt(i);
81+
if (lum < darkest) darkest = lum;
82+
}
83+
if (!Number.isFinite(darkest)) return { error: 'canvas is empty' };
84+
85+
// Average the INTERIOR of the stack, not a box centred on the darkest pixel.
86+
//
87+
// The darkest pixel is frequently on the dot's antialiased rim, so a box
88+
// around it straddles the edge and averages dot with background -- that read
89+
// lands ~100 even on a correct render and would make this test lie in both
90+
// directions. Instead: take every pixel at or very near the darkest value.
91+
// On a correct render that is the flat interior of the stack; if compositing
92+
// regresses, the whole interior shifts together and the mean moves with it.
93+
const NEAR = 4;
94+
let sum = 0;
95+
let count = 0;
96+
for (let i = 0; i < data.length; i += 4) {
97+
const lum = lumAt(i);
98+
if (lum <= darkest + NEAR) {
99+
sum += lum;
100+
count++;
101+
}
102+
}
103+
104+
return {
105+
canvasLuminance: sum / count,
106+
darkest,
107+
interiorPixels: count,
108+
paintedPixels,
109+
// If the SVG side stopped drawing dots the comparison would be vacuous.
110+
svgCircleCount: document.querySelectorAll('#parity-svg circle.oc-mark-point').length,
111+
canvasCircleCount: document.querySelectorAll('#parity-canvas circle.oc-mark-point').length,
112+
};
113+
});
114+
115+
if ('error' in sample) throw new Error(`sampling failed: ${sample.error}`);
116+
117+
// Guard the premise: SVG really did draw the stack, canvas really did not,
118+
// and the canvas actually painted something for us to measure.
119+
expect(sample.svgCircleCount, 'SVG chart should render real circles').toBe(STACK_DEPTH);
120+
expect(sample.canvasCircleCount, 'canvas chart should have no SVG circles').toBe(0);
121+
expect(sample.paintedPixels, 'canvas painted nothing to sample').toBeGreaterThan(100);
122+
123+
// eslint-disable-next-line no-console
124+
console.log(
125+
`alpha parity: interior=${sample.canvasLuminance.toFixed(1)} darkest=${sample.darkest.toFixed(1)} ` +
126+
`expected=${EXPECTED.toFixed(1)} (interiorPx=${sample.interiorPixels})`,
127+
);
128+
129+
// The stack must cover a real area. A single dot at r=14 is ~600px; six
130+
// coincident dots stay ~600. If this collapses, the renderer stopped drawing
131+
// the cluster and the luminance readings below would be measuring noise.
132+
expect(sample.interiorPixels, 'stack interior too small to trust').toBeGreaterThan(200);
133+
134+
const message =
135+
`canvas luminance ${sample.canvasLuminance.toFixed(1)} should be within ${TOLERANCE} of ` +
136+
`${EXPECTED.toFixed(1)}. A value near 166 means overlapping points were batched into one ` +
137+
`path and faded once instead of compositing individually.`;
138+
139+
// Assert on BOTH the interior mean and the darkest pixel. The mean alone is
140+
// weak by construction -- it averages pixels selected for being near the
141+
// darkest, so it tracks `darkest` and would look tight even if the whole
142+
// stack lightened. Pinning `darkest` to the expected composite is what
143+
// actually catches a compositing regression.
144+
expect(Math.abs(sample.darkest - EXPECTED), message).toBeLessThan(TOLERANCE);
145+
expect(Math.abs(sample.canvasLuminance - EXPECTED), message).toBeLessThan(TOLERANCE);
146+
});

examples/src/gallery/charts-scatter-distribution.stories.tsx

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -709,13 +709,16 @@ function CanvasMorphScatter() {
709709
* Small enough that auto leaves it on SVG, so `render: 'canvas'` is explicit.
710710
* This is the visual-parity check: same layout, same colors, same geometry.
711711
*
712-
* Two differences are expected and permanent, both consequences of batching:
713-
* the trendline always draws above the dots, and where dots overlap, a stroke
714-
* can land on a neighbour's fill. Canvas paints every fill and then every
715-
* stroke (two draw calls total, which is what keeps thousands of points
716-
* cheap), whereas SVG paints each dot's fill and stroke together. Sparse
712+
* Two differences are expected and permanent: the trendline always draws above
713+
* the dots, and where dots overlap, a stroke can land on a neighbour's fill.
714+
* The latter is a paint-order effect -- canvas fills every dot, then strokes
715+
* every dot, whereas SVG paints each dot's fill and stroke together. Sparse
717716
* clouds are indistinguishable; tight clusters show it.
718717
*
718+
* Translucent overlaps do NOT differ: each dot composites individually on
719+
* canvas exactly as it does in SVG. That parity is pinned by
720+
* `e2e/invariants/canvas-alpha-parity.spec.ts`.
721+
*
719722
* `animation: false` because a baseline screenshot cannot freeze the canvas
720723
* entrance.
721724
*/
@@ -1092,7 +1095,7 @@ export const ScatterAndDistribution = () => (
10921095
<Demo
10931096
id="canvas-svg-parity"
10941097
title="Canvas vs SVG, same data"
1095-
description="An explicit render: 'canvas' on a 200-point cloud that auto would have left on SVG. Same layout, same geometry. Two differences come from batching, which is what keeps thousands of points cheap: the trendline always draws above the dots, and canvas paints every fill before every stroke, so overlapping dots can show a stroke over a neighbour's fill. Sparse clouds look identical; tight clusters show it."
1098+
description="An explicit render: 'canvas' on a 200-point cloud that auto would have left on SVG. Same layout, same geometry. Two differences remain: the trendline always draws above the dots, and canvas fills every dot before stroking any, so overlapping dots can show a stroke over a neighbour's fill. Sparse clouds look identical; tight clusters show it."
10961099
specForPanel={canvasParitySpec}
10971100
height={420}
10981101
>
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/**
2+
* Testing / Canvas-vs-SVG alpha compositing parity harness.
3+
*
4+
* Not an editorial demo -- this story exists so the Playwright `invariants`
5+
* project can compare what the two mark renderers actually put on screen.
6+
*
7+
* The bug this guards against: the canvas renderer used to batch every point
8+
* sharing a (fill, alpha) into ONE `beginPath()` and pay for it with ONE
9+
* `fill()` at `globalAlpha`. Overlapping circles inside a single path are
10+
* unioned by the fill rule and then faded once, so a dense cluster of
11+
* translucent dots came out at the alpha of a SINGLE dot. SVG composites every
12+
* `<circle>` separately, so the same cluster builds toward opaque. A 5-deep
13+
* stack at opacity 0.35 landed on 0.65 luminance instead of 0.12 -- dense
14+
* scatters rendered visibly washed out, while exports (which re-render through
15+
* real SVG) looked correct.
16+
*
17+
* A stubbed 2D context cannot catch this: the call sequence is legal either
18+
* way and the defect only exists in the rasterizer's output. So both charts
19+
* mount here, in a real browser, over the SAME data, and the spec reads pixels
20+
* out of each.
21+
*
22+
* Both charts are deliberately given heavily overlapping points -- a tight
23+
* cluster is where the two compositing models diverge most. Sparse clouds
24+
* agree even with the bug present, which is exactly why this shipped.
25+
*/
26+
27+
import type { ChartSpec } from '@opendata-ai/openchart-core';
28+
import { createChart } from '@opendata-ai/openchart-vanilla';
29+
import { useEffect, useRef } from 'react';
30+
31+
export default { title: 'Testing / Canvas Alpha Parity' };
32+
33+
/** Fill + opacity are fixed so the expected composite is computable by hand. */
34+
const FILL = '#000000';
35+
const OPACITY = 0.35;
36+
/** Deep enough that per-circle compositing is unmistakably darker than one fill. */
37+
const STACK_DEPTH = 6;
38+
39+
const WIDTH = 420;
40+
const HEIGHT = 320;
41+
42+
/**
43+
* Every point sits at the same data coordinate, so all `STACK_DEPTH` circles
44+
* land on the same pixel. That makes the assertion a closed-form value rather
45+
* than a screenshot: correct output is `1 - (1 - 0.35)^6` over white.
46+
*/
47+
function stackedSpec(render: 'svg' | 'canvas'): ChartSpec {
48+
return {
49+
// A settled chart, not a mid-fade one -- entrance alpha would poison the read.
50+
animation: false,
51+
width: WIDTH,
52+
height: HEIGHT,
53+
mark: {
54+
type: 'point',
55+
render,
56+
size: 14,
57+
opacity: OPACITY,
58+
fill: FILL,
59+
// No separator stroke: a stroke over the sample point would be measuring
60+
// the stroke pass, not the fill compositing under test.
61+
strokeWidth: 0,
62+
},
63+
data: Array.from({ length: STACK_DEPTH }, (_, i) => ({ id: `p${i}`, x: 50, y: 50 })),
64+
encoding: {
65+
x: { field: 'x', type: 'quantitative', scale: { domain: [0, 100] } },
66+
y: { field: 'y', type: 'quantitative', scale: { domain: [0, 100] } },
67+
key: { field: 'id', type: 'nominal' },
68+
},
69+
};
70+
}
71+
72+
function Chart({ render, id }: { render: 'svg' | 'canvas'; id: string }) {
73+
const ref = useRef<HTMLDivElement>(null);
74+
75+
useEffect(() => {
76+
if (!ref.current) return;
77+
const chart = createChart(ref.current, stackedSpec(render));
78+
return () => chart.destroy();
79+
}, [render]);
80+
81+
return <div id={id} ref={ref} style={{ width: WIDTH, height: HEIGHT }} />;
82+
}
83+
84+
/**
85+
* Two charts over identical data and identical opacity, differing only in
86+
* which renderer draws the dots. The spec samples the centre pixel of each.
87+
*/
88+
export const StackedTranslucentPoints = () => (
89+
<div style={{ padding: 16, background: '#ffffff' }}>
90+
<div id="alpha-parity-ready" data-ready="true" />
91+
<Chart render="svg" id="parity-svg" />
92+
<Chart render="canvas" id="parity-canvas" />
93+
</div>
94+
);

packages/vanilla/src/scatter-canvas/__tests__/renderer.test.ts

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,30 @@ describe('ScatterCanvasRenderer DPR handling', () => {
9090
});
9191
});
9292

93-
describe('ScatterCanvasRenderer batching', () => {
94-
it('draws N same-fill points in a single fill() call', () => {
93+
/**
94+
* `fillStyle` values assigned for point fills, i.e. everything after the
95+
* full-bleed background write that `render()` performs first.
96+
*/
97+
function pointFillStyles(s: CanvasStub): unknown[] {
98+
const all = s.sets.filter((set) => set.prop === 'fillStyle').map((set) => set.value);
99+
// Assert rather than blind-slice: if the background write ever stops being
100+
// first, dropping [0] would silently swallow a real point fill and the
101+
// color-elision assertions would start passing for the wrong reason.
102+
expect(all[0], 'expected the full-bleed background fill first').toBe('#ffffff');
103+
return all.slice(1);
104+
}
105+
106+
describe('ScatterCanvasRenderer per-point compositing', () => {
107+
/**
108+
* The renderer deliberately does NOT merge points into shared paths.
109+
*
110+
* Merging unions overlapping circles inside one path, so a single `fill()` at
111+
* `globalAlpha` fades the whole cluster once instead of letting each dot
112+
* composite -- dense translucent scatters rendered washed out. These tests
113+
* pin the call shape; `e2e/invariants/canvas-alpha-parity.spec.ts` pins the
114+
* resulting pixels, which is the part a recording stub cannot see.
115+
*/
116+
it('issues one fill() per point rather than one per color bucket', () => {
95117
stub = stubCanvas2D();
96118
const renderer = new ScatterCanvasRenderer(document.createElement('canvas'));
97119
renderer.resize(400, 300);
@@ -103,11 +125,15 @@ describe('ScatterCanvasRenderer batching', () => {
103125
}));
104126
renderer.render(makeState({ marks: soa(points) }));
105127

106-
expect(stub.callsTo('fill')).toHaveLength(1);
128+
// 50 points => 50 fills and 50 arcs. A single fill() here would mean the
129+
// path batching regressed and translucent overlaps stopped accumulating.
130+
expect(stub.callsTo('fill')).toHaveLength(50);
107131
expect(stub.callsTo('arc')).toHaveLength(50);
132+
// 50 point paths + the one `render()` opens for the clip rect.
133+
expect(stub.callsTo('beginPath')).toHaveLength(51);
108134
});
109135

110-
it('splits fill buckets by color', () => {
136+
it('skips redundant fillStyle writes across a same-color run', () => {
111137
stub = stubCanvas2D();
112138
const renderer = new ScatterCanvasRenderer(document.createElement('canvas'));
113139
renderer.resize(400, 300);
@@ -120,10 +146,34 @@ describe('ScatterCanvasRenderer batching', () => {
120146
]),
121147
}),
122148
);
123-
expect(stub.callsTo('fill')).toHaveLength(2);
149+
150+
// Every point still gets its own fill()...
151+
expect(stub.callsTo('fill')).toHaveLength(3);
152+
// ...but the three colors here alternate, so all three assignments stand.
153+
// Drop the leading background write (makeState paints '#ffffff' full-bleed).
154+
expect(pointFillStyles(stub)).toEqual(['#ff0000', '#00ff00', '#ff0000']);
155+
});
156+
157+
it('collapses fillStyle writes when consecutive points share a color', () => {
158+
stub = stubCanvas2D();
159+
const renderer = new ScatterCanvasRenderer(document.createElement('canvas'));
160+
renderer.resize(400, 300);
161+
renderer.render(
162+
makeState({
163+
marks: soa([
164+
{ x: 50, y: 100, r: 3, fill: '#ff0000' },
165+
{ x: 60, y: 100, r: 3, fill: '#ff0000' },
166+
{ x: 70, y: 100, r: 3, fill: '#00ff00' },
167+
]),
168+
}),
169+
);
170+
171+
expect(stub.callsTo('fill')).toHaveLength(3);
172+
// The second red point reuses the style already set by the first.
173+
expect(pointFillStyles(stub)).toEqual(['#ff0000', '#00ff00']);
124174
});
125175

126-
it('batches strokes by (stroke, width) and skips zero-width strokes', () => {
176+
it('strokes each point individually and skips zero-width strokes', () => {
127177
stub = stubCanvas2D();
128178
const renderer = new ScatterCanvasRenderer(document.createElement('canvas'));
129179
renderer.resize(400, 300);
@@ -137,8 +187,13 @@ describe('ScatterCanvasRenderer batching', () => {
137187
]),
138188
}),
139189
);
140-
// Two stroke buckets; no gridlines, so every stroke() is a point stroke.
141-
expect(stub.callsTo('stroke')).toHaveLength(2);
190+
191+
// Three stroked points => three stroke() calls; the zero-width one is
192+
// skipped entirely. No gridlines here, so every stroke() is a point stroke.
193+
expect(stub.callsTo('stroke')).toHaveLength(3);
194+
// lineWidth changes only where the width actually changes (1 -> 2).
195+
const widths = stub.sets.filter((s) => s.prop === 'lineWidth').map((s) => s.value);
196+
expect(widths).toEqual([1, 2]);
142197
});
143198

144199
it('culls points outside the clip rect', () => {

0 commit comments

Comments
 (0)