Skip to content

Commit 44ffe6e

Browse files
cquil11claude
andcommitted
fix(scatter): keep line labels in the foreground
Inference performance scatter charts create line labels inside the rooflines layer, which renders before the dot groups so roofline paths stay behind the points. That left the labels painted under the scatter points and overlay marks. Add a trailing custom layer that re-raises every `.line-label` to the end of the zoomGroup after all layers render (and on zoom), so they always read as foreground — mirroring GPUGraph, whose line-label layer already renders last. `.raise()` only changes z-order; label placement and the existing de-overlap (hide-on-collision for interactivity, vertical nudge for TTFT/E2EL) are untouched, so labels still never overlap one another. Selects overlay line labels (`overlay-*`) too, so unofficial-run overlays get the same treatment. Adds an E2E assertion that visible labels follow the dot groups in DOM order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent bc2890e commit 44ffe6e

2 files changed

Lines changed: 48 additions & 0 deletions

File tree

packages/app/cypress/e2e/line-labels.cy.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,30 @@ describe('Line Labels Toggle', () => {
3939
);
4040
});
4141

42+
it('line labels render in the foreground, after the scatter points', () => {
43+
// Labels were toggled on in the test above and remain on here.
44+
cy.get('[data-testid="scatter-graph"] svg g.line-label').should('have.length.greaterThan', 0);
45+
46+
cy.get('[data-testid="scatter-graph"] svg').then(($svg) => {
47+
const svg = $svg[0];
48+
const dots = svg.querySelectorAll('.dot-group');
49+
const labels = svg.querySelectorAll('g.line-label');
50+
expect(dots.length, 'scatter dot groups exist').to.be.greaterThan(0);
51+
expect(labels.length, 'line labels exist').to.be.greaterThan(0);
52+
53+
// Every label must paint after every dot group. Comparing the *last* dot
54+
// group against the *first* label is sufficient: if the earliest label
55+
// follows the latest dot in document order, all labels are in front.
56+
const lastDot = dots.item(dots.length - 1)!;
57+
const firstLabel = labels.item(0)!;
58+
const relation = lastDot.compareDocumentPosition(firstLabel);
59+
expect(
60+
relation & Node.DOCUMENT_POSITION_FOLLOWING,
61+
'line label follows the scatter points in DOM order (foreground)',
62+
).to.be.greaterThan(0);
63+
});
64+
});
65+
4266
it('toggling Line Labels off removes label elements', () => {
4367
cy.get('#scatter-line-labels').click();
4468
cy.get('#scatter-line-labels').should('have.attr', 'data-state', 'unchecked');

packages/app/src/components/inference/ui/ScatterGraph.tsx

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,9 +1760,33 @@ const ScatterGraph = React.memo(
17601760
},
17611761
};
17621762

1763+
// ── Last layer: keep line labels in the foreground ──
1764+
// Line labels are created inside the rooflines layer, which must render
1765+
// *before* the dot groups so the roofline paths sit behind the points.
1766+
// That ordering would otherwise leave the labels painted under the scatter
1767+
// points and the overlay marks. Re-raise every `.line-label` to the end of
1768+
// the zoomGroup after all other layers have rendered (and on each zoom) so
1769+
// they always read as foreground. `.raise()` only reorders z-index — label
1770+
// x/y placement (and the hide-on-collision / vertical-nudge de-overlap in
1771+
// the rooflines layer) is untouched, so labels still never overlap one
1772+
// another. Mirrors GPUGraph, whose line-label layer is already rendered
1773+
// last. Selects overlay line labels (key `overlay-*`) too, so unofficial
1774+
// run overlays get the same foreground treatment.
1775+
const lineLabelForegroundLayer: CustomLayerConfig = {
1776+
type: 'custom',
1777+
key: 'line-label-foreground',
1778+
render: (zoomGroup) => {
1779+
zoomGroup.selectAll('.line-label').raise();
1780+
},
1781+
onZoom: (zoomGroup) => {
1782+
zoomGroup.selectAll('.line-label').raise();
1783+
},
1784+
};
1785+
17631786
const result: LayerConfig<InferenceData>[] = [rooflineLayer, scatterLayer];
17641787
if (overlayLayer) result.push(overlayLayer);
17651788
result.push(speedOverlayLayer);
1789+
result.push(lineLabelForegroundLayer);
17661790
return result;
17671791
}, [
17681792
rooflines,

0 commit comments

Comments
 (0)