From 9db8a9641502a2ac8cd425756881816d6a55e713 Mon Sep 17 00:00:00 2001 From: Riley Hilliard Date: Sat, 4 Jul 2026 07:43:28 -0500 Subject: [PATCH] fix(engine): compile leaf layers in the primary layout's chart area compileLayer compiled each leaf spec raw, without the layer-level chrome, legend, and theme that buildPrimarySpec merges into the primary spec. Each leaf therefore computed its own chart area: no title/subtitle reservation, a default legend position instead of the layer's, and default theme fonts. Leaf marks landed in that private coordinate space while axes, gridlines, and annotations rendered in the primary's, so on a layered scatter with a title and top legend every bubble drew up-and-left of where the axis said it should be (about 130px vertical and 128px horizontal drift at 766px). Freeze the primary layout's area into CompileOptions.frozenChartArea for leaf compiles so every layer's scales and marks share the primary's coordinate space, and inherit the layer-level theme/darkMode so leaf mark styling matches the primary. --- packages/core/src/types/layout.ts | 8 ++ .../src/__tests__/compile-layer.test.ts | 94 +++++++++++++++++++ packages/engine/src/compile.ts | 4 +- packages/engine/src/compile/layer.ts | 12 ++- 4 files changed, 116 insertions(+), 2 deletions(-) diff --git a/packages/core/src/types/layout.ts b/packages/core/src/types/layout.ts index 4401a7cf..b6f23a07 100644 --- a/packages/core/src/types/layout.ts +++ b/packages/core/src/types/layout.ts @@ -1448,6 +1448,14 @@ export interface CompileOptions { measureText?: MeasureTextFn; /** Extra pixels to reserve on the right margin for a secondary y-axis. Set by compileLayer when resolve.scale.y is 'independent'. */ rightAxisReserve?: number; + /** + * Use this chart drawing area instead of computing one from chrome, axes, + * and legend reservations. Set by compileLayer when compiling leaf layers so + * their marks land in the primary layout's coordinate space; leaf specs lack + * the layer-level chrome/legend/theme and would otherwise compute a + * different area than the one the axes are rendered in. + */ + frozenChartArea?: Rect; } /** Extended compile options for table visualizations. */ diff --git a/packages/engine/src/__tests__/compile-layer.test.ts b/packages/engine/src/__tests__/compile-layer.test.ts index 62f2145e..8e61d3b4 100644 --- a/packages/engine/src/__tests__/compile-layer.test.ts +++ b/packages/engine/src/__tests__/compile-layer.test.ts @@ -753,4 +753,98 @@ describe('compileLayer', () => { void l0RectCount; // used for conceptual clarity above }); + + it('positions leaf-layer marks in the primary layout coordinate space', () => { + // Regression: leaves were compiled without the layer-level chrome, legend, + // and theme that buildPrimarySpec merges into the primary spec, so their + // marks landed in a different chart area than the rendered axes. A scatter + // with a title + top legend drew every bubble up-and-left of where the + // axis said it should be. + const spec: LayerSpec = { + chrome: { title: 'Scatter with diagonal', subtitle: 'primary vs leaf areas' }, + legend: { position: 'top' }, + layer: [ + { + mark: { type: 'point', filled: true }, + data: [ + { x: 10, y: 10, group: 'a' }, + { x: 50, y: 50, group: 'b' }, + { x: 90, y: 90, group: 'a' }, + ], + encoding: { + x: { + field: 'x', + type: 'quantitative' as const, + scale: { domain: [0, 100], nice: false }, + }, + y: { + field: 'y', + type: 'quantitative' as const, + scale: { domain: [0, 100], nice: false }, + }, + color: { field: 'group', type: 'nominal' as const }, + }, + }, + { + mark: { type: 'rule', stroke: '#888', strokeWidth: 1 }, + data: [{ x: 0, y: 0, x2: 100, y2: 100 }], + encoding: { + x: { + field: 'x', + type: 'quantitative' as const, + scale: { domain: [0, 100], nice: false }, + }, + y: { + field: 'y', + type: 'quantitative' as const, + scale: { domain: [0, 100], nice: false }, + }, + x2: { field: 'x2' }, + y2: { field: 'y2' }, + }, + }, + ], + }; + + const layout = compileLayer(spec, compileOpts); + + // Axis-implied pixel position for a data value, derived from rendered ticks. + const xTicks = layout.axes.x?.ticks ?? []; + const yTicks = layout.axes.y?.ticks ?? []; + expect(xTicks.length).toBeGreaterThanOrEqual(2); + expect(yTicks.length).toBeGreaterThanOrEqual(2); + const xFirst = xTicks[0]; + const xLast = xTicks[xTicks.length - 1]; + const yFirst = yTicks[0]; + const yLast = yTicks[yTicks.length - 1]; + const axisX = (v: number) => + xFirst.position + + ((v - (xFirst.value as number)) / ((xLast.value as number) - (xFirst.value as number))) * + (xLast.position - xFirst.position); + const axisY = (v: number) => + yFirst.position + + ((v - (yFirst.value as number)) / ((yLast.value as number) - (yFirst.value as number))) * + (yLast.position - yFirst.position); + + const points = layout.marks.filter((m) => m.type === 'point'); + expect(points.length).toBe(3); + const expected = [10, 50, 90]; + points.forEach((mark, i) => { + const pm = mark as unknown as { cx: number; cy: number }; + expect(Math.abs(pm.cx - axisX(expected[i]))).toBeLessThan(1.5); + expect(Math.abs(pm.cy - axisY(expected[i]))).toBeLessThan(1.5); + }); + + // The diagonal rule from the second leaf must span the same coordinate space. + const rule = layout.marks.find((m) => m.type === 'rule') as unknown as + | { x1: number; y1: number; x2: number; y2: number } + | undefined; + expect(rule).toBeDefined(); + if (rule) { + expect(Math.abs(rule.x1 - axisX(0))).toBeLessThan(1.5); + expect(Math.abs(rule.x2 - axisX(100))).toBeLessThan(1.5); + expect(Math.abs(rule.y1 - axisY(0))).toBeLessThan(1.5); + expect(Math.abs(rule.y2 - axisY(100))).toBeLessThan(1.5); + } + }); }); diff --git a/packages/engine/src/compile.ts b/packages/engine/src/compile.ts index 731cdda5..70f35faa 100644 --- a/packages/engine/src/compile.ts +++ b/packages/engine/src/compile.ts @@ -610,7 +610,9 @@ export function compileChart(spec: unknown, options: CompileOptions): ChartLayou plan.xAxisExtent, ); const dims = computeDimensions(chartSpec, options, legendStub, theme, strategy, watermark, plan); - const chartArea = dims.chartArea; + // A frozen area (set by compileLayer for leaf layers) overrides the computed + // one so all layers share the primary layout's coordinate space. + const chartArea = options.frozenChartArea ?? dims.chartArea; // Place legend in its final position relative to the computed chart area. const finalLegend = placeLegend( diff --git a/packages/engine/src/compile/layer.ts b/packages/engine/src/compile/layer.ts index 0b887233..3e1dcb0b 100644 --- a/packages/engine/src/compile/layer.ts +++ b/packages/engine/src/compile/layer.ts @@ -70,8 +70,18 @@ export function compileLayer( })); indexedLeaves.sort((a, b) => a.zIndex - b.zIndex); + // Leaf specs lack the layer-level chrome/legend, so left alone they compute + // a different chart area than the primary and their marks drift off the + // axes. Freeze the primary's area (and inherit its theme) for every leaf. + const leafOptions: CompileOptions = { ...options, frozenChartArea: primaryLayout.area }; + for (const { leaf } of indexedLeaves) { - const leafLayout = compileChart(leaf as unknown, options); + const themedLeaf = { + ...(leaf as ChartSpec), + theme: (leaf as ChartSpec).theme ?? spec.theme, + darkMode: (leaf as ChartSpec).darkMode ?? spec.darkMode, + }; + const leafLayout = compileChart(themedLeaf as unknown, leafOptions); allMarks.push(...leafLayout.marks);