diff --git a/src/components/layout/Toolbar.vue b/src/components/layout/Toolbar.vue index 5a33701..8329ff0 100644 --- a/src/components/layout/Toolbar.vue +++ b/src/components/layout/Toolbar.vue @@ -4,6 +4,7 @@ COMPAS ThreeJs + @@ -11,6 +12,7 @@ + + diff --git a/src/components/tools/objects/AddObjectGroup.vue b/src/components/tools/objects/AddObjectGroup.vue new file mode 100644 index 0000000..aa1ef92 --- /dev/null +++ b/src/components/tools/objects/AddObjectGroup.vue @@ -0,0 +1,10 @@ + + + diff --git a/src/components/tools/objects/MaterialButton.vue b/src/components/tools/objects/MaterialButton.vue new file mode 100644 index 0000000..b74652e --- /dev/null +++ b/src/components/tools/objects/MaterialButton.vue @@ -0,0 +1,180 @@ + + + + + diff --git a/src/components/tools/objects/index.ts b/src/components/tools/objects/index.ts new file mode 100644 index 0000000..8f6c3c1 --- /dev/null +++ b/src/components/tools/objects/index.ts @@ -0,0 +1,3 @@ +export { default as AddObjectButton } from "./AddObjectButton.vue"; +export { default as MaterialButton } from "./MaterialButton.vue"; +export { default as AddObjectGroup } from "./AddObjectGroup.vue"; diff --git a/src/viewer/BIDIRECTIONAL_SYNC.md b/src/viewer/BIDIRECTIONAL_SYNC.md new file mode 100644 index 0000000..c8add16 --- /dev/null +++ b/src/viewer/BIDIRECTIONAL_SYNC.md @@ -0,0 +1,142 @@ +# Bidirectional sync — context for a future agent + +This documents the frontend half of making the viewer bidirectional: dragging an object, +adding a new one, and editing its material all send messages back to the backend, which +mutates the corresponding _live_ Python object rather than the frontend just displaying +whatever the backend last pushed. Before this work, outbound traffic was limited to UI +callbacks (`ui_callback`, `object_picked`, `object_action_callback`) — see +`ViewerRuntime.handleUiAction`/`handleObjectAction` in `viewer_runtime.ts` for that +existing pattern, which the new code follows. + +The paired backend implementation lives in the sibling `compas_threejs` repo, at +`src/compas_threejs/viewer/BIDIRECTIONAL_SYNC.md` — read that alongside this file, +especially for the exact message shapes each handler expects. Both repos carry this work +on a branch called `feature/bidirectional-sync`, branched off `main` in each. + +Everything below lives in `ViewerRuntime` (`viewer_runtime.ts`) unless noted otherwise. +All outbound sends go through the existing `sendData()` → `ViewerConnection.send()` path, +same as every pre-existing callback. + +## `object_transform` — the transform gizmo + +The gizmo (`TransformControls`) already existed, wired to picking, before this work — it +just didn't send anything. Two things were added: + +1. In the constructor, the existing `"dragging-changed"` listener now also captures + `this.dragStartMatrix = this.transformControls.object?.matrix.clone()` when a drag + _starts_ (`event.value === true`). +2. A new `"mouseUp"` listener (fires once, when the drag ends — unlike `"objectChange"`, + which fires every frame) calls `sendObjectTransform()`. + +`sendObjectTransform()` computes `delta = object.matrix.clone().multiply(dragStartMatrix.clone().invert())` +and sends it as `{dispatch: "object_transform", guid, matrix}`, where `matrix` is a +**row-major 4x4 nested list**. `THREE.Matrix4.elements` is column-major internally, so the +conversion explicitly transposes — see the comment at the transpose site if you touch +this, it's the kind of thing that silently breaks (this exact class of bug — wrong matrix +convention — is what caused `Remote`'s old camera/background messages to be dead code +before an earlier refactor, per the backend's `CONTEXTE.md`). + +**Why a delta, and why `dragStartMatrix` matters — read this before changing the math.** +Geometry conversion (`buildTransformationFromFrame` + `Object3D.applyMatrix4` in +`conversions/geometry.ts`) does **not** bake an object's frame into its vertex buffer. +`Object3D.applyMatrix4()` premultiplies the matrix into `object.matrix` and then +_decomposes_ it into `position`/`quaternion`/`scale`. So a freshly-converted mesh already +sits at its real, absolute world placement — it is not at identity. Two real bugs came +from getting this wrong, in order: + +- **Bug 1 — sent the absolute matrix as if it were a delta.** The original + implementation assumed `object.matrix` started at identity, so it sent the post-drag + matrix directly. The backend applied it via `geometry.transform(T)`, which composes `T` + _on top of_ the object's current state — so the object landed somewhere else entirely + (looked "inverted" or like it teleported). Fixed by capturing `dragStartMatrix` and + sending `M_after * M_before^-1` instead — see the backend doc's `Transformation` + section for why this composes correctly. +- **Bug 2 — a continuously self-animating object (e.g. a spinning torus with an + `App.loop` callback) fought its own drag.** The backend's loop calls `update_geometry` + many times a second regardless of what the frontend is doing. Every one of those + broadcasts was rebuilding the mesh mid-drag at the backend's last-known (not-yet-moved) + position, undoing the user's drag in real time — by `mouseUp`, the net movement was + ~zero, looking like the object "snapped back." Fixed in `manageGeometry` (see below). + +`manageGeometry` now has an early-return guard: if the incoming update's guid is the +object currently attached to `transformControls` **and** `transformControls.dragging` is +true, the update is dropped entirely rather than rebuilding the mesh out from under the +user. The next update after the drag ends — either the echo of the just-sent +`object_transform`, or the animation's next tick — resyncs normally. + +Separately, `manageGeometry` also carries gizmo attachment and highlight material over to +a freshly-rebuilt mesh when the _currently picked_ object's guid gets an update (e.g. the +echo of your own edit, or an unrelated animation tick while merely selected-but-not- +dragging) — otherwise every echo would silently detach the gizmo. + +**Known limitation, not solved**: the backend applies the delta on top of whatever its +live object's state is _at message-processing time_, which — for a continuously-animating +object — may have moved further since `dragStartMatrix` was captured (the drag can take a +second or more; the backend keeps animating the whole time). The result can carry a small +amount of "extra" motion corresponding to that elapsed animation. This is different from +(and much more minor than) Bug 2 above — it's an accepted characteristic of editing a live +object, not a bug to chase. + +## `create_geometry` — "Add object" toolbar button + +`ViewerRuntime.createGeometry(type, params)` sends +`{dispatch: "create_geometry", type, point: [x,y,z], params}`, where `point` is the +camera's current orbit target (`this.controls.target`) so new objects spawn in view +instead of at a fixed, possibly-buried world origin. + +UI: `src/components/tools/objects/AddObjectButton.vue` — a toolbar `Popover` (pattern +copied from `SavedViewsButton.vue`) with a shape-type `Select` and per-type `NumberField` +params. On "Add", it calls `createGeometry` and closes. + +**No new receive-side code was needed.** The created object comes back as an ordinary +`add_geometry` broadcast — the existing `manageGeometry`/`dispatch()` path renders it +exactly like anything a script adds. This symmetry (reusing the backend's existing +`add_geometry` outbound path) is why this was a small feature: the frontend only had to +learn to _send_ one new message, not _receive_ one. + +**Placement UX was deliberately kept simple**: spawn at a sensible default, then let the +user drag it into place with the (already-existing, already-fixed) gizmo — not a +click/drag-to-draw-in-3D-space sketch tool. That would need a new interaction state +machine (raycasting against a ground plane, live preview mesh, per-shape-type gesture +logic) and was explicitly scoped out as a much larger follow-up. + +## `material_edit` — toolbar color/metalness/roughness + +Two new `ViewerRuntime` methods: + +- `getMaterialSnapshot(guid)` — reads `this.geometryMaterials.get(guid)` → + `this.materials.get(materialGuid)`, returns `{color, metalness, roughness} | null`. + Returns `null` if the object has no material yet, or its `materialType` isn't + `"standard_material"` — this is the gate that keeps material editing scoped to + `compas_threejs.materials.Material`-backed objects; `PointMaterial`/`LineMaterial`/ + `PhysicalMaterial` have unrelated property sets (e.g. a point's material has `size`, not + metalness/roughness) and aren't editable through this control. +- `setMaterial(guid, {color?, metalness?, roughness?})` — mutates the local + `THREE.MeshStandardMaterial` **in place** first (instant visual feedback, no round-trip + wait), then sends `{dispatch: "material_edit", guid, ...fields}`. + +UI: `src/components/tools/objects/MaterialButton.vue`, folded into the same toolbar +group as `AddObjectButton`. Disabled unless something is picked. Color swatch + two +`Slider` controls (0–1, step 0.05) for metalness/roughness, each firing `setMaterial` on +every change — edits stream continuously as you drag, matching how this app's existing +dynamic `Slider`/`NumberField` UI components already behave (see `Openbar.vue`), and +deliberately _not_ the "send once on release" pattern `object_transform` uses — materials +aren't touched by any per-frame animation loop, so there's no equivalent of Bug 2 above to +worry about here. + +**New reactive store field**: `ViewerStore.pickedObjectGuid` (`viewer_store.ts`). Nothing +previously exposed "what's currently picked" to Vue — `pickedObject` was a private plain +TS field on `ViewerRuntime`. Set in `pickFromPointer` (on pick), cleared in +`clearPickedObject` (on deselect/Escape/pick-miss). `MaterialButton.vue`'s enable/disable +state and target guid both come from this. + +## Verifying changes here + +No test suite exists in this repo. Verification during this work: `vue-tsc` +(`npm run build`, which runs `vue-tsc --noEmit` across all tsconfigs before bundling) for +type safety, then manual end-to-end checks against a real running backend `App` — start +an example, pick/drag/add/recolor objects in the browser, and separately confirm the +backend's Python-side object state via ad hoc scripts (see the backend doc). After any +change here, the frontend must be rebuilt (`npm run build`) and the `dist/` output copied +into `compas_threejs/src/compas_threejs/viewer/frontend/` before it's reachable from a +real browser session — the backend serves its own bundled copy, not this repo live. diff --git a/src/viewer/viewer_runtime.ts b/src/viewer/viewer_runtime.ts index c7bc5ef..9f2c80c 100644 --- a/src/viewer/viewer_runtime.ts +++ b/src/viewer/viewer_runtime.ts @@ -126,6 +126,7 @@ export class ViewerRuntime { private disposed = false; private pickedObject: THREE.Object3D | null = null; private pickedMaterial: THREE.Material | THREE.Material[] | null = null; + private dragStartMatrix: THREE.Matrix4 | null = null; private readonly hiddenGuids = new Set(); private readonly highlightMaterial = new THREE.MeshStandardMaterial({ color: "orange", @@ -166,6 +167,19 @@ export class ViewerRuntime { this.transformHelper = this.transformControls.getHelper(); this.transformControls.addEventListener("dragging-changed", (event) => { this.controls.enabled = !event.value; + if (event.value) { + // Capture the object's world matrix as it stood right before this drag, so the + // delta sent to the backend on release is relative to it - NOT relative to + // identity. Conversion bakes each object's frame into its own position/quaternion + // (via THREE.Object3D.applyMatrix4, which decomposes into position/quaternion/ + // scale rather than baking into vertex data), so a freshly-built object already + // sits at its absolute world placement, not at the origin. + this.dragStartMatrix = + this.transformControls.object?.matrix.clone() ?? null; + } + }); + this.transformControls.addEventListener("mouseUp", () => { + this.sendObjectTransform(); }); this.scene.add(this.transformHelper); @@ -276,6 +290,67 @@ export class ViewerRuntime { }); } + /** + * Asks the backend to create a new geometry object of `type` (e.g. "box", "sphere", + * "point") with the given numeric `params`, spawned at the camera's current orbit + * target so it appears in view. The backend constructs the real COMPAS object and + * broadcasts it back via the existing add_geometry path - it arrives here exactly + * like any object added by a running script, so no new receive-side handling is + * needed. Pick it up with the transform gizmo afterwards to position it precisely. + */ + createGeometry(type: string, params: Record): void { + const point = this.vectorData(this.controls.target); + this.sendData({ + dispatch: "create_geometry", + type, + point: [point.x, point.y, point.z], + params, + }); + } + + /** + * Reads the current color/metalness/roughness of the object at `guid`, for + * pre-filling the material editor when it opens. Returns null if the object has no + * material yet, or its material isn't a "standard_material" (e.g. a Point's + * PointMaterial has an entirely different property set) - editing those is out of + * scope for this control. + */ + getMaterialSnapshot( + guid: string, + ): { color: string; metalness: number; roughness: number } | null { + const materialGuid = this.geometryMaterials.get(guid); + if (!materialGuid) return null; + const entry = this.materials.get(materialGuid); + if (!entry || entry.materialType !== "standard_material") return null; + const material = entry.material as THREE.MeshStandardMaterial; + return { + color: `#${material.color.getHexString()}`, + metalness: material.metalness, + roughness: material.roughness, + }; + } + + /** + * Applies a material edit both locally (instant visual feedback on the live + * THREE.Material - no need to wait for the backend round trip) and sends it to the + * backend so the corresponding live Material Python instance is updated the same way, + * e.g. via `examples/objects_action.py`'s "Make it blue" action. + */ + setMaterial( + guid: string, + fields: { color?: string; metalness?: number; roughness?: number }, + ): void { + const materialGuid = this.geometryMaterials.get(guid); + const entry = materialGuid ? this.materials.get(materialGuid) : undefined; + if (entry && entry.materialType === "standard_material") { + const material = entry.material as THREE.MeshStandardMaterial; + if (fields.color !== undefined) material.color.set(fields.color); + if (fields.metalness !== undefined) material.metalness = fields.metalness; + if (fields.roughness !== undefined) material.roughness = fields.roughness; + } + this.sendData({ dispatch: "material_edit", guid, ...fields }); + } + hideObjectInfo(): void { this.store.objectBarData.isVisible = false; } @@ -491,10 +566,31 @@ export class ViewerRuntime { } private manageGeometry(object: CommandRecord): void { - const converted = convertToThreeJSGeometry(object); const externalGuid = resolveExternalGeometryGuid(object); + const draggingTarget = externalGuid + ? this.geometries.get(externalGuid) + : undefined; + if ( + draggingTarget && + this.transformControls.dragging && + draggingTarget === this.transformControls.object + ) { + // The user is actively dragging this exact object with the gizmo - drop this + // incoming update instead of rebuilding it out from under them. This matters a lot + // for a continuously self-animating object (e.g. a spinning torus with an `App.loop` + // callback): its backend loop keeps calling update_geometry many times a second, + // and every one of those would otherwise swap in a freshly-converted mesh sitting at + // the backend's last-known (not-yet-moved) position, fighting the drag to a + // standstill so it looks like the object "snaps back" on release. The next update + // after the drag ends - the echo of our own object_transform, or the animation's + // next tick - resyncs to the real backend state. + return; + } + const converted = convertToThreeJSGeometry(object); const sceneKey = externalGuid ?? converted.uuid; const existing = this.geometries.get(sceneKey); + const wasSelected = + existing !== undefined && existing === this.pickedObject; if (existing) { this.scene.remove(existing); this.disposeObject(existing); @@ -513,6 +609,18 @@ export class ViewerRuntime { edges.layers.set(1); converted.add(edges); } + // If the replaced object was selected (e.g. this update is the echo of a gizmo edit + // the user just made), carry the selection - highlight material and gizmo attachment + // - over to the newly-built object instead of silently losing it. + if (wasSelected) { + this.pickedObject = converted; + if ("material" in converted) { + const renderable = converted as RenderableObject; + this.pickedMaterial = renderable.material ?? null; + renderable.material = this.highlightMaterial; + } + this.transformControls.attach(converted); + } } private manageMaterial(data: MaterialCommand): void { @@ -716,6 +824,7 @@ export class ViewerRuntime { } this.transformControls.attach(picked); const guid = this.findGeometryGuid(picked); + this.store.pickedObjectGuid.value = guid ?? null; if (guid) { this.store.selectedObjectGuid.value = guid; this.sendData({ dispatch: "object_picked", guid }); @@ -733,6 +842,7 @@ export class ViewerRuntime { this.pickedObject = null; this.pickedMaterial = null; this.transformControls.detach(); + this.store.pickedObjectGuid.value = null; this.store.objectBarData.data = null; this.store.objectActionsState.splice(0); this.store.selectedObjectGuid.value = null; @@ -751,6 +861,43 @@ export class ViewerRuntime { return undefined; } + /** + * Sends the object currently attached to the transform gizmo back to the backend as a + * delta transform, once dragging ends. Geometry conversion (`applyMatrix4` in + * `conversions/geometry.ts`) decomposes each object's frame into its own + * position/quaternion/scale (that's what `Object3D.applyMatrix4` does - it does NOT + * bake into vertex data), so `object.matrix` is already the object's absolute world + * placement both before and after a drag, not a delta relative to identity. What the + * backend needs is the delta between the placement captured at drag-start + * (`dragStartMatrix`, set in the `dragging-changed` listener above) and the placement + * after the drag - sending the absolute matrix instead would have the backend compose + * it on top of the object's current state a second time, landing it somewhere else + * entirely (this was the cause of a "moves to another location" bug). + */ + private sendObjectTransform(): void { + const object = this.transformControls.object; + const startMatrix = this.dragStartMatrix; + this.dragStartMatrix = null; + if (!object || !startMatrix) return; + + const delta = object.matrix.clone().multiply(startMatrix.clone().invert()); + if (delta.equals(new THREE.Matrix4())) return; + + const guid = this.findGeometryGuid(object); + if (!guid) return; + + // THREE.Matrix4.elements is column-major; transpose into a row-major 4x4 nested + // list, matching `compas.geometry.Transformation.from_matrix`'s expected shape. + const e = delta.elements; + const matrix = [ + [e[0], e[4], e[8], e[12]], + [e[1], e[5], e[9], e[13]], + [e[2], e[6], e[10], e[14]], + [e[3], e[7], e[11], e[15]], + ]; + this.sendData({ dispatch: "object_transform", guid, matrix }); + } + private handleKeyDown(event: KeyboardEvent): void { if (event.altKey || event.ctrlKey || event.metaKey) return; if (event.key === "Escape") { diff --git a/src/viewer/viewer_store.ts b/src/viewer/viewer_store.ts index bd1384d..a79c9d5 100644 --- a/src/viewer/viewer_store.ts +++ b/src/viewer/viewer_store.ts @@ -82,6 +82,7 @@ export interface ViewerStore { sidebarComponents: DynamicComponent[]; pickerEnabled: { value: boolean }; pickerMode: { value: "translate" | "rotate" | "scale" }; + pickedObjectGuid: { value: string | null }; blockPicker: { value: boolean }; showEdges: { value: boolean }; theme: { value: "light" | "dark" }; @@ -104,6 +105,7 @@ export function createViewerStore(): ViewerStore { sidebarComponents: reactive([]), pickerEnabled: reactive({ value: true }), pickerMode: reactive({ value: "translate" as const }), + pickedObjectGuid: reactive({ value: null as string | null }), blockPicker: reactive({ value: false }), showEdges: reactive({ value: false }), theme: reactive({ value: "light" as const }),