From d43332c6debb9ee6f24e5f29af28a907834b41f2 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 12 Aug 2026 11:09:40 -0700 Subject: [PATCH] fix(workflow): stop the coloured knob leaving a barb at each shoulder tip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The connection knob is a recolour of one stretch of the card outline, so its path has to be the outline's own path. Two things pulled it off: Its span was cut at the exact point the bulge falls under the visibility threshold, which is not one of the points the silhouette sampled — so the knob sat on a grid of its own. And a span clamped its first and last control points to the bare perimeter tangent, where the silhouette derives every control point from the samples either side of it, so those two segments bowed differently from the curve they were painted over. The knob was left still flat where the silhouette had already begun its descent, and the uncovered sliver of dark stroke read as a small spur poking off the shoulder. It is clearest on the Error output's outer shoulder, against the card's bottom-right corner. Measure the span against the interval the silhouette resamples the bulge over rather than in whole pixels, and sample a step wide on each side before trimming back, so every emitted segment has the neighbours the silhouette had. The knob's commands now come out identical to the silhouette's, which the tests pin — including for merged intervals and odd tab lengths, where measuring in whole pixels would still have landed half a step off. The painted footprint is unchanged. --- .../workflow-block-border-mount.test.tsx | 56 ++++++++++++++ .../workflow-block/workflow-block-border.tsx | 75 +++++++++++++------ 2 files changed, 109 insertions(+), 22 deletions(-) diff --git a/packages/workflow-renderer/src/workflow-block/workflow-block-border-mount.test.tsx b/packages/workflow-renderer/src/workflow-block/workflow-block-border-mount.test.tsx index fc69bf9bac1..24eadd3b8ab 100644 --- a/packages/workflow-renderer/src/workflow-block/workflow-block-border-mount.test.tsx +++ b/packages/workflow-renderer/src/workflow-block/workflow-block-border-mount.test.tsx @@ -265,6 +265,62 @@ describe('WorkflowBlockBorder mount', () => { expect(path?.getAttribute('d')?.length ?? 0).toBeGreaterThan(0) }) + /** + * The knob is a recolour of one stretch of the outline, so its commands have + * to BE the outline's commands. When the two were generated off different + * sample grids they disagreed by a fraction of a pixel at the shoulder tip, + * and the sliver of dark stroke the knob failed to cover read as a barb + * hanging off it. + * + * Only holds where a knob's own bulge is the tallest thing under it: a knob + * is painted from its own feature alone, while the silhouette takes the max + * over all of them, so two bulges tall enough to overlap genuinely part + * company. Every port layout the editor builds keeps them clear of one + * another. + */ + const expectKnobsOnSilhouette = (host: HTMLElement) => { + const silhouette = host.querySelector('svg > path')?.getAttribute('d') ?? '' + const knobs = Array.from(host.querySelectorAll('svg > g[clip-path] path')) + expect(knobs.length).toBeGreaterThan(0) + + for (const knob of knobs) { + const commands = (knob.getAttribute('d') ?? '').match(/C[^MLAC]+/g) ?? [] + expect(commands.length).toBeGreaterThan(0) + for (const command of commands) { + expect(silhouette).toContain(command.trim()) + } + } + } + + it('paints every coloured knob on the silhouette’s own curve', () => { + const { host } = mount( +
+ +
+ ) + expectKnobsOnSilhouette(host) + }) + + it('keeps knobs on the curve when their resampled intervals merge', () => { + /* Two coloured ports close enough that `relativeIntervals` merges them + resample as one stretch, on a grid neither port's own bounds predict. + The odd tab length is the other half of the same trap: a knob measured + in whole pixels rather than against the interval it sits in lands half a + step off whenever the bulge does not divide evenly. */ + const crowdedPorts: WorkflowBorderPort[] = [ + { id: 'target', side: 'left', position: 'center', plateau: 33 }, + { id: 'row-a', side: 'right', position: 60, plateau: 24, color: 'var(--brand-accent)' }, + { id: 'row-b', side: 'right', position: 87.5, plateau: 24, color: 'var(--text-error)' }, + { id: 'row-c', side: 'right', position: 140, plateau: 23, color: 'var(--warning)' }, + ] + const { host } = mount( +
+ +
+ ) + expectKnobsOnSilhouette(host) + }) + it('paints a tall selector card across floating-point segment seams', () => { const selectorPorts: WorkflowBorderPort[] = [ { diff --git a/packages/workflow-renderer/src/workflow-block/workflow-block-border.tsx b/packages/workflow-renderer/src/workflow-block/workflow-block-border.tsx index be0fa4147ca..e649c30241d 100644 --- a/packages/workflow-renderer/src/workflow-block/workflow-block-border.tsx +++ b/packages/workflow-renderer/src/workflow-block/workflow-block-border.tsx @@ -602,6 +602,18 @@ const findQuietStart = (intervals: ActiveInterval[], perimeterLength: number) => return largestGap > 0 ? start : 0 } +/** Flat run resampled either side of a bulge, so its tail rejoins the edge. */ +const BULGE_INTERVAL_SLACK_PX = 4 + +/** + * How far either side of a bulge's centre the outline is resampled — wider than + * the bulge itself, so the curve has flat perimeter to settle onto. It also + * fixes the sample grid a knob has to land on, which is why `visibleBulgeHalf` + * measures against it. + */ +const bulgeIntervalHalf = (plateau: number, shoulder: number) => + plateau / 2 + shoulder + BULGE_INTERVAL_SLACK_PX + const relativeIntervals = (features: BulgeFeature[], startS: number, perimeterLength: number) => { const intervals = features .filter( @@ -614,7 +626,7 @@ const relativeIntervals = (features: BulgeFeature[], startS: number, perimeterLe ) .map((feature) => { const center = modulo(feature.center - startS, perimeterLength) - const half = feature.plateau / 2 + feature.shoulder + 4 + const half = bulgeIntervalHalf(feature.plateau, feature.shoulder) return { start: center - half, end: center + half } }) .filter((interval) => interval.end > 0 && interval.start < perimeterLength) @@ -671,19 +683,32 @@ const displacementAt = ( * Where a bulge stops being drawn, by inverting the shoulder's easing at the * visibility threshold. The mathematical footprint (`plateau/2 + shoulder`) * overshoots this, because the tail is cut off once it flattens out. + * + * Pulled back to the silhouette's own sample points. `relativeIntervals` + * resamples a bulge over `bulgeIntervalHalf` either side of its centre, split + * into whole steps of about `SAMPLE_SPACING_PX` — so the crossing itself falls + * between two of them. A knob cut there sits on a grid of its own and drifts + * off the curve it is recolouring; see `buildSpanPath` for what that costs. + * Retreating to the last sample at or beyond the crossing keeps the knob on + * the silhouette's points whatever the bulge measures. */ const visibleBulgeHalf = (plateau: number, shoulder: number, peak: number) => { const plateauHalf = plateau / 2 - if (peak <= BULGE_VISIBLE_THRESHOLD_PX || shoulder <= 0) return plateauHalf - const target = 1 - BULGE_VISIBLE_THRESHOLD_PX / peak - let low = 0 - let high = 1 - for (let step = 0; step < 24; step++) { - const mid = (low + high) / 2 - if (smootherstep(mid) < target) low = mid - else high = mid + let crossing = plateauHalf + if (peak > BULGE_VISIBLE_THRESHOLD_PX && shoulder > 0) { + const target = 1 - BULGE_VISIBLE_THRESHOLD_PX / peak + let low = 0 + let high = 1 + for (let step = 0; step < 24; step++) { + const mid = (low + high) / 2 + if (smootherstep(mid) < target) low = mid + else high = mid + } + crossing = plateauHalf + high * shoulder } - return plateauHalf + high * shoulder + const intervalHalf = bulgeIntervalHalf(plateau, shoulder) + const step = (intervalHalf * 2) / Math.max(2, Math.ceil((intervalHalf * 2) / SAMPLE_SPACING_PX)) + return intervalHalf - Math.floor((intervalHalf - crossing) / step) * step } const appendExactInterval = ( @@ -764,6 +789,7 @@ const appendActiveInterval = ( `C${control1.x.toFixed(2)} ${control1.y.toFixed(2)} ${control2.x.toFixed(2)} ${control2.y.toFixed(2)} ${next.x.toFixed(2)} ${next.y.toFixed(2)}` ) } + return points } /** @@ -777,6 +803,16 @@ const appendActiveInterval = ( * off its own knob, leaving a crescent of base colour showing inside it. Giving * the knob its own path removes the arc-length bookkeeping altogether: the * colour is drawn on the same points the silhouette was. + * + * Sampled a step wide on each side and then trimmed back to the span. Every + * control point is derived from the samples either side of it, so a span that + * stopped at its own ends would have to clamp its first and last to the bare + * perimeter tangent — and those two segments would bow differently from the + * outline they are painted over. The knob was then still flat where the + * silhouette had begun its descent, and the uncovered dark stroke read as a + * barb off the shoulder tip. Borrowing a sample beyond each end gives every + * emitted segment the neighbours the silhouette had, so the two agree command + * for command. */ const buildSpanPath = ( geometry: PerimeterGeometry, @@ -787,18 +823,13 @@ const buildSpanPath = ( ) => { const length = toS - fromS if (length <= 0) return '' - const located = pointAtArcLength(geometry, fromS) - const displacement = displacementAt( - modulo(fromS, geometry.length), - features, - geometry.length, - maximum - ) - const originX = located.point.x + located.point.nx * displacement - const originY = located.point.y + located.point.ny * displacement - const commands = [`M${originX.toFixed(2)} ${originY.toFixed(2)}`] - appendActiveInterval(commands, geometry, features, maximum, fromS, { start: 0, end: length }) - return commands.join(' ') + const commands: string[] = [] + const points = appendActiveInterval(commands, geometry, features, maximum, fromS, { + start: -SAMPLE_SPACING_PX, + end: length + SAMPLE_SPACING_PX, + }) + const origin = points[1] + return [`M${origin.x.toFixed(2)} ${origin.y.toFixed(2)}`, ...commands.slice(1, -1)].join(' ') } const buildPiecewisePath = (