Skip to content

Commit 0ccdaed

Browse files
fix(engine): warn when a dense chart silently falls back to SVG (#111)
The 'auto' renderer promotes to canvas above 1,000 points, but canvas is refused outright on faceted, layered, and sparkline charts and on non-point marks. When both were true -- dense enough to want canvas, shaped so canvas can't run -- the author got no signal. The only symptom was a chart that felt heavy, with nothing pointing at the shape as the cause rather than the point count. Now that case warns once, above AUTO_CANVAS_REFUSAL_WARN_THRESHOLD (5,000 points). That sits well clear of the 1,000-point promotion threshold on purpose: a chart that trips 1,000 by a little renders fine as SVG, and warning there would fire on ordinary facet and layer charts that have nothing wrong with them. Explicit renderer: 'canvas' keeps its existing always-warn behavior and takes precedence, so a dense explicit request still gets one warning, not two. No dev gating -- the engine has no such convention; hosts silence or reroute via onWarn. Writing the end-to-end test surfaced a related bug: compileFaceted never called resolveMarkRenderMode at all, so renderer: 'canvas' on a facet grid was ignored with no warning, unlike every other refused shape. The resolver was unit-tested with faceted: true passed directly, so nothing caught that the compiler never set it. The facet path now resolves too (always 'svg', never stamped -- it runs for the reporting). Co-authored-by: Riley Hilliard <rileyhilliard@users.noreply.github.com>
1 parent 603b8da commit 0ccdaed

4 files changed

Lines changed: 162 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/), and this
3131

3232
### Bug Fixes
3333

34+
- **renderer:** faceted charts never resolved a rendering backend at all, so `renderer: 'canvas'` on a facet grid was ignored with no warning (every other refused shape reported it). The facet compile path now runs the same resolution and reports the refusal.
35+
- **renderer:** a chart dense enough to want the canvas layer but shaped so canvas is refused (faceted, layered, sparkline, non-point) now warns once instead of silently painting a DOM node per point. Fires above 5,000 point marks, well clear of the 1,000-point `'auto'` promotion threshold so ordinary facet/layer charts stay quiet. Hosts can reroute or silence it through `onWarn` like any other advisory warning.
3436
- **graph:** spec validation checks encoding fields against the union of keys across ALL nodes/edges instead of only the first row — a field present on some nodes/edges no longer hard-fails validation. `edgeStyle` is accepted as an edge channel, and `sort` on a quantitative field warns instead of erroring.
3537
- **graph:** node/edge tooltip race eliminated — hovering from an edge onto a node can no longer leave a stale edge tooltip; edge-hover state always clears before the node hover fires.
3638
- **legend:** rows missing the color field no longer manufacture a phantom `undefined` legend entry. In a layered spec every layer's rows flatten into the color-legend source, so a sibling layer (e.g. a diagonal reference-line) that doesn't carry the color field was seeding `String(undefined)` as a category — and with an explicit `scale.domain` it appended past the authored entries, breaking domain authority.

packages/engine/src/compile.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,6 +1364,25 @@ function compileFaceted(
13641364
// Assign animation indices across all panels
13651365
assignAnimationIndices(allMarks, resolvedAnimation);
13661366

1367+
// Rendering backend, resolved against the pooled panel marks. The canvas
1368+
// layer cannot express a facet grid, so this always answers 'svg' and never
1369+
// stamps the layout; it runs purely so the refusal is reported -- an explicit
1370+
// renderer: 'canvas' gets told it was ignored, and a dense facet grid gets
1371+
// told why it is painting every point as a DOM node.
1372+
const facetRenderModeWarnings: string[] = [];
1373+
resolveMarkRenderMode(
1374+
{
1375+
requested: options.renderer,
1376+
markType: chartSpec.markType,
1377+
pointCount: allMarks.reduce((n, m) => (m.type === 'point' ? n + 1 : n), 0),
1378+
display: chartSpec.display,
1379+
faceted: true,
1380+
layered: options.layered ?? false,
1381+
},
1382+
facetRenderModeWarnings,
1383+
);
1384+
emitSpecWarnings(facetRenderModeWarnings, options.onWarn);
1385+
13671386
// Figure-level axes are null (axes live in panels)
13681387
// Figure-level marks are the union of all panel marks (for tooltip/keyboard nav)
13691388
return {

packages/engine/src/compiler/__tests__/mark-render-mode.test.ts

Lines changed: 117 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,19 @@
44
* Pure function: it decides SVG vs canvas from the host's requested backend
55
* (the `renderer` compile option), the mark type, the compiled point count,
66
* and the chart shape (facet/layer/sparkline). Refusals collect a warning
7-
* string only when the host explicitly asked for canvas; the caller emits
8-
* them through emitSpecWarnings.
7+
* string when the host explicitly asked for canvas, or when 'auto' wanted
8+
* canvas for a chart dense enough that the SVG fallback is a real cost; the
9+
* caller emits them through emitSpecWarnings.
910
*/
1011

1112
import type { Display } from '@opendata-ai/openchart-core';
1213
import { describe, expect, it, vi } from 'vitest';
1314
import { compileChart } from '../../compile';
14-
import { AUTO_CANVAS_THRESHOLD, resolveMarkRenderMode } from '../mark-render-mode';
15+
import {
16+
AUTO_CANVAS_REFUSAL_WARN_THRESHOLD,
17+
AUTO_CANVAS_THRESHOLD,
18+
resolveMarkRenderMode,
19+
} from '../mark-render-mode';
1520

1621
/** Baseline args: a plain scatter with a huge point count. */
1722
function args(overrides: Partial<Parameters<typeof resolveMarkRenderMode>[0]> = {}) {
@@ -104,14 +109,75 @@ describe('resolveMarkRenderMode', () => {
104109
expect(warnings).toHaveLength(1);
105110
});
106111

107-
it('never warns on a refused shape when requested is auto or absent', () => {
112+
it('stays quiet on a refused shape for auto or absent below the warn threshold', () => {
108113
const warnings: string[] = [];
109-
resolveMarkRenderMode(args({ requested: 'auto', markType: 'bar' }), warnings);
110-
resolveMarkRenderMode(args({ requested: undefined, markType: 'bar' }), warnings);
111-
resolveMarkRenderMode(args({ requested: undefined, faceted: true }), warnings);
114+
const quiet = { pointCount: AUTO_CANVAS_REFUSAL_WARN_THRESHOLD };
115+
resolveMarkRenderMode(args({ ...quiet, requested: 'auto', markType: 'bar' }), warnings);
116+
resolveMarkRenderMode(args({ ...quiet, requested: undefined, markType: 'bar' }), warnings);
117+
resolveMarkRenderMode(args({ ...quiet, requested: undefined, faceted: true }), warnings);
112118
expect(warnings).toEqual([]);
113119
});
114120

121+
it('warns on an auto refusal once the chart is dense enough to hurt', () => {
122+
const warnings: string[] = [];
123+
expect(
124+
resolveMarkRenderMode(
125+
args({
126+
requested: 'auto',
127+
layered: true,
128+
pointCount: AUTO_CANVAS_REFUSAL_WARN_THRESHOLD + 1,
129+
}),
130+
warnings,
131+
),
132+
).toBe('svg');
133+
expect(warnings).toHaveLength(1);
134+
// Names the count, the shape that refused, and stays on the [openchart] prefix.
135+
expect(warnings[0]).toContain(String(AUTO_CANVAS_REFUSAL_WARN_THRESHOLD + 1));
136+
expect(warnings[0]).toContain('layered');
137+
expect(warnings[0]).toContain('[openchart]');
138+
});
139+
140+
it('treats an absent requested value like auto for the dense refusal warning', () => {
141+
const warnings: string[] = [];
142+
resolveMarkRenderMode(
143+
args({
144+
requested: undefined,
145+
faceted: true,
146+
pointCount: AUTO_CANVAS_REFUSAL_WARN_THRESHOLD + 1,
147+
}),
148+
warnings,
149+
);
150+
expect(warnings).toHaveLength(1);
151+
expect(warnings[0]).toContain('faceted');
152+
});
153+
154+
it('does not warn about density when the shape supports canvas', () => {
155+
// A plain dense scatter is promoted, not refused: nothing to report.
156+
const warnings: string[] = [];
157+
expect(
158+
resolveMarkRenderMode(
159+
args({ requested: 'auto', pointCount: AUTO_CANVAS_REFUSAL_WARN_THRESHOLD + 1 }),
160+
warnings,
161+
),
162+
).toBe('canvas');
163+
expect(warnings).toEqual([]);
164+
});
165+
166+
it('warns once, not twice, when an explicit canvas request is also dense', () => {
167+
const warnings: string[] = [];
168+
resolveMarkRenderMode(
169+
args({
170+
requested: 'canvas',
171+
layered: true,
172+
pointCount: AUTO_CANVAS_REFUSAL_WARN_THRESHOLD + 1,
173+
}),
174+
warnings,
175+
);
176+
expect(warnings).toHaveLength(1);
177+
// The explicit-request phrasing wins; it already tells the author what to do.
178+
expect(warnings[0]).toContain('is not supported');
179+
});
180+
115181
it('promotes auto to canvas above the threshold', () => {
116182
expect(
117183
resolveMarkRenderMode(args({ requested: 'auto', pointCount: AUTO_CANVAS_THRESHOLD + 1 })),
@@ -242,6 +308,50 @@ describe('compileChart markRenderMode', () => {
242308
}
243309
});
244310

311+
it('warns through onWarn when a dense faceted chart silently falls back to svg', () => {
312+
// The case the warning exists for: the author asked for nothing, the chart
313+
// is far too dense for SVG, and the facet shape is what refused canvas.
314+
const onWarn = vi.fn();
315+
const dense = Array.from({ length: AUTO_CANVAS_REFUSAL_WARN_THRESHOLD + 200 }, (_, i) => ({
316+
x: i % 500,
317+
y: i,
318+
g: i % 2 === 0 ? 'a' : 'b',
319+
}));
320+
compileChart(
321+
{
322+
mark: 'point',
323+
data: dense,
324+
encoding: { ...SCATTER_ENCODING, facet: { field: 'g', type: 'nominal' as const } },
325+
width: 600,
326+
height: 400,
327+
},
328+
{ width: 600, height: 400, onWarn },
329+
);
330+
const denseWarnings = onWarn.mock.calls.filter((c) =>
331+
String(c[0]).includes('point marks as SVG'),
332+
);
333+
expect(denseWarnings).toHaveLength(1);
334+
expect(String(denseWarnings[0][0])).toContain('faceted');
335+
});
336+
337+
it('stays silent for a small faceted chart with no renderer requested', () => {
338+
const onWarn = vi.fn();
339+
compileChart(
340+
{
341+
mark: 'point',
342+
data: SCATTER_DATA.map((row, i) => ({ ...row, g: i % 2 === 0 ? 'a' : 'b' })),
343+
encoding: { ...SCATTER_ENCODING, facet: { field: 'g', type: 'nominal' as const } },
344+
width: 600,
345+
height: 400,
346+
},
347+
{ width: 600, height: 400, onWarn },
348+
);
349+
const denseWarnings = onWarn.mock.calls.filter((c) =>
350+
String(c[0]).includes('point marks as SVG'),
351+
);
352+
expect(denseWarnings).toEqual([]);
353+
});
354+
245355
it('warns on and strips the removed mark.render spec field', () => {
246356
const onWarn = vi.fn();
247357
const layout = compileChart(

packages/engine/src/compiler/mark-render-mode.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,27 @@
55
* High-cardinality scatter plots are the only case where the trade is worth
66
* it, so canvas is refused for every other mark type and for the layout shapes
77
* the canvas layer cannot express (facets, layers, sparklines). Refusals are
8-
* advisory: they warn only when the author explicitly asked for canvas, never
9-
* for `'auto'`.
8+
* advisory: an explicit `'canvas'` request always warns, and an `'auto'`
9+
* refusal warns only once the chart is dense enough for the SVG fallback to
10+
* actually hurt (AUTO_CANVAS_REFUSAL_WARN_THRESHOLD).
1011
*/
1112

1213
import type { Display } from '@opendata-ai/openchart-core';
1314

1415
/** Point count above which `'auto'` prefers canvas over SVG. */
1516
export const AUTO_CANVAS_THRESHOLD = 1000;
1617

18+
/**
19+
* Point count above which an `'auto'` refusal is worth telling the author about.
20+
*
21+
* Set well clear of AUTO_CANVAS_THRESHOLD on purpose. A chart that trips the
22+
* threshold by a little renders fine as SVG, and warning there would fire on
23+
* ordinary faceted and layered charts that have nothing wrong with them. This
24+
* is the "you are painting enough DOM nodes to feel it" line, so the advice is
25+
* actionable rather than noise.
26+
*/
27+
export const AUTO_CANVAS_REFUSAL_WARN_THRESHOLD = 5000;
28+
1729
/** Inputs to mark render mode resolution. */
1830
export interface MarkRenderModeArgs {
1931
/** The host's requested backend (`CompileOptions.renderer`), or undefined for `'auto'`. */
@@ -36,8 +48,9 @@ export interface MarkRenderModeArgs {
3648
* Precedence:
3749
* 1. Explicit `'svg'` wins outright.
3850
* 2. Unsupported shape (non-point mark, facet, layer, sparkline) falls back to
39-
* SVG, pushing one warning onto `warnings` only when the author explicitly
40-
* asked for canvas.
51+
* SVG, pushing one warning onto `warnings` when the author explicitly asked
52+
* for canvas, or when `'auto'` wanted canvas for a chart dense enough that
53+
* the SVG fallback is a real cost.
4154
* 3. Explicit `'canvas'` wins at any point count.
4255
* 4. `'auto'` (or absent) promotes to canvas above AUTO_CANVAS_THRESHOLD.
4356
*
@@ -59,6 +72,13 @@ export function resolveMarkRenderMode(
5972
warnings.push(
6073
`Chart warning: renderer "canvas" is not supported ${refusal}; rendering marks as SVG instead.`,
6174
);
75+
} else if (pointCount > AUTO_CANVAS_REFUSAL_WARN_THRESHOLD) {
76+
// The author asked for nothing and got the slow path. Without this the
77+
// only symptom is a chart that feels heavy, with no hint that the shape
78+
// (not the point count) is what kept it on SVG.
79+
warnings.push(
80+
`[openchart] Rendering ${pointCount} point marks as SVG: the canvas mark layer is not supported ${refusal}. Expect slow paint and interaction. Reduce the point count, or restructure so the dense marks compile as a single unlayered, unfaceted point chart.`,
81+
);
6282
}
6383
return 'svg';
6484
}

0 commit comments

Comments
 (0)