From 3c51332ac19c3e7f274f800ce8e4ed948fb94c6b Mon Sep 17 00:00:00 2001 From: AlexZ005 Date: Sat, 8 Aug 2026 21:41:57 +0300 Subject: [PATCH 1/2] [fix] weld/create-face refresh the session in place - stale edit wireframe gone User report: the edit wireframe overlay stayed unchanged after a weld. Weld (and create-face) still did a pre-D1 exit->enter dance to regroup handles, so the overlay rebuild depended on re-entry succeeding; any early-out left the old wireframe on screen. Since D1, applyMeshGeo refreshes a live vertex session in place (handles + overlay + selection) - weld and create-face now ride that same path, one refresh route shared with undo and remote commits. - also fixes a latent size-cap bug the trace surfaced: a too-large weld mutated the position attribute in place, then failed the commit WITHOUT reverting (and never set needsUpdate) - mesh, wire and peers silently diverged; the raw attribute is now restored on a failed commit - e2e: mesh-ops +2 (overlay fingerprint tracks the welded geometry; the session survives the weld with no exit/enter); regressions green (mesh-edit-popup, vertices-create-face, vr-face-weld, vr-face-edit, vr-mesh-undo, faces-toolbar); build green; 419/62 held Co-Authored-By: Claude Fable 5 --- src/lib/meshEdit.js | 35 ++++++++++++++++++++++------------- tests/e2e/mesh-ops.test.cjs | 22 +++++++++++++++++++++- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/src/lib/meshEdit.js b/src/lib/meshEdit.js index 63a0e6b..9497099 100644 --- a/src/lib/meshEdit.js +++ b/src/lib/meshEdit.js @@ -363,10 +363,11 @@ export function clearVertexSelection() { } /** - * B4: WELD the ctrl-multi-selected vertices (>=2 handles) to their shared - * centroid — replicated + ONE undo entry. Committed as a meshgeo snapshot - * (a 'verts' entry holds one position for all indices, so it cannot undo - * per-handle befores). Re-enters edit mode so the merged handles regroup. + * B4: WELD the selected vertices (>=2 handles) to their shared centroid — + * replicated + ONE undo entry. Committed as a meshgeo snapshot (a 'verts' + * entry holds one position for all indices, so it cannot undo per-handle + * befores). The commit's session refresher regroups the merged handles and + * rebuilds the wireframe overlay in place. * @returns {boolean} */ export function weldSelectedVerts() { @@ -379,6 +380,10 @@ export function weldSelectedVerts() { // triangles ("weld mangles the mesh"), and undo replayed the same wrong // representation. Both snapshots below are in applyMeshGeo's representation. const before = trisToPositions(readTriangles(edited.geometry)); + // raw attribute copy so a FAILED commit (size cap) can revert the in-place + // centroid write — without it the attribute silently diverged from what + // peers and the GPU see (needsUpdate was never set on that path) + const rawBefore = Array.from(position.array); const centroid = new THREE.Vector3(); const picked = [...vertexSelection]; picked.forEach((i) => centroid.add(handles[i].position)); @@ -390,9 +395,17 @@ export function weldSelectedVerts() { ); const after = trisToPositions(readTriangles(edited.geometry)); if (JSON.stringify(before) === JSON.stringify(after)) return false; // already coincident - exitEditMode(); // handles regroup on re-entry (merged verts share a key now) + // NO exit/enter dance (a pre-D1 relic): commitMeshGeoSnapshot swaps the + // geometry and applyMeshGeo's session refresher rebuilds handles, wireframe + // overlay and selection IN PLACE — the same path undo and remote commits + // take, so the overlay can never diverge from the welded mesh (the dance + // left it stale whenever re-entry took any early-out). const ok = commitMeshGeoSnapshot(uuid, before, after); - enterEditMode(uuid); + if (ok) clearVertexSelection(); // the weld consumed the multi-pick + else { + position.array.set(rawBefore); + position.needsUpdate = true; + } return ok; } @@ -403,13 +416,9 @@ export function createSelectedFace(viewerPos = null) { const uuid = edited.uuid; const verts = [...vertexSelection].map((i) => handles[i].position.clone()); const ok = createFaceFromVerts(uuid, verts, viewerPos); - if (ok) { - // geometry changed under us: rebuild the handle visuals from the new mesh - vertexSelection.clear(); - selectedHandle = -1; - exitEditMode(); - enterEditMode(uuid); - } + // the commit's applyMeshGeo already rebuilt the session in place (D1 + // refresher) — same no-dance rule as weldSelectedVerts + if (ok) clearVertexSelection(); return ok; } diff --git a/tests/e2e/mesh-ops.test.cjs b/tests/e2e/mesh-ops.test.cjs index 14c9b8a..5b83aaf 100644 --- a/tests/e2e/mesh-ops.test.cjs +++ b/tests/e2e/mesh-ops.test.cjs @@ -498,8 +498,23 @@ h.run(async () => { me.selectHandle(2); me.toggleVertexSelection(3); const weldOk = me.weldSelectedVerts(); + // the wireframe overlay must track the welded geometry (user report: + // it stayed stale) — fingerprint it against a fresh WireframeGeometry + const o = live().children.find((c) => c.name === 'edit-overlay'); + const fresh = new s.THREE.WireframeGeometry(live().geometry); + const sum = (/** @type {any} */ a) => { + let t = 0; + for (let i = 0; i < a.length; i++) t += a[i]; + return Math.round(t * 1e3); + }; + const wireTracksWeld = + !!o && + o.geometry.attributes.position.array.length === fresh.attributes.position.array.length && + sum(o.geometry.attributes.position.array) === sum(fresh.attributes.position.array); + let weldKeptSession; + me.editingObject.subscribe((v) => (weldKeptSession = v === uuid))(); me.exitEditMode(); - resolve({ single, gizmoAtAdded, multi, moved, undone, parked, weldOk }); + resolve({ single, gizmoAtAdded, multi, moved, undone, parked, weldOk, wireTracksWeld, weldKeptSession }); }, 400) ); }); @@ -518,6 +533,11 @@ h.run(async () => { h.check(multiSel.undone, 'ONE undo restores the whole multi-drag (meshgeo entry)'); h.check(multiSel.parked, 'emptying the selection parks the gizmo'); h.check(multiSel.weldOk, 'the reported flow welds: plain click + one Ctrl+click'); + h.check( + multiSel.wireTracksWeld, + 'the edit wireframe overlay tracks the welded geometry (no stale wire)' + ); + h.check(multiSel.weldKeptSession, 'weld refreshes the session in place (no exit/enter dance)'); await h.finish(browser); }); From 59c0af8949a9b1ffaa3959fdee1dd8755e81ad7e Mon Sep 17 00:00:00 2001 From: AlexZ005 Date: Sat, 8 Aug 2026 22:00:47 +0300 Subject: [PATCH 2/2] [feat] face multi-select by default w/ live counts, inset selects its cap + selection-aware move, gizmo re-seat, face-basis local/world gizmo, per-axis face scale Roadmap 15-E (E10 -> E6 -> E7 -> E9 -> E8), the unifying rule: the selection IS the op target whenever non-empty; hover/highlight is the fallback. - E10: ctrl/shift-click ADDS to the face selection everywhere (desktop had no ctrl path at all); the Multi toolbar button is retired (the store + toggleFaceMulti stay - VR drives them); pickFaceUnit = plain click replaces the selection with the granularity-aware unit; stale-selection auto-heal in highlightFaceByTriangle (aiming outside the selection dissolves it; additive clicks skip the heal or it would wipe what they add to); live counts segment #mesh-sel-counts (N faces . M tris + the boundary-edge counts of exactly-2 faces, mismatch tinted red - bridge mismatches visible BEFORE clicking) - E6: inset/extrude/move KEEP the cap selected (indices survive: cloneTris keeps order, ring/walls append) + highlight re-derived - groupFaces re-merges a coplanar inset cap with its ring, so "inset then Move the cap" was impossible; the gizmo drag path reads the same selection-first target (the CL-B stash) - E7: the gizmo comes back seated on the new cap after an inset/extrude/ move commit; NEVER on arm (the B1 fix holds - mesh-ops B1 assertions stay green); Scene's tail only re-seats armed Move on non-commit clicks - E9: face-basis gizmo - Z = the face WORLD normal, X/Y deterministic tangents (least-aligned world axis seed); persisted faceGizmoSpace pref + [Local|World] toolbar segment; setSpace LEAK fixed (detach/exit restore 'world' - it used to stick to the shared TransformControls); rotation deltas conjugated through R = objQuat^-1 * proxyStartQuat (identity pre-E9, so the old path is a special case) - E8: per-axis face scale - applyFaceGrab takes scale: Vector3|number + scaleQuat, diagonal scale sandwiched in the proxy frame (R*S*R^-1); a number stays uniform so every VR caller is untouched - attachFaceGizmo never seats in VR (isVRMode guard - commitFaceOp now calls it, and VR commits ride the same code) - e2e: NEW mesh-inset-move (6); desktop-face-gizmo +7 (proxy Z = normal, space toggle + leak fix, tangential (2,1,1) scale w/ normal extent unchanged, rotate-about-normal coplanar, committed rotation watertight); faces-toolbar +5 (Multi gone, counts, equal/mismatch edge tints); regressions green: mesh-ops, mesh-edit-popup, novr-inset, vr-face-edit, vr-face-polygon, vr-face-live-adjust. Build green; svelte-check 419/62. Co-Authored-By: Claude Fable 5 --- src/components/Scene.svelte | 40 +++-- src/components/menu/MeshEditPopup.svelte | 57 ++++++-- src/lib/faceEdit.js | 179 ++++++++++++++++++++--- tests/e2e/desktop-face-gizmo.test.cjs | 98 +++++++++++++ tests/e2e/faces-toolbar.test.cjs | 67 +++++++++ tests/e2e/mesh-inset-move.test.cjs | 88 +++++++++++ 6 files changed, 477 insertions(+), 52 deletions(-) create mode 100644 tests/e2e/mesh-inset-move.test.cjs diff --git a/src/components/Scene.svelte b/src/components/Scene.svelte index 2c615e9..a1b86ee 100644 --- a/src/components/Scene.svelte +++ b/src/components/Scene.svelte @@ -21,7 +21,7 @@ import { capturePathClick } from '$lib/pathCapture'; import { surfaceSnap, dropToSurface } from '$lib/snapping'; import { editingObject, exitEditMode, raycastHandles, clearVertexSelection, onProxyMoved, onProxyDragChanged, tickMeshEdit } from '$lib/meshEdit'; - import { faceEditObject, faceEditOp, commitArmedFaceOp, exitFaceEdit, highlightFaceByTriangle, attachFaceGizmo, detachFaceGizmo, onFaceGizmoMoved, onFaceGizmoDragChanged, autoApplyFaceOp, faceEditMulti, toggleFaceSelection, clearFaceSelection, lookupEditable } from '$lib/faceEdit'; + import { faceEditObject, faceEditOp, commitArmedFaceOp, exitFaceEdit, highlightFaceByTriangle, attachFaceGizmo, detachFaceGizmo, onFaceGizmoMoved, onFaceGizmoDragChanged, autoApplyFaceOp, faceEditMulti, toggleFaceSelection, clearFaceSelection, pickFaceUnit, lookupEditable } from '$lib/faceEdit'; import { fireObjectClick } from '$lib/flowRuntime'; import { initVRControls, updateVRControls, raycastMenu, raycastPanel, raycastPalette, raycastProps, raycastPrefabs, raycastKeyboard, raycastChat, raycastEdit, raycastSnap, raycastSettings, raycastApprove, placePrefabGhost, vrFaceTrigger, vrVertexTrigger, vrVertexGrabStart, vrVertexGrabEnd, beginStretchSliderDrag, endStretchSliderDrag, executeVRMenuAction, resetWorldRig, onInputSourcesChange, worldToContentPose, boxSelectStart, boxSelectEnd, boxSelectActive, applyVRFrameRate, shouldSendHands, onHandPinchStart, onHandPinchEnd, pinchMenuToggledAt, firePingIfArmed, vrModuleTriggerStart, vrModuleTriggerEnd, vrModuleSelectSwallowed } from '$lib/vrControls'; import { vrKeyboardTarget } from '$lib/vrKeyboard'; @@ -590,19 +590,33 @@ const edited = lookupEditable($faceEditObject); const hit = edited ? selectionRaycaster.intersectObject(edited, false)[0] : null; const tri = hit && hit.faceIndex != null ? hit.faceIndex : -1; - highlightFaceByTriangle(tri); - // 212: Multi mode accumulates picks (the op button applies to the set); - // otherwise 176 auto-applies the active extrude/inset on the click + // E10: ctrl/shift-click ADDS to the selection (never auto-applies); a + // plain click REPLACES it with the unit under the cursor. The heal + // flag is off for additive clicks — the heal would wipe the very + // selection they are adding to. + const additive = event.ctrlKey || event.shiftKey || event.metaKey || $faceEditMulti; + highlightFaceByTriangle(tri, !additive); if (tri >= 0) { - if ($faceEditMulti) toggleFaceSelection(tri); - else autoApplyFaceOp(); - } else clearFaceSelection(); // D2: a miss drops the accumulated multi-pick - // B1 (inset fix): a seated MOVE gizmo intercepts the NEXT click (the - // dragging||axis guard above skips face dispatch), so click 2 of an - // armed inset/extrude DRAGGED the face instead. Only the Move op - // seats the gizmo; a miss still detaches it. - if ($faceEditOp === 'move' || tri < 0) attachFaceGizmo(); - else detachFaceGizmo(); + if (additive) { + toggleFaceSelection(tri); + // B1: only the armed Move op keeps a gizmo on a non-commit click + if ($faceEditOp === 'move') attachFaceGizmo(); + else detachFaceGizmo(); + } else { + pickFaceUnit(tri); + const committed = autoApplyFaceOp(); + // E7: a commit re-seats the gizmo itself (on the new cap); + // otherwise the B1 rule holds — a seated gizmo intercepts the + // NEXT click, so only Move keeps one armed + if (!committed) { + if ($faceEditOp === 'move') attachFaceGizmo(); + else detachFaceGizmo(); + } + } + } else { + clearFaceSelection(); // D2: a miss drops the accumulated multi-pick + attachFaceGizmo(); // no target left -> detaches + } return; } // light pick-proxies select their light (lights have no raycastable geometry) diff --git a/src/components/menu/MeshEditPopup.svelte b/src/components/menu/MeshEditPopup.svelte index a612b77..4a5b58c 100644 --- a/src/components/menu/MeshEditPopup.svelte +++ b/src/components/menu/MeshEditPopup.svelte @@ -30,9 +30,9 @@ commitFaceOp, faceEditGranularity, setFaceGranularity, - faceEditMulti, faceEditSelectedTris, - toggleFaceMulti, + faceSelectionInfo, + faceGizmoSpace, meshEditWireframe, meshEditHotkeys } from '$lib/faceEdit'; @@ -95,21 +95,24 @@ } ]; - /** a target exists for a one-shot op (multi selection or a picked unit) */ + /** a target exists for a one-shot op (E10: the selection first, else a picked unit) */ function hasTarget() { - if ($faceEditMulti && $faceEditSelectedTris.length) return true; + if ($faceEditSelectedTris.length) return true; if (($faceEditGranularity === 'face' ? $faceEditHighlight : $faceEditHoverTri) >= 0) return true; return false; } + // E10: live counts — selected faces/tris + boundary-edge counts when exactly + // two faces are picked (a bridge mismatch shows BEFORE clicking Bridge) + const selInfo = $derived.by(() => { + void $faceEditSelectedTris; // the trigger; the geometry poke rides objectsGroup + void $objectsGroup; + return faceSelectionInfo(); + }); + /** @param {string} op */ function runOp(op) { const spec = OPS.find((o) => o.op === op); - // 212: Multi mode — the op button applies to the whole accumulated selection - if ($faceEditMulti && $faceEditSelectedTris.length) { - commitFaceOp(/** @type {any} */ (op), $faceEditAmount); - return; - } if (op === 'bridge') { commitFaceOp('bridge', 0); // validates the two-face selection + toasts return; @@ -237,13 +240,15 @@ > {/each} - + + + {selInfo.faces} face{selInfo.faces === 1 ? '' : 's'} · {selInfo.tris} tri{selInfo.tris === 1 ? '' : 's'}{#if selInfo.loops} + · {selInfo.loops[0]} ↔ {selInfo.loops[1]} edges{/if} + @@ -263,6 +268,26 @@ > {/each} + + + + +
+ + +
{:else} diff --git a/src/lib/faceEdit.js b/src/lib/faceEdit.js index c245843..326c3ef 100644 --- a/src/lib/faceEdit.js +++ b/src/lib/faceEdit.js @@ -1,7 +1,7 @@ // @ts-ignore - no bundled three type declarations (project-wide) import * as THREE from 'three'; import { writable, get } from 'svelte/store'; -import { globalScene, objectsGroup, TControls, lockedObjects } from '../stores/sceneStore'; +import { globalScene, objectsGroup, TControls, lockedObjects, isVRMode } from '../stores/sceneStore'; import { peers, showToast, settingsOpen, settingsSection } from '../stores/appStore'; import { registerHistoryKind, recordEntry } from './history'; @@ -351,9 +351,10 @@ export function flipFaceNormals(tris, targetTris) { } /** Ordered boundary LOOP of a tri set: its directed boundary edges walked - * p1 -> next p0. Null unless the boundary is ONE closed loop. + * p1 -> next p0. Null unless the boundary is ONE closed loop. (E10: exported + * for faceSelectionInfo's live edge counts.) * @param {any[]} tris @param {number[]} triIndices @returns {any[] | null} */ -function boundaryLoop(tris, triIndices) { +export function boundaryLoop(tris, triIndices) { const dir = boundaryEdges(tris, { triIndices }); if (dir.length < 3) return null; /** @type {Map} */ @@ -807,17 +808,25 @@ export function toggleFaceMulti() { clearFaceSelection(); } -/** The tri indices the next op targets: the multi selection, else the hovered - * unit (polygon = the tri; face = the coplanar group under the ray/highlight). +/** The tri indices the next op targets: the SELECTION whenever non-empty + * (E10 — the unifying rule, Multi no longer gates it), else the hovered unit + * (polygon = the tri; face = the coplanar group under the ray/highlight). * @returns {number[]} */ function opTargetTris() { - if (get(faceEditMulti) && get(faceEditSelectedTris).length) + if (get(faceEditSelectedTris).length) return get(faceEditSelectedTris).filter((/** @type {number} */ ti) => workingTris[ti]); if (granularity() !== 'face') return pickFaceUnitTris(get(faceEditHoverTri)); const fi = get(faceEditHighlight); return fi >= 0 && faces[fi] ? faces[fi].triIndices.filter((/** @type {number} */ ti) => workingTris[ti]) : []; } +/** E10: a plain (non-additive) pick REPLACES the selection with the + * granularity-aware unit under the cursor. @param {number} tri */ +export function pickFaceUnit(tri) { + faceEditSelectedTris.set(pickFaceUnitTris(tri)); + refreshFaceOverlay(); +} + /** Synthesize a face {triIndices, normal, centroid} from the op target tris (212). * For a single coplanar group this equals the groupFaces() face, so the default * FACE path is unchanged. */ @@ -835,10 +844,11 @@ function opTargetFace() { return { triIndices: tris, normal: normal.normalize(), centroid: centroid.divideScalar(cnt || 1) }; } -/** Tris to tint in the overlay: the multi selection plus the hovered unit (212) */ +/** Tris to tint in the overlay: the selection plus the hovered unit (212; + * E10 — the selection always shows, Multi no longer gates it) */ function overlayTris() { const set = new Set(); - if (get(faceEditMulti)) get(faceEditSelectedTris).forEach((t) => set.add(t)); + get(faceEditSelectedTris).forEach((t) => set.add(t)); pickFaceUnitTris(get(faceEditHoverTri)).forEach((t) => set.add(t)); return [...set].filter((ti) => workingTris[ti]); } @@ -955,17 +965,33 @@ export function exitFaceEdit() { * index and highlight it. Returns TRUE only when the highlight changed, so the * caller ticks a haptic once and the overlay rebuilds once (121). A negative * triangleIndex clears the highlight. @param {number} triangleIndex + * @param {boolean} [healStale] E10: outside Multi mode, aiming at anything + * OUTSIDE the current selection dissolves it (the cap E6 keeps selected is a + * convenience, not a lock) — VR calls this per-frame from the beam, so VR + * trigger semantics stay identical. Additive (ctrl) desktop clicks pass false + * or the heal would wipe the selection they are adding to. */ -export function highlightFaceByTriangle(triangleIndex) { +export function highlightFaceByTriangle(triangleIndex, healStale = true) { // 212: track the raw tri (polygon picking) + refresh per-tri in polygon mode const prevTri = get(faceEditHoverTri); faceEditHoverTri.set(triangleIndex); const fi = triangleIndex < 0 ? -1 : faces.findIndex((f) => f.triIndices.includes(triangleIndex)); const prevFi = get(faceEditHighlight); faceEditHighlight.set(fi); + let healed = false; + if (healStale && triangleIndex >= 0 && !get(faceEditMulti)) { + const sel = get(faceEditSelectedTris); + if (sel.length) { + const set = new Set(sel); + if (!pickFaceUnitTris(triangleIndex).every((t) => set.has(t))) { + faceEditSelectedTris.set([]); + healed = true; + } + } + } // triangle/shell units are picked per-tri, so refresh per raw tri change const changed = granularity() !== 'face' ? triangleIndex !== prevTri : fi !== prevFi; - if (changed) refreshFaceOverlay(); + if (changed || healed) refreshFaceOverlay(); return changed; } @@ -979,6 +1005,33 @@ export function faceIndexForTriangle(triangleIndex) { return triangleIndex < 0 ? -1 : faces.findIndex((f) => f.triIndices.includes(triangleIndex)); } +/** + * E10: live counts for the toolbar — selected tris, the logical faces they + * cover, and (with EXACTLY two faces) their boundary-edge counts, so a bridge + * mismatch is visible BEFORE clicking. @returns {{tris: number, faces: number, + * loops: [number, number] | null}} + */ +export function faceSelectionInfo() { + const sel = get(faceEditSelectedTris).filter((/** @type {number} */ ti) => workingTris[ti]); + if (!sel.length) return { tris: 0, faces: 0, loops: null }; + /** @type {Set} */ + const faceSet = new Set(); + sel.forEach((/** @type {number} */ ti) => { + const fi = faceIndexForTriangle(ti); + if (fi >= 0) faceSet.add(fi); + }); + /** @type {[number, number] | null} */ + let loops = null; + if (faceSet.size === 2) { + const [a, b] = [...faceSet]; + loops = [ + boundaryLoop(workingTris, faces[a].triIndices)?.length ?? 0, + boundaryLoop(workingTris, faces[b].triIndices)?.length ?? 0 + ]; + } + return { tris: sel.length, faces: faceSet.size, loops }; +} + /** the op target's world-space centroid + normal (for ghost/preview) — the * multi selection or hovered unit (212), falling back to the highlighted face */ export function highlightedFaceInfo() { @@ -1056,10 +1109,23 @@ export function commitFaceOp(op, amount) { applyGeometrySnapshot(positions); broadcastMeshGeo(faceEdited.uuid, positions); recordEntry({ kind: 'meshgeo', uuid: faceEdited.uuid, before, after: positions }); - // the geometry changed: any accumulated tri indices are now stale (212) - if (get(faceEditSelectedTris).length) faceEditSelectedTris.set([]); - // delete drops the face; keep the highlight only if it still exists - if (op === 'delete') faceEditHighlight.set(-1); + if (op === 'inset' || op === 'extrude' || op === 'move') { + // E6: keep the CAP selected — its tri indices survive the op (cloneTris + // keeps order, ring/walls APPEND). groupFaces re-merges a coplanar inset + // cap with its ring into ONE logical face, so the highlight alone cannot + // describe the cap; the selection is what makes "inset then Move the cap" + // possible at all. + faceEditSelectedTris.set([...face.triIndices]); + faceEditHighlight.set(faceIndexForTriangle(face.triIndices[0])); + refreshFaceOverlay(); + // E7: the gizmo comes back seated on the new cap (attachFaceGizmo reads + // the selection-first op target). NEVER on arm — that's the B1 fix. + if (typeof window !== 'undefined') attachFaceGizmo(); + } else { + // subdivide/flip/delete rebuild the topology: indices are stale (212) + if (get(faceEditSelectedTris).length) faceEditSelectedTris.set([]); + if (op === 'delete') faceEditHighlight.set(-1); + } return true; } @@ -1253,7 +1319,10 @@ export function beginFaceGrab(faceOrIndex) { /** * Apply a LOCAL-space rigid transform to the grabbed face around its centroid * (rebuilt from the snapshot each call — no drift). Pure + testable. - * @param {{dPos?: any, dQuat?: any, push?: number, scale?: number}} t + * E8: `scale` also takes a Vector3 (per-axis, in the PROXY frame given by + * `scaleQuat` — v' = R·S·R⁻¹·v); a plain number stays uniform, so every VR + * caller is untouched. + * @param {{dPos?: any, dQuat?: any, push?: number, scale?: number | any, scaleQuat?: any}} t */ export function applyFaceGrab(t) { if (!faceGrab || !faceEdited) return; @@ -1262,9 +1331,18 @@ export function applyFaceGrab(t) { const dQuat = t.dQuat || new THREE.Quaternion(); const scale = t.scale ?? 1; const pushVec = faceGrab.normal.clone().multiplyScalar(t.push || 0); + /** @type {(v: any) => any} diagonal scale sandwiched in the proxy frame */ + let applyScale; + if (typeof scale === 'number') { + applyScale = (v) => v.multiplyScalar(scale); + } else { + const R = t.scaleQuat || new THREE.Quaternion(); + const Rinv = R.clone().invert(); + applyScale = (v) => v.applyQuaternion(Rinv).multiply(scale).applyQuaternion(R); + } // the ONE rigid transform (about the face centroid) applied to a base vertex const xf = (/** @type {any} */ v) => - v.clone().sub(pivot).multiplyScalar(scale).applyQuaternion(dQuat).add(pivot).add(dPos).add(pushVec); + applyScale(v.clone().sub(pivot)).applyQuaternion(dQuat).add(pivot).add(dPos).add(pushVec); faceGrab.triIndices.forEach((/** @type {number} */ ti, /** @type {number} */ k) => { workingTris[ti] = faceGrab.originals[k].map(xf); }); @@ -1307,6 +1385,35 @@ export function cancelFaceGrab() { /** the op target the gizmo was seated on — what a drag actually moves */ /** @type {any} */ let gizmoTarget = null; +/** E9: face-gizmo space — 'local' = the FACE basis (Z = normal), 'world' = + * world axes. Persisted local pref. NOTE three r185: scale mode always + * orients local, whatever `.space` says. Declared AFTER faceProxy: the + * subscriber runs at module eval (the store-subscriber TDZ gotcha). + * @type {import('svelte/store').Writable<'local'|'world'>} */ +export const faceGizmoSpace = writable( + typeof localStorage !== 'undefined' && localStorage.getItem('faceGizmoSpace') === 'world' + ? 'world' + : 'local' +); +faceGizmoSpace.subscribe((value) => { + if (typeof localStorage !== 'undefined') localStorage.setItem('faceGizmoSpace', String(value)); + /** @type {any} */ + const controls = get(TControls); + // live flip while the face gizmo is seated + if (faceProxy && controls && controls.object === faceProxy) controls.setSpace?.(value); +}); + +/** E9: pick the world axis least aligned with n (deterministic tangent seed) + * @param {any} n */ +function axisLeastAlignedWith(n) { + const ax = Math.abs(n.x), + ay = Math.abs(n.y), + az = Math.abs(n.z); + if (ax <= ay && ax <= az) return new THREE.Vector3(1, 0, 0); + if (ay <= az) return new THREE.Vector3(0, 1, 0); + return new THREE.Vector3(0, 0, 1); +} + function ensureFaceProxy() { if (faceProxy) return faceProxy; const scene = get(globalScene); @@ -1338,6 +1445,7 @@ export function focusTargetFace() { */ export function attachFaceGizmo() { if (typeof window === 'undefined' || !faceEdited) return; + if (get(isVRMode)) return; // the desktop gizmo helper would render in-headset /** @type {any} */ const controls = get(TControls); const target = opTargetFace(); @@ -1351,9 +1459,16 @@ export function attachFaceGizmo() { if (!proxy) return; faceEdited.updateMatrixWorld(true); proxy.position.copy(faceEdited.localToWorld(target.centroid.clone())); - proxy.quaternion.copy(faceEdited.getWorldQuaternion(new THREE.Quaternion())); + // E9: FACE basis — gizmo Z = the face WORLD normal (push/pull), X/Y = its + // tangents (deterministic seed: the world axis least aligned with n). The + // old object-quaternion copy gave every face of an axis-aligned box the + // same handles. + const n = target.normal.clone().transformDirection(faceEdited.matrixWorld).normalize(); + const t = new THREE.Vector3().crossVectors(axisLeastAlignedWith(n), n).normalize(); + const bit = new THREE.Vector3().crossVectors(n, t); + proxy.quaternion.setFromRotationMatrix(new THREE.Matrix4().makeBasis(t, bit, n)); proxy.scale.setScalar(1); - controls.setSpace?.('local'); + controls.setSpace?.(get(faceGizmoSpace)); controls.attach(proxy); } @@ -1368,6 +1483,9 @@ export function detachFaceGizmo() { } faceProxyStart = null; gizmoTarget = null; + // E9 leak fix: setSpace('local') used to stick to the SHARED TransformControls + // after a face session, silently flipping normal object transforms to local + controls?.setSpace?.('world'); } /** Gizmo dragging-changed for the face proxy (163). @param {boolean} dragging */ @@ -1375,8 +1493,17 @@ export function onFaceGizmoDragChanged(dragging) { if (!faceEdited || !faceProxy) return; if (dragging) { // the target captured when the gizmo was seated (see attachFaceGizmo) - if (beginFaceGrab(gizmoTarget ?? get(faceEditHighlight))) - faceProxyStart = { pos: faceProxy.position.clone(), quat: faceProxy.quaternion.clone() }; + if (beginFaceGrab(gizmoTarget ?? get(faceEditHighlight))) { + // E9: R maps proxy-local into object-local (identity when the proxy + // copied the object frame, the pre-E9 case) — deltas measured in the + // proxy frame must be conjugated through it before hitting the verts + const objQuat = faceEdited.getWorldQuaternion(new THREE.Quaternion()); + faceProxyStart = { + pos: faceProxy.position.clone(), + quat: faceProxy.quaternion.clone(), + R: objQuat.invert().multiply(faceProxy.quaternion) + }; + } } else if (faceProxyStart) { faceProxyStart = null; commitFaceGrab(); // ONE meshgeo + undo; rebuilds the face cache @@ -1384,14 +1511,20 @@ export function onFaceGizmoDragChanged(dragging) { } } -/** Gizmo onchange for the face proxy — apply the rigid transform (163/162). */ +/** Gizmo onchange for the face proxy — apply the rigid transform (163/162; + * E9 face-basis frame conjugation; E8 per-axis scale). */ export function onFaceGizmoMoved() { if (!faceEdited || !faceProxy || !faceProxyStart) return; const dPos = faceEdited .worldToLocal(faceProxy.position.clone()) .sub(faceEdited.worldToLocal(faceProxyStart.pos.clone())); - const dQuat = faceProxyStart.quat.clone().invert().multiply(faceProxy.quaternion); - applyFaceGrab({ dPos, dQuat, scale: faceProxy.scale.x }); + // the delta in the PROXY frame, conjugated into object-local (dQuatLocal = + // R·dQuat·R⁻¹) — with the face-basis proxy the raw delta is in the wrong + // frame and a rotate-about-normal would smear the cap off its plane + const dQuatProxy = faceProxyStart.quat.clone().invert().multiply(faceProxy.quaternion); + const R = faceProxyStart.R; + const dQuat = R.clone().multiply(dQuatProxy).multiply(R.clone().invert()); + applyFaceGrab({ dPos, dQuat, scale: faceProxy.scale.clone(), scaleQuat: R }); } /** The op target as a synthesized face (multi selection / hovered polygon / diff --git a/tests/e2e/desktop-face-gizmo.test.cjs b/tests/e2e/desktop-face-gizmo.test.cjs index 56ed13c..ca9b25f 100644 --- a/tests/e2e/desktop-face-gizmo.test.cjs +++ b/tests/e2e/desktop-face-gizmo.test.cjs @@ -68,5 +68,103 @@ h.run(async () => { h.check(Math.abs(res.undone - res.before) < 1e-3, 'the face move is undoable'); h.check(res.detached, 'leaving face mode detaches the gizmo'); + // ---- 15-E (E9/E8): face-basis gizmo, Local/World toggle + setSpace leak + // fix, per-axis tangential scale, rotation-frame conjugation ---- + const e9 = await A.page.evaluate(() => { + const s = window.__stores; + const fe = s.faceEdit; + const T = s.THREE; + s.commandsHandler.sceneCommand('/create Box 1 1 1'); + let g; + s.objectsGroup.subscribe((v) => (g = v))(); + const box = g.children[g.children.length - 1]; + fe.enterFaceEdit(box.uuid); + let controls; + s.TControls.subscribe((c) => (controls = c))(); + const faces = fe.currentFaces(); + const xi = faces.findIndex((f) => f.normal.x > 0.9); + fe.pickFaceUnit(faces[xi].triIndices[0]); + fe.highlightFaceByTriangle(faces[xi].triIndices[0]); + fe.attachFaceGizmo(); + // E9: proxy Z = the +X face WORLD normal (was: the object quaternion, + // identical handles on every face of an axis-aligned box) + const z = new T.Vector3(0, 0, 1).applyQuaternion(controls.object.quaternion); + const zIsNormal = z.distanceTo(new T.Vector3(1, 0, 0)) < 1e-4; + const spaceLocal = controls.space === 'local'; + fe.faceGizmoSpace.set('world'); + const spaceWorld = controls.space === 'world'; + fe.faceGizmoSpace.set('local'); + + /** verts on the +X plane (cap + welded corner instances) */ + const readPlane = () => { + const p = box.geometry.attributes.position; + const out = { xs: [], ys: [], zs: [] }; + for (let i = 0; i < p.count; i++) + if (p.getX(i) > 0.499) { + out.xs.push(p.getX(i)); + out.ys.push(p.getY(i)); + out.zs.push(p.getZ(i)); + } + return out; + }; + const span = (/** @type {number[]} */ a) => Math.max(...a) - Math.min(...a); + + // E8: scale (2,1,1) in the PROXY frame stretches exactly one tangent — + // for n=+X the deterministic tangent seed makes proxy X the world Z axis + fe.onFaceGizmoDragChanged(true); + controls.object.scale.set(2, 1, 1); + fe.onFaceGizmoMoved(); + const scaled = readPlane(); + const zSpan = span(scaled.zs); + const ySpan = span(scaled.ys); + const xFlat = scaled.xs.every((x) => Math.abs(x - 0.5) < 1e-3); + fe.cancelFaceGrab(); + + // E9 invariant: rotate 90° about the face normal keeps the cap ON its plane + fe.attachFaceGizmo(); + fe.onFaceGizmoDragChanged(true); + const qz90 = new T.Quaternion().setFromAxisAngle(new T.Vector3(0, 0, 1), Math.PI / 2); + controls.object.quaternion.multiply(qz90); // a proxy-local delta + fe.onFaceGizmoMoved(); + const rotFlat = readPlane().xs.every((x) => Math.abs(x - 0.5) < 1e-3); + fe.cancelFaceGrab(); + + // no tears: commit a GENERIC rotation (25° — corners land at generic + // positions) and count odd edges (watertight = every edge shared by 2) + fe.attachFaceGizmo(); + fe.onFaceGizmoDragChanged(true); + const q25 = new T.Quaternion().setFromAxisAngle(new T.Vector3(0, 0, 1), (25 * Math.PI) / 180); + controls.object.quaternion.multiply(q25); + fe.onFaceGizmoMoved(); + fe.onFaceGizmoDragChanged(false); // commit + const tris = fe.readTriangles(box.geometry); + const counts = new Map(); + const key = (/** @type {any} */ v) => + Math.round(v.x * 1e4) + ',' + Math.round(v.y * 1e4) + ',' + Math.round(v.z * 1e4); + tris.forEach((t) => { + for (let e = 0; e < 3; e++) { + const k = [key(t[e]), key(t[(e + 1) % 3])].sort().join('|'); + counts.set(k, (counts.get(k) || 0) + 1); + } + }); + const oddEdges = [...counts.values()].filter((c) => c !== 2).length; + + // E9 leak fix: leaving face mode restores world space on the SHARED controls + fe.exitFaceEdit(); + const spaceRestored = controls.space === 'world'; + return { zIsNormal, spaceLocal, spaceWorld, zSpan, ySpan, xFlat, rotFlat, oddEdges, spaceRestored }; + }); + + h.check(e9.zIsNormal, 'E9: the gizmo Z axis is the face normal on the +X face'); + h.check(e9.spaceLocal && e9.spaceWorld, 'the Local/World toggle flips the live gizmo space'); + h.check( + Math.abs(e9.zSpan - 2) < 1e-3 && Math.abs(e9.ySpan - 1) < 1e-3, + `E8: per-axis (2,1,1) scale stretches one tangent only (z ${e9.zSpan.toFixed(2)}, y ${e9.ySpan.toFixed(2)})` + ); + h.check(e9.xFlat, 'the tangential scale leaves the normal extent unchanged'); + h.check(e9.rotFlat, 'E9: rotating 90° about the normal keeps the cap on its plane'); + h.check(e9.oddEdges === 0, `a committed rotation leaves no tears (${e9.oddEdges} odd edges)`); + h.check(e9.spaceRestored, 'E9 leak fix: exitFaceEdit restores world space'); + await h.finish(browser); }); diff --git a/tests/e2e/faces-toolbar.test.cjs b/tests/e2e/faces-toolbar.test.cjs index 9f06e24..dc679c8 100644 --- a/tests/e2e/faces-toolbar.test.cjs +++ b/tests/e2e/faces-toolbar.test.cjs @@ -44,5 +44,72 @@ h.run(async () => { h.check((await A.page.evaluate(opStore)) === 'extrude' && (await active('extrude')), 'clicking Extrude switches the active tool'); h.check(!(await active('inset')), 'only one op is active at a time'); + // ---- 15-E (E10): Multi button retired; live counts segment ---- + h.check( + await A.page.evaluate(() => !document.querySelector('#mesh-multi')), + 'the Multi button is retired (ctrl-click always adds)' + ); + await A.page.evaluate(() => { + const fe = window.__stores.faceEdit; + fe.setFaceGranularity('face'); + const faces = fe.currentFaces(); + const xi = faces.findIndex((f) => f.normal.x > 0.9); + fe.pickFaceUnit(faces[xi].triIndices[0]); + }); + await A.page.waitForTimeout(150); + const counts1 = await A.page.evaluate(() => + document.querySelector('#mesh-sel-counts')?.textContent?.replace(/\s+/g, ' ').trim() + ); + h.check(/1 face · 2 tris/.test(counts1 ?? ''), `counts show the picked face ("${counts1}")`); + + // two whole faces -> boundary-edge counts appear, equal = no red tint + const counts2 = await A.page.evaluate(() => { + const fe = window.__stores.faceEdit; + const faces = fe.currentFaces(); + const xn = faces.findIndex((f) => f.normal.x < -0.9); + fe.toggleFaceSelection(faces[xn].triIndices[0]); + return new Promise((r) => + setTimeout(() => { + const el = document.querySelector('#mesh-sel-counts'); + r({ + text: el?.textContent?.replace(/\s+/g, ' ').trim(), + red: !!el?.querySelector('.text-red-400') + }); + }, 150) + ); + }); + h.check( + /2 faces · 4 tris/.test(counts2.text ?? '') && /4 ↔ 4 edges/.test(counts2.text ?? ''), + `two faces show their boundary-edge counts ("${counts2.text}")` + ); + h.check(!counts2.red, 'equal edge counts are not tinted red'); + + // subdivide one face (8-edge boundary) -> mismatched counts tint red + const mismatch = await A.page.evaluate(() => { + const fe = window.__stores.faceEdit; + let faces = fe.currentFaces(); + const xn = faces.findIndex((f) => f.normal.x < -0.9); + fe.pickFaceUnit(faces[xn].triIndices[0]); + fe.commitFaceOp('subdivide', 0); // topology op clears the selection + faces = fe.currentFaces(); + const xi = faces.findIndex((f) => f.normal.x > 0.9); + const xn2 = faces.findIndex((f) => f.normal.x < -0.9); + fe.pickFaceUnit(faces[xi].triIndices[0]); + fe.toggleFaceSelection(faces[xn2].triIndices[0]); + return new Promise((r) => + setTimeout(() => { + const el = document.querySelector('#mesh-sel-counts'); + r({ + text: el?.textContent?.replace(/\s+/g, ' ').trim(), + red: !!el?.querySelector('.text-red-400') + }); + }, 150) + ); + }); + h.check( + /(4 ↔ 8|8 ↔ 4) edges/.test(mismatch.text ?? '') && mismatch.red, + `mismatched edge counts tint red ("${mismatch.text}")` + ); + await h.finish(browser); }); diff --git a/tests/e2e/mesh-inset-move.test.cjs b/tests/e2e/mesh-inset-move.test.cjs new file mode 100644 index 0000000..012fb4f --- /dev/null +++ b/tests/e2e/mesh-inset-move.test.cjs @@ -0,0 +1,88 @@ +// 15-E (E6/E7/E10): inset keeps its new CAP selected (groupFaces re-merges a +// coplanar cap with its ring, so the highlight alone cannot describe it); +// Move — commit or gizmo — moves ONLY the cap plus its welded ring verts; the +// gizmo comes back seated on the cap after a commit. +const h = require('./helpers.cjs'); + +h.run(async () => { + const browser = await h.launch(); + const A = await h.setupPage(browser, 'A'); + + const res = await A.page.evaluate(() => { + const s = window.__stores; + const fe = s.faceEdit; + s.commandsHandler.sceneCommand('/create Box 1 1 1'); + let g; + s.objectsGroup.subscribe((v) => (g = v))(); + const box = g.children[g.children.length - 1]; + fe.enterFaceEdit(box.uuid); + const read = (/** @type {any} */ store) => { + let v; + store.subscribe((/** @type {any} */ x) => (v = x))(); + return v; + }; + // inset the +X face + const faces = fe.currentFaces(); + const xi = faces.findIndex((f) => f.normal.x > 0.9); + fe.highlightFaceByTriangle(faces[xi].triIndices[0]); + const capTris = faces[xi].triIndices.length; + const insetOk = fe.commitFaceOp('inset', 0.3); + const sel = read(fe.faceEditSelectedTris); + // E7: the gizmo is seated after the commit + let controls; + s.TControls.subscribe((c) => (controls = c))(); + const gizmoBack = controls?.object?.userData?.isFaceProxy === true; + const planeXs = () => { + const p = box.geometry.attributes.position; + const xs = []; + for (let i = 0; i < p.count; i++) xs.push(p.getX(i)); + return xs; + }; + const before = planeXs(); + const maxBefore = Math.max(...before); + // MOVE the selected cap +0.4 along its normal + const moveOk = fe.commitFaceOp('move', 0.4); + const after = planeXs(); + const maxAfter = Math.max(...after); + // the outer +X plane (ring boundary + side corners) must stay at 0.5 + const stayedOuter = after.filter((x) => Math.abs(x - 0.5) < 1e-3).length; + const selAfterMove = read(fe.faceEditSelectedTris); + // the gizmo grab path targets the SAME selection + const began = fe.beginFaceGrab(fe.currentTargetFace()); + fe.applyFaceGrab({ dPos: new s.THREE.Vector3(0.2, 0, 0) }); + const dragMax = Math.max(...planeXs()); + fe.cancelFaceGrab(); + fe.exitFaceEdit(); + return { + insetOk, + capTris, + sel: sel.length, + gizmoBack, + moveOk, + maxBefore, + maxAfter, + stayedOuter, + selAfterMove: selAfterMove.length, + began, + dragMax + }; + }); + + h.check( + res.insetOk && res.sel === res.capTris, + `inset keeps its cap selected (${res.sel}/${res.capTris} tris)` + ); + h.check(res.gizmoBack, 'E7: the gizmo comes back seated after the commit'); + h.check( + res.moveOk && Math.abs(res.maxAfter - (res.maxBefore + 0.4)) < 1e-3, + `Move moves the selected cap only (+X ${res.maxBefore.toFixed(2)} -> ${res.maxAfter.toFixed(2)})` + ); + h.check(res.stayedOuter >= 8, `the outer +X plane stays put (${res.stayedOuter} entries at 0.5)`); + h.check(res.selAfterMove === res.sel, 'the cap stays selected through the move'); + h.check( + res.began && Math.abs(res.dragMax - (res.maxBefore + 0.6)) < 1e-3, + `the gizmo grab path moves the same selection (max ${res.dragMax.toFixed(2)})` + ); + + await h.finish(browser); +});