Review fixes round 2: honest WebGPU fallback, curve-edit preservation, lamp 3D leaks#109
Open
dompm wants to merge 11 commits into
Open
Review fixes round 2: honest WebGPU fallback, curve-edit preservation, lamp 3D leaks#109dompm wants to merge 11 commits into
dompm wants to merge 11 commits into
Conversation
The build script ran plain `tsc` against a solution-style tsconfig
("files": [] + references), which type-checks nothing and exits 0, so
real type errors shipped unchecked. Switch to `tsc -b`, fix everything
it surfaces, and add a CI workflow so it stays green:
- App.tsx: "Replace pattern" passed a File to updatePatternImage(url,
w, h), corrupting the project (and persisting the corruption via
autosave); route both upload and drag-drop through a shared
loadPatternImageFile helper
- App.tsx: lamp profile dialog referenced a nonexistent updateProject —
ReferenceError on pattern-scale unit change; use updatePatternScale
(same path as the measure tool)
- App.tsx: drop 11 props passed to SheetPanel that it never declared
or read
- SheetPanel: `forceTool` was an undefined name — ReferenceError when
switching to an uncalibrated sheet with the measure tool active;
align with the handleToolChange path which checks !sheet.scale only
- SheetPanel: CANVAS.fg doesn't exist; use var(--text-soft) for the
pack-popover label
- Lamp3DPreview: convert hoisted `function resize()` to a const arrow
so the container null-narrowing applies
- Tutorial: type STEPS as Record<AnchoredStepId, StepConfig> instead of
a Record keyed by the full StepId union it never covered
- Remove unused locals/imports; add vite-env.d.ts so the ?worker import
in packing.ts resolves; gitignore *.tsbuildinfo
- Add GitHub Actions workflow running pnpm build (tsc -b + vite) on
pushes to main and PRs
Closes #85
https://claude.ai/code/session_018ug1kMLE8aSdT8k4prFPYV
- saveToOPFS no longer swallows write errors; failures now propagate to persist() so the existing "Couldn't save / Retry" status UI actually fires (it was unreachable before) - loadProjectFromOPFS returns null only for genuine NotFoundError and throws on anything else (corrupt JSON, transient OPFS errors), so startup no longer mistakes a broken-but-present project for a first visit and overwrites it with an empty one; on load failure the app starts under "<name> (recovered)" so autosave can never clobber the original file - Rename now writes the project under the new name and deletes the old file only after the write succeeds, instead of deleting immediately and leaving a 500 ms window with no copy on disk - Flush the pending debounced save on pagehide so edits made within the debounce window survive closing the tab - Request navigator.storage.persist() once on startup (OPFS is best-effort storage and silently evictable otherwise) - Importing a project now clears the undo/redo stacks; undoing after an import could previously restore the prior project's state and persist it over that project's file - Export uses a Blob object URL instead of a data: URI, which hit browser URL limits on projects with embedded images - switchProject/deleteProject/createNewProject handle the now-throwing save/load calls explicitly (stay on current project on a failed switch; surface save failures via saveStatus) Closes #86 https://claude.ai/code/session_018ug1kMLE8aSdT8k4prFPYV
- SheetPanel: Konva synthesizes a click after every pointerup with no movement threshold, so committing a marquee selection immediately ran handleStageClick's background-deselect and wiped the selection in the same gesture. Suppress the synthesized click right after a marquee commit. The handler itself stays: on touch devices (where pointerdown pans instead of marqueeing) it is the only tap-to-deselect path. - Both canvas panels' single-letter tool shortcuts now ignore Cmd/Ctrl/Alt chords: Cmd+C no longer switches to crop, Cmd+V to select, and Cmd+S no longer toggles refine-remove. The pen tool's Cmd+Z vertex-pop is hoisted above the guard and unchanged. - App's Delete/Backspace handler now ignores events from <select> elements, matching the panels' own guards; pressing Delete with the piece popover's sheet dropdown focused deleted the selected piece. Fixes #95, partially addresses #94 (panel scoping and the listener- ordering race still need the shared shortcut hook). https://claude.ai/code/session_018ug1kMLE8aSdT8k4prFPYV
… model cache retry - Serve onnxruntime-web's wasm binary as a same-origin Vite asset instead of fetching it from jsdelivr at runtime. The WebGPU EP runs on top of the wasm runtime, so every user needed the CDN to be reachable for segmentation to initialize at all, and the hardcoded version string in the CDN URL could silently drift from the installed package. (#88) - Remove @huggingface/transformers: zero imports anywhere in src, and it bundles its own copy of onnxruntime-web that would conflict with ours if ever actually imported. (#88) - SAM backend: reject all in-flight requests when the worker crashes (GPU device lost, wasm OOM) instead of leaving their promises — and the pieces marked pending — hanging forever, and reset the singleton so the next call spawns a fresh worker. Handle messageerror too. (#90) - Nesting worker: post ERROR instead of logging the exception and reporting COMPLETE as if packing succeeded; packing promise now rejects on worker error/load failure and always terminates the worker, so the Pack spinner can't spin forever. (#90) - Model cache: a failed OPFS cache write no longer triggers a full re-download of the ~305 MB encoder that is already in memory, and the network fallback now checks res.ok instead of handing an HTML error page to InferenceSession.create as model bytes. (#89) https://claude.ai/code/session_018ug1kMLE8aSdT8k4prFPYV
…ions - The welcome-tutorial decision now waits for the OPFS project load (gated on isLoaded) instead of inspecting the empty placeholder project at mount; returning users with saved work but cleared localStorage were dumped into the tutorial on top of their project (#105) - handleAddPiece/handleUpdatePrompt clip segmentation results against the pieces as they are when the worker resolves (via an always-current ref) rather than the render-time snapshot; two boxes drawn in quick succession were each clipped without the other's final polygon, producing overlapping pieces (#97) - debugMask ImageBitmaps are closed when replaced instead of waiting for GC; repeated refine clicks accumulated tens of MB of pinned bitmap memory (#97) - Sheet IDs use crypto.randomUUID() like piece IDs instead of Date.now(), which could collide within a millisecond and break piece->sheet references (#98) https://claude.ai/code/session_018ug1kMLE8aSdT8k4prFPYV
For a tier whose radius decreases downward (Rt > Rb), the unrolled sector has L_top > L_bot, but three code paths assumed the opposite: - patternToSurface rejected every point in the tier because the d-range check used [L_top, L_bot] as [lo, hi]; pieces in contracting tiers vanished from the 3D preview entirely - The v parameter clamped its denominator positive with Math.max(1e-6, L_bot - L_top); for contracting tiers numerator and denominator are both legitimately negative, so v collapsed to 0 — and since reflowLampPoints round-trips every drawn polygon through patternToSurfaceRobust on profile edits, committing a profile change destructively flattened those polygons onto the tier's top arc - Same clamp in Lamp3DPreview's per-vertex mapping The inverse mapping (surfaceToPatternRobust) already handled both signs correctly, so only the forward direction needed fixing. Also guard the cylinder branches against 0/0 NaN from zero-size tiers, which passes straight through Math.min/max into the mesh position buffer. Verified numerically (esbuild-bundled module, node): contracting profile 800->400 now maps v=0.3 exactly where the inverse places it, round-trip reflow error < 1e-12 across a v/theta sweep, degenerate zero-height tier produces no NaN. The cross-tier horizontal mirroring of contracting tiers in the 3D preview (theta01 direction) is intentionally left untouched — it needs visual verification. See #92. https://claude.ai/code/session_018ug1kMLE8aSdT8k4prFPYV
findBestPlacement only applied gapPx against the bin walls; the inter-piece test was a raw intersection check, so pieces were packed flush against each other — leaving no clearance for the glass cutter, which is the entire purpose of the gap (defaultCuttingGapPx computes a 2 mm physical allowance). Pieces now conflict when they intersect, one contains the other, or any edge pair comes closer than gapPx (point-to-segment distance over edge pairs; both directions covers the min distance between non-crossing segments). The bounding-box prefilter is inflated by gapPx so the exact check sees near-misses, which also fixes exactly-touching edges slipping through segmentsIntersect's strict inequalities. Verified numerically: packing 30x30 squares into a 100x100 bin with gapPx=8 yields >= 8px separation between all pairs (10px after grid quantization); gapPx=0 still packs flush with no overlap. Addresses the headline bug of #91 (silently-skipped oversized pieces and the rotation reset are still open there). https://claude.ai/code/session_018ug1kMLE8aSdT8k4prFPYV
…urprise phases
First-run flow was: init downloads the decoder (~25 MB), progress hits
100% ("downloaded!"), bar hides — then the first encode kicks off the
~309 MB encoder download and the bar restarts near zero, which reads as
"it's downloading the same thing again" (observed during the tutorial).
- init() now registers the encoder files in the progress accounting up
front (first run only — skipped when they're already in the OPFS
cache), so the reported fraction covers the full first-run download
and never claims completion after just the decoder
- After the decoder is ready, the encoder download is warmed in the
background instead of waiting for the first encode; buffers are
released to the OPFS cache when no encode has claimed them, so blank
projects don't pin ~300 MB of worker memory
- Encoder fetches are deduplicated behind a shared promise so the
warm-up and a concurrent first encode can't download the 309 MB
twice; failed warm-ups remove their progress entries (no stalled bar)
and retry transparently on the first segment
https://claude.ai/code/session_018ug1kMLE8aSdT8k4prFPYV
…urve edits samWorker (#87): - Probe for a usable WebGPU adapter in the worker instead of hardcoding device 'webgpu' in the ready message — machines without WebGPU were told "WebGPU ready" while running single-threaded wasm - On the wasm fallback, set numThreads from hardwareConcurrency (capped at 8): the site is cross-origin isolated precisely so threaded wasm works, yet the fallback ran the ~300 MB encoder on one thread. The em-pthread top-of-module guard already isolates ORT's pthread workers from our init/onmessage - Status line now distinguishes "WebGPU ready" from the CPU fallback with a slowness warning geometry (#93, the verifiable half): - subtractPolygons strips polygon-clipping's closing duplicate vertex; piece polygons are stored open everywhere else and the stray vertex skewed flattenCurves/centroid/snapping - arePolygonsEqual now treats re-anchored, closed, or winding-reversed rings as equal (anchored cyclic compare, near-linear in the common case). Previously every curve edit near a neighbor compared unequal purely because of ring normalization, so handleUpdatePieceCurves cleared the piece's Bezier metadata even when nothing was clipped Verified with node against the bundled module: unchanged ring after a no-op clip is open and compares equal; a real subtraction still registers as a change; reversed/rotated/closed variants compare equal. The multipolygon/hole semantics of subtractPolygons (what should happen when a piece is split in two) remain open in #93. https://claude.ai/code/session_018ug1kMLE8aSdT8k4prFPYV
Lamp3DPreview (#99): - Dispose InstancedMesh instance buffers on rebuild (geometry/material disposal alone leaves the instanceMatrix GPU buffer behind, and the rebuild runs per pointer-move while dragging the profile editor) - Dispose the scene's meshes on unmount, not just the renderer, and forceContextLoss() so repeated dialog open/close can't exhaust the browser's WebGL context cap - Cap the module-level texture cache at 24 entries with oldest-first eviction; multi-MB data-URL textures from replaced uploads previously accumulated GPU memory for the app's lifetime Guards (#105): - smoothPolygon stops subdividing past 512 vertices; repeated Smooth clicks doubled the count each time (10 clicks: 30 -> ~30k vertices) - LampProfileDialog clamps a zero/invalid pattern scale; GRID_RAW = 0 made the axis-tick for-loops spin forever and NaN'd the view bounds - computeCentroid([]) returns (0,0) instead of (NaN, NaN) https://claude.ai/code/session_018ug1kMLE8aSdT8k4prFPYV
Deploying vitrai with
|
| Latest commit: |
5b49308
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://29ac7bf5.vitrai.pages.dev |
| Branch Preview URL: | https://claude-focused-rubin-pmcknj.vitrai.pages.dev |
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
2 participants
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.
Follow-up to #108, same approach: fixes that are verifiable without manual app testing, verified analytically/numerically where possible.
#87 — honest device detection + usable WASM fallback
device: 'webgpu'in the ready message. Machines without WebGPU were being told "WebGPU ready" while running single-threaded WASM.numThreadsis set fromhardwareConcurrency(capped at 8). The site ships COOP/COEP precisely so threaded WASM works — the fallback was paying the isolation tax and running the ~300 MB encoder on one thread anyway. The existingem-pthreadmodule guard isolates ORT's pthread workers from our init/onmessage, so this is safe by construction; still worth one run on a non-WebGPU machine (Firefox stable on Linux/macOS) if you have access to one.#93 (the verifiable half) — clipping no longer destroys curve edits
subtractPolygonsstrips polygon-clipping's closing duplicate vertex (piece polygons are stored open everywhere else).arePolygonsEqualnow compares rings cyclically (re-anchored, closed, or winding-reversed rings of the same polygon count as equal). Previously every curve edit near a neighbor compared "changed" purely due to ring normalization, sohandleUpdatePieceCurvesdiscarded the user's Bezier metadata even when nothing was actually clipped.#99 — lamp 3D resource leaks
InstancedMeshinstance buffers disposed on rebuild (the rebuild runs per pointer-move while dragging the profile editor — hundreds of leaked GPU buffers per drag session before).forceContextLoss(), so repeatedly opening/closing the lamp dialog can't exhaust the browser's WebGL context cap.#105 — remaining guards
smoothPolygoncaps at 512 vertices (was doubling per click, unbounded).LampProfileDialogclamps a zero/invalid pattern scale (GRID_RAW = 0previously made the axis-tick loops spin forever and hang the tab).computeCentroid([])returns (0,0) instead of NaN.Verification
tsc -bandpnpm buildgreen on every commit; geometry changes verified numerically against esbuild-bundled modules (details in commit messages).Closes #99. Partially addresses #87 (a friendlier in-UI warning/feature-gating for CPU users still open), #93, #105.
https://claude.ai/code/session_018ug1kMLE8aSdT8k4prFPYV
Generated by Claude Code