From e53684ea67c15644d1bc7362cd37e785228b670c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 05:39:23 +0000 Subject: [PATCH 1/2] fix(curves): correct curved-line direction and dribble wave orientation Curved movement/passing lines were rendered wrong in two ways and dribble waves bent incorrectly: - curvedPath built only a half curve for a 3-point line (Q over the first span, then a straight L to the end) and offset the control point along the angle bisector pointing INTO the bend, so the curve bulged toward the inside instead of through the waypoint. Replace with a midpoint-quadratic spline: each interior waypoint is the control point and spans join at segment midpoints, so the curve passes smoothly through the waypoints and bows outward as drawn. Endpoints are preserved. - dribblingPath computed the wave perpendicular from a segIdx that was never advanced (always segment 0), so after a bend the zig-zag pointed the wrong way. pointAlongPath now returns the local unit tangent and the wave uses it. Tests: tests/curves.spec.js asserts the curve geometry (single quadratic with the waypoint as control, apex on the same side as the waypoint, endpoints and interior controls preserved) and that dribble waves alternate sides and follow bends perpendicular to the local segment. The three discriminating cases were verified to fail on the old implementation. Full suite: 37 passed. --- src/court/curves.js | 110 ++++++++++--------------------- tests/curves.spec.js | 152 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 187 insertions(+), 75 deletions(-) create mode 100644 tests/curves.spec.js diff --git a/src/court/curves.js b/src/court/curves.js index 96ac014..961aae6 100644 --- a/src/court/curves.js +++ b/src/court/curves.js @@ -4,10 +4,14 @@ */ /** - * Compute angle bisector control points for quadratic bezier interpolation. - * For waypoints P0, P1, ..., Pn: - * At each interior point Pi, compute angle bisector of incoming/outgoing vectors. - * + * Smooth quadratic spline through a set of waypoints. + * + * Each interior waypoint becomes the control point of a quadratic Bézier, and + * consecutive spans are joined at the segment midpoints so the curve passes + * smoothly *through* the waypoints and bulges toward them (the intuitive + * "outward" direction the author drew). The first span starts at P0 and the + * last span ends exactly on Pn, so endpoints are honored. + * * @param {Array<{x:number, y:number}>} points - Waypoints * @returns {string} SVG path data */ @@ -17,64 +21,19 @@ export function curvedPath(points) { return `M ${points[0].x},${points[0].y} L ${points[1].x},${points[1].y}`; } + const n = points.length; let d = `M ${points[0].x},${points[0].y}`; - for (let i = 1; i < points.length - 1; i++) { - const prev = points[i - 1]; - const curr = points[i]; - const next = points[i + 1]; - - // Vectors from curr to prev and curr to next - const v1x = prev.x - curr.x; - const v1y = prev.y - curr.y; - const v2x = next.x - curr.x; - const v2y = next.y - curr.y; - - const len1 = Math.sqrt(v1x * v1x + v1y * v1y); - const len2 = Math.sqrt(v2x * v2x + v2y * v2y); - - if (len1 === 0 || len2 === 0) { - d += ` L ${curr.x},${curr.y}`; - continue; - } - - // Normalized vectors - const n1x = v1x / len1, n1y = v1y / len1; - const n2x = v2x / len2, n2y = v2y / len2; - - // Bisector direction (pointing "outward" from the angle) - const bx = n1x + n2x; - const by = n1y + n2y; - const bLen = Math.sqrt(bx * bx + by * by); - - if (bLen < 0.001) { - // Points are collinear, use straight line to midpoints - const mid1x = (prev.x + curr.x) / 2; - const mid1y = (prev.y + curr.y) / 2; - d += ` L ${mid1x},${mid1y} L ${curr.x},${curr.y}`; - continue; - } - - // Control point offset along bisector - const controlDist = Math.min(len1, len2) * 0.3; - const cx = curr.x + (bx / bLen) * controlDist; - const cy = curr.y + (by / bLen) * controlDist; - - // Midpoints for smooth transitions - const mid1x = (prev.x + curr.x) / 2; - const mid1y = (prev.y + curr.y) / 2; - - if (i === 1) { - d += ` Q ${cx},${cy} ${curr.x},${curr.y}`; - } else { - d += ` Q ${cx},${cy} ${curr.x},${curr.y}`; - } + for (let i = 1; i < n - 1; i++) { + const ctrl = points[i]; + // End each span at the midpoint to the next waypoint, except the final span + // which lands on the last point. + const end = (i === n - 2) + ? points[n - 1] + : { x: (points[i].x + points[i + 1].x) / 2, y: (points[i].y + points[i + 1].y) / 2 }; + d += ` Q ${ctrl.x},${ctrl.y} ${end.x},${end.y}`; } - // Last segment - const last = points[points.length - 1]; - d += ` L ${last.x},${last.y}`; - return d; } @@ -125,27 +84,21 @@ export function dribblingPath(points, scale = 1) { // Walk along path and create wave control points let d = `M ${points[0].x},${points[0].y}`; let walked = 0; - let segIdx = 0; - let segWalked = 0; let side = 1; for (let w = 0; w < numWaves; w++) { const halfTarget = walked + segLen / 2; const endTarget = walked + segLen; - // Find midpoint along path + // Midpoint of this wave, plus the tangent of whichever segment it lies on — + // so the perpendicular follows bends in the path instead of always using + // the first segment's direction. const mid = pointAlongPath(segments, halfTarget); const end = pointAlongPath(segments, endTarget); - // Direction at midpoint - const seg = segments[Math.min(segIdx, segments.length - 1)]; - const dx = seg.to.x - seg.from.x; - const dy = seg.to.y - seg.from.y; - const len = Math.sqrt(dx * dx + dy * dy) || 1; - - // Perpendicular - const px = -dy / len; - const py = dx / len; + // Perpendicular to the local tangent at the midpoint + const px = -mid.ty; + const py = mid.tx; const cx = mid.x + px * amplitude * side; const cy = mid.y + py * amplitude * side; @@ -159,20 +112,27 @@ export function dribblingPath(points, scale = 1) { } /** - * Find a point at a given distance along a series of segments. + * Find a point at a given distance along a series of segments, together with the + * unit tangent (tx, ty) of the segment it falls on. */ function pointAlongPath(segments, distance) { let remaining = distance; - for (const seg of segments) { + for (let i = 0; i < segments.length; i++) { + const seg = segments[i]; const dx = seg.to.x - seg.from.x; const dy = seg.to.y - seg.from.y; const len = Math.sqrt(dx * dx + dy * dy); - if (remaining <= len || seg === segments[segments.length - 1]) { + const tx = len > 0 ? dx / len : 0; + const ty = len > 0 ? dy / len : 0; + if (remaining <= len || i === segments.length - 1) { const t = len > 0 ? Math.min(remaining / len, 1) : 0; - return { x: seg.from.x + dx * t, y: seg.from.y + dy * t }; + return { x: seg.from.x + dx * t, y: seg.from.y + dy * t, tx, ty }; } remaining -= len; } const last = segments[segments.length - 1]; - return { x: last.to.x, y: last.to.y }; + const dx = last.to.x - last.from.x; + const dy = last.to.y - last.from.y; + const len = Math.sqrt(dx * dx + dy * dy) || 1; + return { x: last.to.x, y: last.to.y, tx: dx / len, ty: dy / len }; } diff --git a/tests/curves.spec.js b/tests/curves.spec.js new file mode 100644 index 0000000..8c1410a --- /dev/null +++ b/tests/curves.spec.js @@ -0,0 +1,152 @@ +/** + * Geometric unit tests for the curve renderers (pure functions — no browser). + * + * Focus: the reported rendering bugs — + * - curved movement lines bulged toward the *inside* of a bend (and were + * half-straight) instead of passing smoothly through the waypoint; + * - dribbling waves used the first segment's direction for every wave, so + * after a bend the zig-zag pointed the wrong way. + */ + +import { test, expect } from '@playwright/test'; +import { curvedPath, straightPath, dribblingPath } from '../src/court/curves.js'; + +// ─── Tiny SVG-path sampler (supports the M / L / Q we emit) ───────────────────── + +/** Parse "M/L/Q" path data into a list of commands with numeric args. */ +function parsePath(d) { + const tokens = d.match(/[MLQ][^MLQ]*/g) || []; + return tokens.map(tok => { + const cmd = tok[0]; + const nums = (tok.slice(1).match(/-?\d+(?:\.\d+)?/g) || []).map(Number); + return { cmd, nums }; + }); +} + +/** Collect the anchor points the path passes through (M start + each L/Q end). */ +function pathAnchors(d) { + const pts = []; + for (const { cmd, nums } of parsePath(d)) { + if (cmd === 'M' || cmd === 'L') pts.push({ x: nums[0], y: nums[1] }); + else if (cmd === 'Q') pts.push({ x: nums[2], y: nums[3] }); + } + return pts; +} + +/** Quadratic control points (cx,cy) of every Q command. */ +function quadControls(d) { + return parsePath(d) + .filter(c => c.cmd === 'Q') + .map(c => ({ x: c.nums[0], y: c.nums[1] })); +} + +/** Signed side of point p relative to directed line a→b (>0 left, <0 right). */ +function sideOf(a, b, p) { + return (b.x - a.x) * (p.y - a.y) - (b.y - a.y) * (p.x - a.x); +} + +/** Sample a single quadratic Bézier at parameter t. */ +function quadAt(p0, c, p1, t) { + const u = 1 - t; + return { + x: u * u * p0.x + 2 * u * t * c.x + t * t * p1.x, + y: u * u * p0.y + 2 * u * t * c.y + t * t * p1.y, + }; +} + +// ─── curvedPath ───────────────────────────────────────────────────────────────── + +test.describe('curvedPath', () => { + test('two points → a straight segment', () => { + const d = curvedPath([{ x: 0, y: 0 }, { x: 10, y: 0 }]); + expect(d).toBe('M 0,0 L 10,0'); + }); + + test('three points → a single quadratic with the waypoint as control', () => { + const p0 = { x: 0, y: 0 }, p1 = { x: 5, y: 4 }, p2 = { x: 10, y: 0 }; + const d = curvedPath([p0, p1, p2]); + const cmds = parsePath(d); + // Exactly: M p0 ; Q p1 p2 — no trailing straight "L" half. + expect(cmds.map(c => c.cmd)).toEqual(['M', 'Q']); + const q = cmds[1]; + expect(q.nums).toEqual([5, 4, 10, 0]); // control = waypoint, ends on p2 + }); + + test('curve bulges toward the waypoint (outward), not into the bend', () => { + const p0 = { x: 0, y: 0 }, p1 = { x: 5, y: 4 }, p2 = { x: 10, y: 0 }; + const d = curvedPath([p0, p1, p2]); + const ctrl = quadControls(d)[0]; + const apex = quadAt(p0, ctrl, p2, 0.5); + // Apex and the waypoint must be on the SAME side of the chord p0→p2. + const sApex = sideOf(p0, p2, apex); + const sWaypoint = sideOf(p0, p2, p1); + expect(Math.sign(sApex)).toBe(Math.sign(sWaypoint)); + // And it must actually bow out (not be ~collinear / straight). + expect(Math.abs(apex.y)).toBeGreaterThan(1); + }); + + test('endpoints are preserved for a multi-point path', () => { + const pts = [ + { x: 0, y: 0 }, { x: 4, y: 3 }, { x: 8, y: -2 }, { x: 12, y: 1 }, + ]; + const anchors = pathAnchors(curvedPath(pts)); + expect(anchors[0]).toEqual(pts[0]); // starts at P0 + expect(anchors[anchors.length - 1]).toEqual(pts[3]); // ends at Pn + }); + + test('interior waypoints are used as control points', () => { + const pts = [ + { x: 0, y: 0 }, { x: 4, y: 3 }, { x: 8, y: -2 }, { x: 12, y: 1 }, + ]; + const controls = quadControls(curvedPath(pts)); + expect(controls).toContainEqual({ x: 4, y: 3 }); + expect(controls).toContainEqual({ x: 8, y: -2 }); + }); +}); + +// ─── dribblingPath ────────────────────────────────────────────────────────────── + +test.describe('dribblingPath', () => { + test('starts at the first point and ends near the last', () => { + const pts = [{ x: 0, y: 0 }, { x: 20, y: 0 }]; + const anchors = pathAnchors(dribblingPath(pts, 1)); + expect(anchors[0]).toEqual({ x: 0, y: 0 }); + const end = anchors[anchors.length - 1]; + expect(end.x).toBeCloseTo(20, 5); + expect(end.y).toBeCloseTo(0, 5); + }); + + test('consecutive waves alternate sides of a straight path', () => { + const a = { x: 0, y: 0 }, b = { x: 30, y: 0 }; + const controls = quadControls(dribblingPath([a, b], 1)); + expect(controls.length).toBeGreaterThanOrEqual(2); + // On a horizontal path the wave controls sit above/below the x-axis in turn. + for (let i = 1; i < controls.length; i++) { + expect(Math.sign(controls[i].y)).toBe(-Math.sign(controls[i - 1].y)); + expect(Math.abs(controls[i].y)).toBeGreaterThan(0.1); + } + }); + + test('waves follow a bend: displacement is perpendicular to the local segment', () => { + // L-shaped path: first leg goes +x, second leg goes +y. + const a = { x: 0, y: 0 }, b = { x: 20, y: 0 }, c = { x: 20, y: 20 }; + const controls = quadControls(dribblingPath([a, b, c], 1)); + + // Controls whose midpoint lies on the vertical second leg must be displaced + // horizontally (perpendicular to +y). With the old bug they were displaced + // vertically (using the first segment's perpendicular) — i.e. x≈20. + const onSecondLeg = controls.filter(p => p.y > 1 && p.y < 19); + expect(onSecondLeg.length).toBeGreaterThan(0); + for (const p of onSecondLeg) { + // Perpendicular to the vertical leg ⇒ noticeable x offset from the line x=20. + expect(Math.abs(p.x - 20)).toBeGreaterThan(0.3); + } + }); +}); + +// ─── straightPath (sanity) ────────────────────────────────────────────────────── + +test('straightPath chains line segments through every point', () => { + const d = straightPath([{ x: 0, y: 0 }, { x: 1, y: 2 }, { x: 3, y: 4 }]); + expect(d).toBe('M 0,0 L 1,2 L 3,4'); +}); From a8ce3b06c128ccc029733a15ae4b455d4e866103 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 05:54:20 +0000 Subject: [PATCH 2/2] fix(renderer): draw the FIBA defender as a proper horseshoe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The defender symbol was a filled cubic-Bézier path whose outer and inner curves bowed the same way, so the fill self-intersected and rendered as two small crescents at the bottom of the token instead of a horseshoe (Ω). Replace it with a single thick stroked elliptical arc, open at the bottom, with the number inside — matching the FIBA convention and reading clearly at token size. Verified visually by rasterizing the example drills before/after. Tests: tests/renderer.spec.js asserts the defender is a stroked open arc (fill="none", stroke=defense color, no Z/C) plus invariants for offense, ball, coach, court surface, rim and movement/passing line styling. The defender structure test fails on the pre-fix symbol. Full suite: 46 passed. --- src/court/renderer.js | 29 ++++++------- tests/renderer.spec.js | 96 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 17 deletions(-) create mode 100644 tests/renderer.spec.js diff --git a/src/court/renderer.js b/src/court/renderer.js index 7545871..e211187 100644 --- a/src/court/renderer.js +++ b/src/court/renderer.js @@ -256,25 +256,20 @@ function renderPlayer(pos, transform, scheme) { svg += `${e.nr}`; } else if (e.type === 'defense') { const fill = e.color ? getColor(e.color, scheme) : scheme.defense_fill; - // Defense: FIBA-style symbol — horseshoe (open-top arc) + small inner circle - // Original FIBA path at scale 1 (width=40, height=24): outer arc + inner arc → closed horseshoe - // We scale to fit our radius r: scale = r*2/40 = r/20 - const s = r / 20; - const n = (v) => Math.round(v * 10) / 10; // round to 1 decimal const cx = svgPos.x, cy = svgPos.y; + const n = (v) => Math.round(v * 10) / 10; + // White background for contrast. svg += ``; - // Horseshoe: outer bezier arc + inner bezier arc (closed), centered at (cx, cy) - // Outer arc: M (cx-20s, cy+10s) C (cx-10s, cy-6s) (cx+10s, cy-6s) (cx+20s, cy+10s) - // Inner arc: C (cx+15s, cy-14s) (cx-15s, cy-14s) (cx-20s, cy+10s) Z - const ox1 = n(cx - 20*s), oy1 = n(cy + 10*s); - const ox2 = n(cx + 20*s), oy2 = n(cy + 10*s); - const oc1x = n(cx - 10*s), oc1y = n(cy - 6*s); - const oc2x = n(cx + 10*s), oc2y = n(cy - 6*s); - const ic1x = n(cx + 15*s), ic1y = n(cy - 14*s); - const ic2x = n(cx - 15*s), ic2y = n(cy - 14*s); - svg += ``; - // Inner white circle - svg += ``; + // FIBA defender: a chunky horseshoe (open at the bottom) drawn as a single + // thick stroked arc — no self-intersecting fill — with the number inside. + const Rc = r * 0.72; // centerline radius of the band + const sw = Math.max(2.5, r * 0.42); // band thickness + const gap = 52 * Math.PI / 180; // half-angle of the bottom opening + const aStart = -Math.PI / 2 - gap; // lower-left leg + const aEnd = -Math.PI / 2 + gap; // lower-right leg + const pt = (a) => `${n(cx + Rc * Math.cos(a))},${n(cy - Rc * Math.sin(a))}`; + // Major arc over the top: large-arc-flag=1, sweep-flag=1. + svg += ``; svg += `${e.nr}`; } else if (e.type === 'coach') { svg += ``; diff --git a/tests/renderer.spec.js b/tests/renderer.spec.js new file mode 100644 index 0000000..e5208f3 --- /dev/null +++ b/tests/renderer.spec.js @@ -0,0 +1,96 @@ +/** + * Renderer output tests (pure string generation — no browser). + * + * Guards the entity symbols, especially the FIBA defender, whose filled + * self-intersecting Bézier previously collapsed into two stray crescents + * instead of a horseshoe. The defender is now a single stroked arc. + */ + +import { test, expect } from '@playwright/test'; +import { renderOCF } from '../src/court/renderer.js'; + +function doc(entities, lines = []) { + return { + meta: { id: 't', title: 't' }, + court: { ruleset: 'fiba', type: 'half_court' }, + entities, + frames: [{ id: 'f1', lines }], + }; +} + +/** Pull every from an SVG string. */ +function paths(svg) { + return [...svg.matchAll(/]*\bd="([^"]*)"[^>]*>/g)].map(m => ({ + full: m[0], + d: m[1], + })); +} + +test.describe('defense symbol', () => { + const svg = renderOCF(doc([{ type: 'defense', nr: 5, x: 0, y: 7 }]), 0, 700, 760).svgContent; + + test('is drawn as a single open stroked arc, not a closed fill', () => { + // Identify the defender path by its fill color (#58001d), which court lines + // never use. It must be a stroked open arc — not the old self-closing blob. + const defenderPaths = paths(svg).filter(p => p.full.includes('58001d')); + expect(defenderPaths.length, 'expected one defender path').toBe(1); + const arc = defenderPaths[0]; + expect(arc.d).toContain('A'); // elliptical arc + expect(arc.d).not.toContain('Z'); // open at the bottom + expect(arc.d).not.toContain('C'); // not the old cubic-bezier blob + expect(arc.full).toContain('fill="none"'); // stroked, not filled + expect(arc.full).toContain('stroke="#58001d"'); + expect(arc.full).toMatch(/stroke-width="[\d.]+"/); + }); + + test('shows the defender number', () => { + expect(svg).toMatch(/>5<\/text>/); + }); + + test('does not draw the offense-style solid disc for a defender', () => { + // Offense draws a filled colored circle of radius 12; defense must not. + expect(svg).not.toMatch(/]*r="12"[^>]*fill="#58001d"/); + }); +}); + +test.describe('other entity symbols', () => { + test('offense is a filled numbered circle', () => { + const svg = renderOCF(doc([{ type: 'offense', nr: 7, x: 0, y: 7 }]), 0, 700, 760).svgContent; + expect(svg).toMatch(/]*r="12"[^>]*fill="#003366"/); + expect(svg).toMatch(/>7<\/text>/); + }); + + test('ball renders as a gradient ellipse', () => { + const svg = renderOCF(doc([{ type: 'ball', x: 0, y: 7 }]), 0, 700, 760).svgContent; + expect(svg).toMatch(/]*fill="url\(#ocf-ball-gradient\)"/); + }); + + test('coach renders as an outlined "C"', () => { + const svg = renderOCF(doc([{ type: 'coach', x: 0, y: 7 }]), 0, 700, 760).svgContent; + expect(svg).toMatch(/>C<\/text>/); + }); +}); + +test.describe('court + lines', () => { + test('court surface and rim are present', () => { + const svg = renderOCF(doc([]), 0, 700, 760).svgContent; + expect(svg).toContain('fill="#d4a06a"'); // court surface + expect(svg).toMatch(/stroke="#cc5500"/); // rim + }); + + test('a movement line ends with an arrow marker', () => { + const svg = renderOCF( + doc([], [{ type: 'movement', coords: [{ x: -4, y: 6 }, { x: 4, y: 6 }] }]), + 0, 700, 760, + ).svgContent; + expect(svg).toContain('marker-end="url(#ocf-arrow)"'); + }); + + test('a passing line is dashed', () => { + const svg = renderOCF( + doc([], [{ type: 'passing', coords: [{ x: -4, y: 6 }, { x: 4, y: 6 }] }]), + 0, 700, 760, + ).svgContent; + expect(svg).toMatch(/stroke-dasharray="6,4"/); + }); +});