Spaces:
Running
Running
| # Annotated-Photo Fallback: Image Sizing and Annotation Accuracy | |
| Review date: 2026-06-14. Scope: one user report — when the **annotated-photo | |
| fallback** triggers (instead of the 3D cutaway), the uploaded image is "too | |
| large to display," and the annotation placement should be confirmed accurate. | |
| This document is findings + next steps for Codex to implement. It does **not** | |
| authorize any deployment or Hugging Face changes on its own; follow the normal | |
| GitHub → HF sync and verification flow in `AGENTS.md`. All file/line references | |
| are to `index.html` unless noted. | |
| --- | |
| ## TL;DR | |
| 1. **The image-sizing bug is real and confirmed in a browser.** The | |
| annotated-photo image renders at its **intrinsic aspect height at full panel | |
| width**, ignoring the viewport height, so tall photos overflow the viewport | |
| by ~2x. `object-fit: contain` never engages because the box it should fit | |
| inside is itself oversized. | |
| 2. **Root cause:** nested `display: grid; place-items: center` containers | |
| (`.annotated-stage` → `.annotation-frame`) with auto-sized rows. A child's | |
| `height: 100%` resolves against an *auto* (content-sized) grid row, not the | |
| container's pixel height, so it collapses back to the image's natural size. | |
| 3. **The annotation accuracy is mostly broken *as a side effect* of the sizing | |
| bug** — labels are positioned against the oversized, overflowing image box, | |
| so any callout in the lower half of a tall photo lands below the visible | |
| viewport and is clipped. Fixing the sizing fixes most of the misplacement. | |
| A few annotation-accuracy improvements remain that are independent of sizing | |
| (no marker dot at the point, edge-clamp detaches labels, unused `box`). | |
| --- | |
| ## How this was verified (not just code inspection) | |
| I reproduced the exact `buildAnnotatedPhoto` DOM (`index.html:996-1076`) and the | |
| exact CSS rules (`.fallback-stage`/`.annotated-stage`/`.annotation-frame`/ | |
| `.annotation-image`, `index.html:466-511`) in a standalone page, loaded a | |
| **3000 × 4000 portrait** test image into a **900 × 600** viewport pane, and | |
| measured the rendered geometry in a real browser (Chromium via Playwright): | |
| | Scenario | Image rendered (W×H) | Fits 600px-tall pane? | | |
| | --- | --- | --- | | |
| | **Current code (baseline)** | **864 × 1151** | ❌ overflows (image bottom at 1209px) | | |
| | Fix: pin image absolutely to padded box | 864 × 498 | ✅ contained (top 58 → bottom 556) | | |
| | Fix: definite `minmax(0,1fr)` tracks at **both** grid levels | 864 × 498 | ✅ contained | | |
| `864 = 900 − 18px×2` (panel width minus horizontal padding); `1151 ≈ 864 × | |
| (4000/3000)` — i.e. the image is laid out at panel width and natural aspect, | |
| with **zero** vertical constraint. That is the "too large to display" symptom. | |
| (The repro page was temporary and has been removed; re-create it if you want to | |
| re-measure.) | |
| --- | |
| ## Root cause (precise) | |
| The fallback markup is three nested boxes: | |
| ``` | |
| #viewport (absolute, inset:0) <- definite size, good | |
| └─ .fallback-stage.annotated-stage index.html:1011-1012 | |
| position:absolute; inset:0 (definite height ✔) | |
| display:grid; place-items:center; padding:58/18/44 index.html:490-495 | |
| └─ .annotation-frame index.html:1013-1014 | |
| width:100%; height:100% index.html:497-503 | |
| display:grid; place-items:center <-- second nested grid | |
| ├─ img.annotation-image index.html:1015-1018 | |
| │ width:100%; height:100%; object-fit:contain index.html:505-511 | |
| └─ .label-layer (absolute, inset:0) | |
| ``` | |
| Two compounding CSS facts: | |
| - **`place-items: center` sets `align-items: center`, not `stretch`.** Combined | |
| with the default `grid-auto-rows: auto`, the single grid row is **content | |
| sized**, not stretched to the container's definite height. | |
| - **A percentage `height` resolves against the grid *area* (the track), and an | |
| `auto` track is *indefinite*.** So `height: 100%` on a child of a | |
| `place-items: center` grid computes as `auto`. | |
| This happens **twice**: | |
| 1. `.annotation-frame { height: 100% }` inside `.annotated-stage` → | |
| the frame's track is auto → frame height becomes auto. | |
| 2. `img.annotation-image { height: 100% }` inside `.annotation-frame` → | |
| the image's track is auto → image height becomes auto → the `<img>` uses its | |
| intrinsic aspect ratio at the available width (864px) → 1151px tall. | |
| Measured proof that the bug is the **inner** grid too: even after forcing | |
| `.annotation-frame` to a correct definite 498px box, the image still rendered | |
| 1151px, because its `height:100%` resolves against `.annotation-frame`'s *inner | |
| auto row*, not the frame's element box. Both grid levels must be fixed (or the | |
| nested-grid sizing dropped entirely). | |
| > Note: `.source-image` in the side panel (`index.html:384-390`) does **not** | |
| > have this bug because it has a fixed `height: 180px` — a definite height, so | |
| > `object-fit: contain` works there. The viewport fallback wants to fill | |
| > available space rather than a fixed height, which is why it hit the trap. | |
| --- | |
| ## Fix options (both verified to contain the image at 864 × 498) | |
| Pick one. **Option A is recommended** — fewest moving parts, no dependence on | |
| fragile nested-grid percentage resolution. | |
| ### Option A (recommended): pin the image to the padded content box | |
| Stop relying on `height: 100%` chaining through two grids. Make the image fill a | |
| single absolutely-positioned box and let `object-fit: contain` do the framing. | |
| ```css | |
| /* .annotated-stage can keep its background; drop the grid centering */ | |
| .annotation-frame { | |
| position: absolute; | |
| inset: 58px 18px 44px; /* same as the old .annotated-stage padding */ | |
| /* remove width/height:100% + display:grid + place-items:center */ | |
| } | |
| .annotation-image { | |
| position: absolute; | |
| inset: 0; | |
| width: 100%; | |
| height: 100%; | |
| object-fit: contain; /* now fits a definite box -> letterboxed, contained */ | |
| } | |
| ``` | |
| `.label-layer` already is `position:absolute; inset:0` (`index.html:466-471`), | |
| so it keeps lining up with the frame. The `58/18/44` padding currently lives on | |
| `.annotated-stage` (`index.html:493`); move that inset onto `.annotation-frame` | |
| (or keep the padding on the stage and set `.annotation-frame { position:absolute; | |
| inset:0 }`). Either way the frame becomes a **definite** box and the image is | |
| contained. | |
| ### Option B: keep the grids but make every track definite | |
| If you prefer to keep `display:grid` centering, the breakage is the auto rows — | |
| give **both** grids definite tracks and let the items stretch: | |
| ```css | |
| .annotated-stage { | |
| display: grid; | |
| grid-template-columns: minmax(0, 1fr); | |
| grid-template-rows: minmax(0, 1fr); | |
| place-items: stretch; /* not center: stretch the frame to fill */ | |
| padding: 58px 18px 44px; | |
| } | |
| .annotation-frame { | |
| display: grid; | |
| grid-template-columns: minmax(0, 1fr); | |
| grid-template-rows: minmax(0, 1fr); | |
| min-width: 0; | |
| min-height: 0; | |
| width: auto; /* track sizing handles it; 100% no longer needed */ | |
| height: auto; | |
| } | |
| .annotation-image { width: 100%; height: 100%; object-fit: contain; } | |
| ``` | |
| This also measured 864 × 498, contained. It is more brittle (relies on readers | |
| understanding the `minmax(0,1fr)` + `min-height:0` idiom), hence Option A is | |
| preferred. | |
| > Whichever option: this is **CSS-only**. No change to `buildAnnotatedPhoto`'s | |
| > JS, the schema, the backend, or `app.py` is required for the sizing fix. | |
| --- | |
| ## Annotation accuracy | |
| ### A) Most misplacement is a *symptom* of the sizing bug — fixing sizing fixes it | |
| `updatePhotoLabels` (`index.html:1031-1062`) positions each callout from: | |
| - `containedImageRect(image)` (`index.html:1096-1118`) — computes the | |
| letterboxed sub-rect of the image from `image.naturalWidth/Height` and the | |
| `<img>` bounding box, then | |
| - `label.left = imageRect.left − layerRect.left + point[0] * imageRect.width` | |
| (and the `point[1]` equivalent for top), clamped to the layer. | |
| This math is **correct** *only when the `<img>` box matches the visible image | |
| area*. With the current sizing bug the `<img>` box is 864 × 1151 and overflows | |
| the viewport, so: | |
| - `imageRect.width/height` come from the oversized box, so a `point[1] = 0.8` | |
| callout is placed at ~0.8 × 1151 ≈ 920px down — **far below** the 600px | |
| viewport, which is `overflow: hidden` (`.viewport-pane`, `index.html:105`), so | |
| the label is clipped and the user never sees it. | |
| - The clamp uses `layerRect.height` (also 1151), so the "keep on screen" clamp | |
| (`index.html:1044-1047`) clamps to the wrong, oversized bounds. | |
| **Once the image is contained (Option A/B), `containedImageRect` returns the | |
| real letterboxed rect and the existing placement math is accurate.** So the | |
| sizing fix is also the primary annotation-accuracy fix. Re-verify after fixing. | |
| ### B) Independent annotation-accuracy improvements (do after the sizing fix) | |
| **User decision (2026-06-14): in scope — implement sizing fix *and* this | |
| annotation-accuracy polish.** Do items #1–#3 below; #4 (1px border) and #5 | |
| (coercion clamp) are optional robustness items, implement if cheap. | |
| 1. **No marker at the actual point.** The callout box is centered *on* the point | |
| via `.scene-label { transform: translate(-50%, -50%) }` (`index.html:476`), | |
| so the text occludes the component instead of pointing at it, and an | |
| edge-clamped label has nothing tying it back to the real location. Recommend | |
| drawing a small dot/crosshair at the exact `point` (a 1–2px element, not | |
| transformed) and offsetting the text label, optionally with a short leader | |
| line. Numbered prefixes already exist (`(index + 1) + ". "`, | |
| `index.html:1051`) but there's no matching number on the photo, so the number | |
| is currently meaningless spatially. | |
| 2. **Edge clamp silently detaches the label from its point.** `Math.min(width − | |
| 72, Math.max(72, …))` (`index.html:1040-1047`) keeps the label on screen but, | |
| with no leader/marker (see #1), a clamped label no longer indicates which | |
| component it describes. The marker dot from #1 is the fix — keep the *dot* at | |
| the true (clamped-to-image) point and only clamp the *text*. | |
| 3. **`annotation.box` is parsed and validated but never drawn.** The schema and | |
| coercion carry an optional normalized `box: [x, y, w, h]` | |
| (`snap2sim/schema.py:197-202`, `snap2sim/model_io.py:325-327`), but | |
| `buildAnnotatedPhoto` ignores it. When present, drawing the bounding box over | |
| the contained image would be a much stronger, more accurate annotation than a | |
| single point. Optional enhancement. | |
| 4. **1px border offset.** `.annotation-image` has a `1px` border | |
| (`index.html:509`) and `box-sizing: border-box` is global (`index.html:38-40`), | |
| so `object-fit: contain` fits the *content* box (inside the border) while | |
| `containedImageRect` measures the *border* box — a 1px placement error. | |
| Negligible; mention only. If you want it exact, account for the border or use | |
| `outline` instead of `border`. | |
| 5. **Coercion rejects (drops) out-of-range points instead of clamping.** | |
| `_unit_number_list` returns `None` if any coordinate is outside `[0,1]` | |
| (`snap2sim/model_io.py:311-315`), so a point like `[1.02, 0.5]` discards the | |
| *entire* annotation and the part may then fall through to "unavailable". The | |
| browser already `clamp01`s at render (`index.html:1120-1122`), so consider | |
| clamping (not rejecting) server-side for resilience. Minor robustness item; | |
| the prompt already asks for `[0,1]` (`snap2sim/prompts.py:31-34`). | |
| The point-coordinate contract itself is consistent end-to-end: normalized | |
| `[0,1]`, **origin top-left**, asserted in the prompt (`snap2sim/prompts.py:31-34`) | |
| and consumed as `point[0]→x*width`, `point[1]→y*height` from the image's | |
| top-left (`index.html:1040-1047`). No change needed there. | |
| --- | |
| ## Security / invariants to preserve | |
| - This is a client-side CSS/JS change to an existing render path; keep the | |
| "no model-authored HTML" rule from `AGENTS.md`/`SECURITY.md`. `label`/`note` | |
| must stay `textContent`, never `innerHTML` (already correct, | |
| `index.html:1051-1058`). A new marker-dot/box element must be built with | |
| `document.createElement` + style, not injected markup. | |
| - Keep using the **local** `currentPreviewUrl` object URL for the photo | |
| (`index.html:1018`, `index.html:1134-1139`); do not round-trip the image back | |
| from the server for display. | |
| - No change to rate limiting, upload caps, or decompression-bomb guards in | |
| `app.py` is required. | |
| --- | |
| ## Suggested implementation order for Codex | |
| 1. **Sizing fix first (Option A).** CSS-only in `index.html`. This is the bug | |
| the user reported and it also corrects most annotation placement. | |
| 2. **Re-verify in a real browser** with both a **tall (portrait)** and a **wide | |
| (landscape)** photo that triggers the annotate fallback: the whole image is | |
| visible and contained, no viewport overflow/scroll, and callouts land on the | |
| correct components within the image (especially lower-half points that were | |
| previously clipped). Check the ≤860px stacked layout (`index.html:555-581`). | |
| 3. **Then** the annotation-accuracy items (in scope per the user): marker dot at | |
| the true point + offset/leader label (#1, #2), and render `annotation.box` | |
| when present (#3). Items #4/#5 optional. | |
| 4. Run the standard local checks per `AGENTS.md` (schema/parser + FastAPI | |
| `TestClient` for `/`, `/analyze_image`, `/generate_scene`), then the normal | |
| PR -> GitHub Actions HF sync -> Space verification. | |
| ## Resolved scope | |
| - **Sizing fix + annotation-accuracy polish are both in scope** | |
| (user decision, 2026-06-14): the CSS sizing fix, marker dots at the true | |
| point, offset/leader labels, and rendering `annotation.box` when the model | |
| provides it. | |
| - Final submission note, 2026-06-15: this work shipped before the public | |
| `build-small-hackathon/Snap2Sim` submission. | |