feat(annotator): the bbox tool — three numbers, three questions, and a cursor that cannot lie (#43) - #119
Merged
Merged
Conversation
…a cursor that cannot lie (#43) #42 already drew, moved and resized boxes. What it left, by name in four docstrings, is what this is: the creation threshold (`bbox.ts`: "the drawing tool (#43) owns that one and should choose it knowingly rather than inherit it"), attribute defaults (`draft.ts`), the affordance layer `resolveTarget` was shaped for (`target.ts`), and the acceptance matrix stated as tests. v1 had two spellings of `3` — `width > 3` strictly when a drawn box was accepted, `< 3` when a resized one was pushed back out. The fault was not that there were two numbers; it was that both answered the same question and disagreed about its boundary. There are now three, each named and each in its own frame: CLICK_SLOP_PX (did a press travel?), MIN_DRAW_SIZE_PX (did a human mean to draw?) and MIN_BBOX_SIZE (may a stored box be this degenerate?). The first two are facts about hands and are measured on the screen; the third is a fact about the document and stays in asset pixels. A consequence stated rather than hidden: at 400% the gate admits a box below MIN_BBOX_SIZE, because somebody who zoomed in to draw two pixels demonstrably meant to. `affordanceAt` mirrors IDLE_ROW's precedence, and that is the whole of its correctness. That row checks the tool before it resolves any target, so in a drawing tool a grip is unreachable and the cursor is a crosshair on top of one; it groups `edge` with `body`, so an edge offers `move`. Both are asserted in the same `it` as the press they claim to predict, so the two cannot drift. A drag answers from what it is holding rather than from the point, and the scene an adapter passes is built from `rendered` where the machine's is `document` — hover hit-tests the pixels on screen, a transition never resolves against a preview.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #43. M4 position 7 of 14 — "the first tool, and the one that exercises the machine's drag and resize variants."
Ledger
No migration.
FORMAT_VERSIONstays 11,VERSIONstays0.0.1.dev0,openapi.jsonandfrontend/ui-core/src/generated/api.tsare byte-identical, and not one file undersrc/ortests/is touched (git diff origin/main --statover all six is empty). No new dependency of any kind. M4's finding H says a change to any of those is the signal the annotator reached for the backend early.454 annotator tests, up from 384 — three new files and two extended. Plus the 15
node:testboundary/script tests, unchanged. No new interaction state and no new event: the seven and the eight stand.The honest starting position
#42 already drew, moved and resized boxes. What it left is what this is — and it left it by name, in four committed docstrings:
geometry/bbox.tsinteraction/machine.tsinteraction/draft.tsLabelClass.attributesis #43's."interaction/target.tsAll four close here.
gestures.test.tsis not duplicated: its there-and-back / walked-versus-jumped / dropped-move trio are properties of an absolute transform and stay where they are.bboxTool.test.tsis the matrix.Three numbers, three questions
v1 had two spellings of
3—width > 3strictly when a drawn box was accepted,< 3when a resized one was pushed back out. The fault was not that there were two numbers. It was that both answered the same question and disagreed about its boundary.CLICK_SLOP_PXMIN_DRAW_SIZE_PXMIN_BBOX_SIZEScreen pixels, for
tolerance.ts's own thesis. v1 compared its threshold against a coordinate it had already divided by the zoom, so a box drawn at 30% needed about one screen pixel of drag and one at 200% needed six — the same inversion #41 exists to have fixed, on the one gate that decides whether an annotation exists at all. It isTolerances' seventh field, so no signature moved;tolerance.test.ts's totalRecord<keyof Tolerances, number>refused to compile until it was converted, which is that gate working.Reusing
CLICK_SLOP_PXwas considered and declined. The two are 3 today and their questions rhyme, but they are independently tunable: raising the size a box must reach before it enters a dataset should not make a selection click harder to land.isDrawnBox(bbox, minimum, bounds)lives inbbox.tsbesideMIN_BBOX_SIZE, takes its number as an argument, and:>=), pinned from both sides, so the boundary is written down once.resizeBboxalready resolves that way. A 2-px asset must not be a surface on which nothing can be drawn.Math.abs, likebboxContainsreads edges through min/max. It is exported from the package root, so it can be asked about a negative-width box off the wire; its only in-repo caller hands itnormalizeBboxoutput and would never have noticed.A consequence, stated because it reads like an inconsistency and is not: above 100% the gate admits a box smaller than
MIN_BBOX_SIZE, so that constant is an invariant ofresizeBboxand not of the document. That is the right way round — somebody who zoomed to 400% and drew two pixels demonstrably meant to; the same two pixels out of a resize at 100% is a grip that got away from them.A rejected drag returns
idle()with no effects: nothing added, nothing selected, and the selection the press cleared stays cleared. The press did land on empty canvas.The affordance layer — and the bug the design caught
resolveTargetanswers what is under this point. A renderer needs two more things — the cursor, and which grip to paint hot — and neither is a fact about the document.interaction/affordance.tsis that layer, one pure function, so the cursor and the press cannot disagree.That thesis earned its keep during review. The first draft answered
defaultover anedge, reasoning that a 15-px band only a double-click uses should not advertise itself.IDLE_ROWsays otherwise:A press in that band picks the polygon and enters
moving.defaultthere was the drawing-tool lie inverted — under-promising rather than over — and still a disagreement with the table. It ismovenow, and the test asserts the cursor and the resultingtransitionin the sameit, so the two cannot drift.The rules:
IDLE_ROWcheckstool !== "select"before it resolves any target, so a grip on a selected box is unreachable however exactly the pointer sits on one. Draw mode answerscrosshair+NO_TARGETunconditionally — pinned with the pointer parked on the picked box's ownnwgrip, in the same test that asserts the press starts a drawing.pressing-emptyshows nothing. The button is already down on empty canvas and that row has nopointer-down, so for the whole gesture nothing is reachable. Answering whatidlewould answer lights a grip that cannot be taken.store.rendered; the machine's isstore.document. Hover has to hit-test the pixels actually on screen, and mid-drag those are the preview's; a transition must never resolve against a preview. Small, load-bearing, and annotator: React adapter — SVG render, zoom/pan, asset image, annotation layer #47 is the caller that has to get it right.A CSS cursor keyword is vocabulary, not a DOM API.
Cursoris seven strings; nothing touches a node and it compiles undertsconfig.core.jsonwithlib: ["ES2022"]andtypes: []like everything else incore/. The alternative — an abstract union an adapter maps one-for-one onto these very names — is a second spelling of one fact.LabelClass.coloris already a plain string for the same reason. A test asserts every member of the union is actually produced by a real call, because vocabulary nobody speaks reads as capability.Attribute defaults, and the one deliberately not filled
draftAnnotationseeds every attribute the class declares adefaultfor, read through the document's ownclassNamed. An attribute with anulldefault contributes no key rather than a null value —AttributeValuehas no null on the wire.A required attribute with no default is drawn anyway.
document.tshands "the tools refuse at draw time, where a user can be told" to the tools — but there is nobody to tell yet. Refusing would make such a class simply undrawable, with no panel in existence to satisfy it and no channel to explain why the pointer did nothing. The kernel'sMissingRequiredAttributeis the backstop on write; M5's attributes panel is what turns that sentence into something a user can act on.Active class stays the host's — recorded, not changed
Nothing in
core/stores it;InteractionContext.labelClassarrives per turn andfinishDrawingstamps it, which is the whole of "class assignment from the active class". Moving it intoAnnotatorStorewas declined: the store is the document and its history, and a palette click is neither — putting it in the history would make Ctrl+Z step through palette clicks. What the host owes in exchange is one line, now intool.tsso #47 inherits it: when the class changes such thattoolForreturns a different tool, sendtool-changed— not on every class change, sincedrawing-bboxcaptured its class at the press and a switch between two bbox classes retargets nothing.The audit, tightened rather than weakened
tolerance.tsdocumentsgrep -rn zoom src/core | grep -v ' \* 'as the proof that one module names a viewport. #43's new tests setassetTolerances(4)andassetTolerances(0.25)deliberately — nothing else in the suite varies the frame, so swappingtolerances.minDrawback for the raw constant would have gone unnoticed. A test naming a zoom is the discipline being exercised, not broken. The audit is now scoped to the shipped engine, which is what it always meant and whattsconfig.build.jsonalready defines:It answers with
tolerance.tsalone — a stronger statement than the one it replaces.Acceptance criteria
"Scripted event-sequence tests: draw/move/resize produce exact expected geometry" —
bboxTool.test.ts, 32 tests. Four drag directions to one expectation; a draw clamped at the frame; the threshold from both sides plus the sliver, the click, and both zoom extremes; move and its frame pin; oneitper grip, all eight, against a totalRecord<BboxHandle, {to, out}>whose expectations are written out, not recomputed fromresizeBbox— whichbbox.test.tspins independently. Plus one assertion at the effect level in the formeffects.tsspecified for this task, with no store in the test."Undo after each operation restores the prior document" — asserted with
toBe, by reference, because undo is a pointer swap over kept snapshots and the object that comes back must be the very one. Draw, move and resize each undone; then a three-operation session unwound one entry at a time back to the document it began with; then a gesture that changed nothing, recorded as nothing.Next: #44 — the polygon tool.