From d50d55aea9e7377f0cb4253056cf3d1cc09f982d Mon Sep 17 00:00:00 2001 From: Ty Date: Fri, 25 Sep 2026 08:53:38 -0700 Subject: [PATCH] fix(m4): tape line is a fixed fixture, not draggable (#99) --- docs/REQUIREMENTS.md | 6 +- frontend/src/capture/AxisPanel.svelte | 48 ++---- .../src/capture/session.batchApply.test.ts | 2 - .../src/capture/session.calibrate.test.ts | 2 - frontend/src/capture/session.ts | 10 +- frontend/src/capture/tapeLine.test.ts | 26 ++++ frontend/tests/e2e/artifacts/README.md | 6 + frontend/tests/e2e/tape-fixed-line.spec.ts | 138 ++++++++++++++++++ 8 files changed, 189 insertions(+), 49 deletions(-) create mode 100644 frontend/tests/e2e/tape-fixed-line.spec.ts diff --git a/docs/REQUIREMENTS.md b/docs/REQUIREMENTS.md index 50f9732..35c8b4b 100644 --- a/docs/REQUIREMENTS.md +++ b/docs/REQUIREMENTS.md @@ -29,9 +29,9 @@ Sources reconciled 2026-09-18 from the current branch docs plus the `pre-simplif | CAPTURE-60 | The owner aligns the length axis to the prevailing slab length and confirms it; the tape-line calibration is independent of the mask's minimum-area bounding rectangle. | M4 | frontend/src/capture/tapeLine.test.ts | | CAPTURE-65 | On a nearly square slab the owner chooses which axis is length. | shipped | — | | CAPTURE-70 | The owner enters length and thickness in 1/8 inch steps. | shipped | frontend/src/capture/measureInputs.test.ts | -| CAPTURE-75 | Scale equals the pixel distance between the two owner-positioned tape-line ticks divided by the owner-declared length; mask span (`maskSpanAlong`) is not checked or used as a length or scale source. | M4 | frontend/src/measure/measure.test.ts | -| CAPTURE-170 | The calibration overlay shows a draggable tape-line with two rifle-scope end ticks; the owner positions the ticks at the tape's actual start and end. | M4 | frontend/src/capture/tapeLine.test.ts | -| CAPTURE-175 | The owner-declared length is the distance represented by the tape-line ticks, and scale uses their pixel distance in source-photo coordinates, independent of mask span. | M4 | frontend/src/measure/measure.test.ts | +| CAPTURE-75 | Scale equals the pixel distance between the two ends of the fixed tape line divided by the owner-declared length; mask span (`maskSpanAlong`) is not checked or used as a length or scale source. | M4 | frontend/src/measure/measure.test.ts | +| CAPTURE-170 | The calibration overlay draws one black tape-line fixed in the middle of the 3:4 frame, with a tick at each end. The line is a stage fixture: it is not editable, and its ends are set by the crop-fill setting alone (at 90 percent each end sits 5 percent in from the frame edge). The owner pans, zooms, and rotates the slab until its long axis lies along the line. | M4 | frontend/src/capture/tapeLine.test.ts | +| CAPTURE-175 | The owner-declared length is the tape reading taken where the slab was aligned to the line; scale equals the fixed line's pixel distance between its two ends, in source-photo coordinates, divided by that declared length. Mask span (`maskSpanAlong`) is not checked or used as a length or scale source. | M4 | frontend/src/measure/measure.test.ts | | CAPTURE-80 | Widths sample every 6 inches perpendicular to the approved axis, dropping the first and last 6 inches; min, max, and average are reported; if length is 12 inches or less all three are zero. | shipped | frontend/src/measure/measure.test.ts | | CAPTURE-85 | sqft equals mask_pixels divided by scale squared divided by 144, rounded half-up to 2 decimals. | shipped | frontend/src/measure/measure.test.ts | | CAPTURE-90 | bdft equals sqft times thickness_in, rounded half-up to 2 decimals, with no divide-by-12; 1 sqft at 1 inch equals 1 bdft. | shipped | frontend/src/measure/measure.test.ts | diff --git a/frontend/src/capture/AxisPanel.svelte b/frontend/src/capture/AxisPanel.svelte index e9ed030..7242634 100644 --- a/frontend/src/capture/AxisPanel.svelte +++ b/frontend/src/capture/AxisPanel.svelte @@ -8,15 +8,12 @@ IDENTITY_TRANSFORM, cropEnds, fitScale, - hitRadiusStage, } from './tapeLine'; export let state: Extract; export let cropFill = 90; export let setTapePose: (pose: { transform: PhotoTransform; - tickStartY: number; - tickEndY: number; sourceW: number; sourceH: number; }) => void; @@ -33,14 +30,13 @@ let sourceH = state.source.heightPx; let renderedW = 320; let transform: PhotoTransform = { ...(state.axis.photoTransform ?? IDENTITY_TRANSFORM) }; - let tickA = state.axis.tickStartY; - let tickB = state.axis.tickEndY; - let focus: 'A' | 'B' = 'A'; + // The line is a stage fixture. Its two ends are the crop insets, so they are + // a pure function of crop_fill and are never an owner input (#99). let fill = cropFill; let lengthIn = state.lengthEighths === 0 ? '' : String(eighthsToIn(state.lengthEighths)); let thicknessIn = state.thicknessEighths === 0 ? '' : String(eighthsToIn(state.thicknessEighths)); let calcBusy = false; - let drag: 'tick' | 'tilt' | 'pan' | 'pinch' | null = null; + let drag: 'tilt' | 'pan' | 'pinch' | null = null; let pointers = new Map(); let pinch0 = 0; let zoom0 = 1; @@ -56,8 +52,6 @@ function publish() { setTapePose({ transform: { ...transform }, - tickStartY: tickA, - tickEndY: tickB, sourceW, sourceH, }); @@ -105,12 +99,12 @@ ctx.moveTo(x, ends.startY); ctx.lineTo(x, ends.endY); ctx.stroke(); - for (const [id, y] of [['A', tickA], ['B', tickB]] as const) { + for (const y of [ends.startY, ends.endY]) { ctx.beginPath(); ctx.moveTo(x - 13, y); ctx.lineTo(x + 13, y); ctx.stroke(); - ctx.fillStyle = focus === id ? '#e23' : '#000'; + ctx.fillStyle = '#000'; ctx.beginPath(); ctx.arc(x, y, 5, 0, Math.PI * 2); ctx.fill(); @@ -121,10 +115,8 @@ ctx.fill(); } - function reseat(pct: number) { - const ends = cropEnds(pct); - tickA = ends.startY; - tickB = ends.endY; + function reseat() { + // The line's ends follow crop_fill; nothing about them is owner-draggable. publish(); draw(); } @@ -167,14 +159,7 @@ return; } const x = frame.width / 2; - const hit = hitRadiusStage(renderedW); - if (Math.hypot(p[0] - x, p[1] - tickA) <= hit) { - focus = 'A'; - drag = 'tick'; - } else if (Math.hypot(p[0] - x, p[1] - tickB) <= hit) { - focus = 'B'; - drag = 'tick'; - } else if (Math.hypot(p[0] - x, p[1] - (frame.height - 28)) <= 22) { + if (Math.hypot(p[0] - x, p[1] - (frame.height - 28)) <= 22) { drag = 'tilt'; tilt0 = transform.tiltDeg; tiltX0 = p[0]; @@ -199,11 +184,7 @@ zoomAt(mid, clamp(dist / pinch0, 0.8, 1.25)); return; } - if (drag === 'tick') { - const y = clamp(p[1], 8, frame.height - 8); - if (focus === 'A') tickA = clamp(y, 8, tickB - 24); - else tickB = clamp(y, tickA + 24, frame.height - 8); - } else if (drag === 'tilt') { + if (drag === 'tilt') { transform = { ...transform, tiltDeg: tilt0 + (p[0] - tiltX0) * 0.6 }; } else if (drag === 'pan') { transform = { @@ -254,20 +235,13 @@ publish(); draw(); event.preventDefault(); - } else if (event.key === 'ArrowUp' || event.key === 'ArrowDown') { - const step = (event.shiftKey ? 0.1 : 1) * (event.key === 'ArrowUp' ? -1 : 1); - if (focus === 'A') tickA = clamp(tickA + step, 8, tickB - 24); - else tickB = clamp(tickB + step, tickA + 24, frame.height - 8); - publish(); - draw(); - event.preventDefault(); } } function onFill(event: Event) { fill = Number((event.currentTarget as HTMLInputElement).value); setCropFill(fill); - reseat(fill); + reseat(); } const positiveInches = (raw: string): number | null => { @@ -325,7 +299,7 @@

Length

-

Move the slab until its long axis sits on the line. Drag the ticks to the tape ends. The line does not move.

+

Move the slab until its long axis sits on the line, the way it lines up with the tape. The line is fixed in the middle; its ends are set by the crop slider and do not move.

| null = null; + // The tape line is a stage fixture: the owner only moves the slab. Its ends are + // the crop insets, so a pose carries the photo transform and nothing else (#99). const axisFrom = ( bbox: Axis['bbox'], sourceW: number, sourceH: number, - pose?: { transform?: PhotoTransform; tickStartY?: number; tickEndY?: number; tapeStart?: [number, number]; tapeEnd?: [number, number] }, + pose?: { transform?: PhotoTransform; tapeStart?: [number, number]; tapeEnd?: [number, number] }, ): Axis => { const ends = cropEnds(cropFill); const transform = pose?.transform ?? { ...IDENTITY_TRANSFORM }; - const tickStartY = pose?.tickStartY ?? ends.startY; - const tickEndY = pose?.tickEndY ?? ends.endY; + const tickStartY = ends.startY; + const tickEndY = ends.endY; const w = sourceW > 0 ? sourceW : 1; const h = sourceH > 0 ? sourceH : 1; const seg = pose?.tapeStart && pose?.tapeEnd diff --git a/frontend/src/capture/tapeLine.test.ts b/frontend/src/capture/tapeLine.test.ts index 37c9c20..fba6e41 100644 --- a/frontend/src/capture/tapeLine.test.ts +++ b/frontend/src/capture/tapeLine.test.ts @@ -52,6 +52,32 @@ test('scale is source-pixel tick distance divided by the declared length', () => assert.equal(calibrationScale(32, 608, 480, 640, IDENTITY_TRANSFORM, 0), 0); }); +test('tick ends are fixed by the crop slider and are never an owner input (#99)', () => { + // The line is a stage fixture: its two ends are the crop insets, set by + // crop_fill alone. There is no owner drag that can move them. + for (const fill of [1, 25, 50, 75, 90, 100]) { + const ends = cropEnds(fill); + assert.equal(ends.startY, cropEnds(fill).startY, 'ends are a pure function of fill'); + assert.equal(ends.endY, cropEnds(fill).endY, 'ends are a pure function of fill'); + assert.ok(ends.endY > ends.startY, 'line keeps a positive span at any fill'); + } + // Ends stay symmetric about the frame centre, so the line is fixed in the middle. + for (const fill of [1, 50, 90, 100]) { + const ends = cropEnds(fill); + const top = ends.startY; + const bottom = CALIBRATION_FRAME.height - ends.endY; + assert.equal(top, bottom, `fill ${fill}: top and bottom insets match`); + } +}); + +test('the scale span is the fixed line span, not a mask span', () => { + const ends = cropEnds(90); + const distance = sourceTickDistance(ends.startY, ends.endY, 480, 640, IDENTITY_TRANSFORM); + const scale = calibrationScale(ends.startY, ends.endY, 480, 640, IDENTITY_TRANSFORM, 72); + assert.ok(distance > 0, 'line has a real source-pixel span'); + assert.equal(scale, distance / 72, 'scale is the line span over the tape reading'); +}); + test('the tick hit target is at least 44px on a 320px-wide stage', () => { const radius = hitRadiusStage(320); const screen = radius * (320 / CALIBRATION_FRAME.width); diff --git a/frontend/tests/e2e/artifacts/README.md b/frontend/tests/e2e/artifacts/README.md index 2a1780d..d195953 100644 --- a/frontend/tests/e2e/artifacts/README.md +++ b/frontend/tests/e2e/artifacts/README.md @@ -31,6 +31,12 @@ Fixtures: `tests/fixtures/set1-48x2/` (L=48, thickness=2) and `set2-31x2p5/` (L= | `04-axis.png` | Tape calibration page | | `05-tape-calibration.png` | Length and thickness entered on the tape page | | `06-photos-ready.png` | Photos ready lineup (clean + overlay + siblings) | +| `tape-fixed-line.png` | #99 fixed tape line: no tick-drag copy, ticks follow the crop slider | +| `tape-fixed-line-crop50.png` | #99 fixed tape line at 50% crop fill (line ends pulled in) | + +`tape-fixed-line.spec.ts` asserts the #99 contract: the line is a stage fixture, +its ends are a pure function of `crop_fill`, a drag starting on a tick behaves +exactly like a pan elsewhere, and arrow keys cannot move the ends. Large regenerating `*.png` and Playwright `test-results/` / `playwright-report/` are gitignored. Committed small evidence JPEGs live in `evidence/` (regenerate with a green `npm run test:e2e`, then re-encode if needed). diff --git a/frontend/tests/e2e/tape-fixed-line.spec.ts b/frontend/tests/e2e/tape-fixed-line.spec.ts new file mode 100644 index 0000000..5cbe0e7 --- /dev/null +++ b/frontend/tests/e2e/tape-fixed-line.spec.ts @@ -0,0 +1,138 @@ +import { test, expect, type Page } from '@playwright/test'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import fs from 'node:fs'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const FACE_A = path.resolve(here, '../fixtures/set1-48x2/face-a.jpeg'); +const ARTIFACTS = path.resolve(here, 'artifacts'); + +const shot = async (page: Page, name: string) => { + fs.mkdirSync(ARTIFACTS, { recursive: true }); + await page.screenshot({ path: path.join(ARTIFACTS, name), fullPage: true }); +}; + +const ensureFreshBatch = async (page: Page) => { + await page.goto('/'); + const resumeHeading = page.getByRole('heading', { name: /resume/i }); + const batchHeading = page.getByRole('heading', { name: 'Batch photos' }); + await expect(resumeHeading.or(batchHeading)).toBeVisible({ timeout: 15_000 }); + if (await resumeHeading.isVisible().catch(() => false)) { + await page.getByRole('button', { name: /start fresh/i }).click(); + } + await expect(batchHeading).toBeVisible(); +}; + +const prepareMaskForApprove = async (page: Page) => { + await expect(page.getByRole('heading', { name: 'Mask' })).toBeVisible(); + await page.getByRole('button', { name: 'Black', exact: true }).click(); + const approve = page.getByRole('button', { name: 'Approve mask' }); + try { + await expect(approve).toBeEnabled({ timeout: 45_000 }); + return; + } catch { + /* sensitivity ladder */ + } + for (const target of [60, 70, 40, 30, 80]) { + await page.locator('input[type="range"]').fill(String(target)); + await page.waitForTimeout(2800); + if (await approve.isEnabled()) return; + } + await expect(approve).toBeEnabled({ timeout: 15_000 }); +}; + +const toLengthPage = async (page: Page) => { + await ensureFreshBatch(page); + await page.locator('input[type="file"][multiple]').setInputFiles([FACE_A]); + await expect(page.getByAltText('Batch photo 1')).toBeVisible({ timeout: 30_000 }); + await page.getByRole('button', { name: 'Continue' }).click(); + await page.getByRole('button', { name: 'Use photo 1 as calibration' }).click(); + await prepareMaskForApprove(page); + await page.getByRole('button', { name: 'Approve mask' }).click(); + await expect(page.getByRole('heading', { name: 'Length' })).toBeVisible(); +}; + +test.describe('#99 fixed tape line', () => { + test('the line is not editable: no tick-drag copy and no tick drag', async ({ page }) => { + test.setTimeout(180_000); + expect(fs.existsSync(FACE_A), 'fixture face-a').toBeTruthy(); + await toLengthPage(page); + + // The hint must never tell the owner to drag the ticks. + await expect(page.getByText('Drag the ticks')).toHaveCount(0); + await expect(page.getByText('The line is fixed in the middle')).toBeVisible(); + + const stage = page.getByTestId('tape-line'); + await expect(stage).toBeVisible(); + const box = (await stage.boundingBox())!; + const canvas = stage.locator('canvas').first(); + + // A pan delta produces the same result wherever you grab the stage, because + // pan is measured as a delta from the grab point. So: drag from a tick + // position, then repeat the same delta from a plain part of the stage. If + // the tick drag were still live it would move the line too, and the two + // results would differ. Identical output ⇒ the line is fixed. + const centerX = box.x + box.width / 2; + const tickY = box.y + box.height * 0.05; + const plainY = box.y + box.height * 0.62; + const delta = box.height * 0.18; + + const dragBy = async (from: number) => { + await page.mouse.move(centerX, from); + await page.mouse.down(); + await page.mouse.move(centerX, from + delta, { steps: 10 }); + await page.mouse.up(); + await page.waitForTimeout(300); + }; + + await dragBy(tickY); + const fromTick = await canvas.screenshot(); + + // Reset the transform, then repeat the identical delta from a non-tick point. + await toLengthPage(page); + await dragBy(plainY); + const fromPlain = await canvas.screenshot(); + + expect( + Buffer.compare(fromTick, fromPlain), + 'a tick drag must behave exactly like a pan elsewhere (line stays fixed)', + ).toBe(0); + + // Length and thickness live on this same screen; Calculate stays usable. + await page.getByLabel('Length L').fill('48'); + await page.getByLabel('Thickness').fill('2'); + await expect(page.getByRole('button', { name: 'Calculate' })).toBeEnabled(); + await shot(page, 'tape-fixed-line.png'); + }); + + test('the crop slider sets the line ends and nothing else does', async ({ page }) => { + test.setTimeout(180_000); + await toLengthPage(page); + + const canvas = page.getByTestId('tape-line').locator('canvas').first(); + const before = await canvas.screenshot(); + + // Move the crop-fill slider to a different value. + const fill = page.getByLabel('Crop fill percent'); + await fill.fill('50'); + await page.waitForTimeout(500); + + const after = await canvas.screenshot(); + expect(Buffer.compare(before, after), 'crop fill must move the line ends').not.toBe(0); + + // Arrow-key nudge on the line must not change the geometry either. + // Blur the slider first: arrow keys on a focused range input legitimately + // change that input, which would move the line ends for the wrong reason. + await page.locator('h1').click(); + await page.keyboard.press('ArrowUp'); + await page.keyboard.press('ArrowUp'); + await page.keyboard.press('ArrowDown'); + await page.waitForTimeout(500); + const afterKeys = await canvas.screenshot(); + expect( + Buffer.compare(after, afterKeys), + 'arrow keys must not move the fixed line ends', + ).toBe(0); + await shot(page, 'tape-fixed-line-crop50.png'); + }); +});