From 61f4232a532ec60fe4bebcb45c84e3e020a396b2 Mon Sep 17 00:00:00 2001 From: KPal Date: Mon, 19 Oct 2026 14:30:25 +0100 Subject: [PATCH 1/2] fix: Gizmo shared layers, enabled state, theme hover, multi-touch, arc mesh leak, uniform plane scaling and picking order --- src/extras/gizmo/gizmo.js | 91 ++++++++++++++++++++--------- src/extras/gizmo/scale-gizmo.js | 2 +- src/extras/gizmo/shape/arc-shape.js | 15 +++-- src/extras/gizmo/transform-gizmo.js | 2 +- 4 files changed, 76 insertions(+), 34 deletions(-) diff --git a/src/extras/gizmo/gizmo.js b/src/extras/gizmo/gizmo.js index 275058aa1ef..0d8d3b76e78 100644 --- a/src/extras/gizmo/gizmo.js +++ b/src/extras/gizmo/gizmo.js @@ -240,6 +240,20 @@ class Gizmo extends EventHandler { */ _renderUpdate = false; + /** + * Internal enabled state requested by the user. + * + * @private + */ + _enabled = true; + + /** + * Internal id of the pointer interacting with the gizmo. + * + * @private + */ + _pointerId = -1; + /** * The graph nodes attached to the gizmo. * @@ -289,9 +303,9 @@ class Gizmo extends EventHandler { * Creates a new Gizmo object. * * @param {CameraComponent} camera - The camera component. - * @param {Layer} layer - The render layer. This can be provided by the user or will be created - * and added to the scene and camera if not provided. Successive gizmos will share the same layer - * and will be removed from the camera and scene when the last gizmo is destroyed. + * @param {Layer} layer - The render layer. The layer is added to the camera, and gizmos can share + * the same layer. It is removed from the camera when the last gizmo using it on that camera is + * destroyed or moved to another camera. * @param {string} [name] - The name of the gizmo. Defaults to 'gizmo'. * @example * const gizmo = new Gizmo(camera, layer); @@ -331,6 +345,7 @@ class Gizmo extends EventHandler { * @type {boolean} */ set enabled(state) { + this._enabled = state; const cameraDist = this.root.getLocalPosition().distance(this.camera.entity.getPosition()); const enabled = state ? this.nodes.length > 0 && cameraDist > DIST_EPSILON : false; if (enabled !== this.root.enabled) { @@ -373,11 +388,11 @@ class Gizmo extends EventHandler { if (this._layer === layer) { return; } - this._camera.layers = this._camera.layers.filter(id => id !== this._layer.id); + this._releaseCameraLayer(); this._layer = layer; this._camera.layers = this._camera.layers.concat(this._layer.id); - this.enabled = true; + this.enabled = this._enabled; } /** @@ -398,11 +413,11 @@ class Gizmo extends EventHandler { if (this._camera === camera) { return; } - this._camera.layers = this._camera.layers.filter(id => id !== this._layer.id); + this._releaseCameraLayer(); this._camera = camera; this._camera.layers = this._camera.layers.concat(this._layer.id); - this.enabled = true; + this.enabled = this._enabled; } /** @@ -486,6 +501,12 @@ class Gizmo extends EventHandler { if (!this.mouseButtons[e.button]) { return; } + + // ignore other pointers while one is pressed + const { canvas } = this._device; + if (e.pointerId !== this._pointerId && canvas.hasPointerCapture(this._pointerId)) { + return; + } const selection = this._getSelection(e.offsetX, e.offsetY); if (selection[0]) { if (this.preventDefault) { @@ -495,7 +516,7 @@ class Gizmo extends EventHandler { } // capture the pointer during drag - const { canvas } = this._device; + this._pointerId = e.pointerId; canvas.setPointerCapture(e.pointerId); this.fire(Gizmo.EVENT_POINTERDOWN, e.offsetX, e.offsetY, selection[0]); @@ -509,6 +530,9 @@ class Gizmo extends EventHandler { if (!this.enabled || document.pointerLockElement) { return; } + if (e.pointerId !== this._pointerId && this._device.canvas.hasPointerCapture(this._pointerId)) { + return; + } const selection = this._getSelection(e.offsetX, e.offsetY); if (selection[0]) { if (this.preventDefault) { @@ -524,7 +548,7 @@ class Gizmo extends EventHandler { * @private */ _onPointerUp(e) { - if (!this.mouseButtons[e.button]) { + if (!this.mouseButtons[e.button] || e.pointerId !== this._pointerId) { return; } @@ -544,6 +568,21 @@ class Gizmo extends EventHandler { this.fire(Gizmo.EVENT_POINTERUP, e.offsetX, e.offsetY, selection[0]); } + /** + * Removes a single reference to the gizmo layer from the camera, as other gizmos may share the + * layer with the same camera. + * + * @private + */ + _releaseCameraLayer() { + const ids = this._camera.layers.slice(); + const index = ids.indexOf(this._layer.id); + if (index !== -1) { + ids.splice(index, 1); + } + this._camera.layers = ids; + } + /** @protected */ _updatePosition() { position.set(0, 0, 0); @@ -617,7 +656,10 @@ class Gizmo extends EventHandler { const end = this._camera.screenToWorld(x, y, this._camera.farClip - this._camera.nearClip); const dir = v.copy(end).sub(start).normalize(); - const selection = []; + /** @type {MeshInstance[] | null} */ + let best = null; + let bestDist = 0; + let bestPriority = 0; for (let i = 0; i < this.intersectShapes.length; i++) { const shape = this.intersectShapes[i]; if (shape.disabled || !shape.entity.enabled) { @@ -638,27 +680,22 @@ class Gizmo extends EventHandler { for (let k = 0; k < tris.length; k++) { if (tris[k].intersectsRay(ray, hit)) { - selection.push({ - dist: triWTM.transformPoint(hit).sub(start).length(), - meshInstances: shape.meshInstances, - priority: priority - }); + const dist = triWTM.transformPoint(hit).sub(start).length(); + + // priority decides between prioritized hits, otherwise distance + const wins = !best || (bestPriority !== 0 && priority !== 0 ? + priority > bestPriority : dist < bestDist); + if (wins) { + best = shape.meshInstances; + bestDist = dist; + bestPriority = priority; + } } } } } - if (selection.length) { - selection.sort((s0, s1) => { - if (s0.priority !== 0 && s1.priority !== 0) { - return s1.priority - s0.priority; - } - return s0.dist - s1.dist; - }); - return selection[0].meshInstances; - } - - return []; + return best ?? []; } /** @@ -749,6 +786,8 @@ class Gizmo extends EventHandler { destroy() { this.detach(); + this._releaseCameraLayer(); + this._device.canvas.removeEventListener('pointerdown', this._onPointerDown); this._device.canvas.removeEventListener('pointermove', this._onPointerMove); this._device.canvas.removeEventListener('pointerup', this._onPointerUp); diff --git a/src/extras/gizmo/scale-gizmo.js b/src/extras/gizmo/scale-gizmo.js index f0a43f50d6e..ed6dab815c2 100644 --- a/src/extras/gizmo/scale-gizmo.js +++ b/src/extras/gizmo/scale-gizmo.js @@ -577,7 +577,7 @@ class ScaleGizmo extends TransformGizmo { // project to diagonal line v1.set(1, 1, 1); v1[axis] = 0; - point.copy(v1.mulScalar(v1.dot(point))); + point.copy(v1.mulScalar(v1.dot(point) / v1.lengthSq())); point[axis] = 0; } diff --git a/src/extras/gizmo/shape/arc-shape.js b/src/extras/gizmo/shape/arc-shape.js index 7220f0319d6..f61c903caeb 100644 --- a/src/extras/gizmo/shape/arc-shape.js +++ b/src/extras/gizmo/shape/arc-shape.js @@ -78,9 +78,6 @@ class ArcShape extends Shape { this._createTorusMesh(360) ]); this.show('sector'); - - // update transform - this._update(); } /** @@ -184,9 +181,15 @@ class ArcShape extends Shape { this._triDataCache[0].fromGeometry(this._createTorusGeometry(this._sectorAngle)); this._triDataCache[1].fromGeometry(this._createTorusGeometry(360)); - // render - this.meshInstances[0].mesh = this._createTorusMesh(this._sectorAngle); - this.meshInstances[1].mesh = this._createTorusMesh(360); + // render, destroying the replaced meshes + const angles = [this._sectorAngle, 360]; + for (let i = 0; i < angles.length; i++) { + const mesh = this.meshInstances[i].mesh; + this.meshInstances[i].mesh = this._createTorusMesh(angles[i]); + if (mesh && mesh.refCount < 1) { + mesh.destroy(); + } + } } /** diff --git a/src/extras/gizmo/transform-gizmo.js b/src/extras/gizmo/transform-gizmo.js index b2dca8055c4..e801c6e59c5 100644 --- a/src/extras/gizmo/transform-gizmo.js +++ b/src/extras/gizmo/transform-gizmo.js @@ -768,7 +768,7 @@ class TransformGizmo extends Gizmo { // update shapes for (const name in this._shapes) { - this._shapes[name].hover(!!this._hoverAxis); + this._shapes[name].hover(this._hovering.has(/** @type {GizmoAxis} */ (name))); } } From 796d7b36d537f3cc1d1f03c8797c7d6434d38d2f Mon Sep 17 00:00:00 2001 From: KPal Date: Mon, 19 Oct 2026 14:48:05 +0100 Subject: [PATCH 2/2] fix: Gizmo layer setter moves meshes and destroy releases the layer once --- src/extras/gizmo/gizmo.js | 22 ++++++++++++++++++++++ src/extras/gizmo/rotate-gizmo.js | 27 +++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/extras/gizmo/gizmo.js b/src/extras/gizmo/gizmo.js index 0d8d3b76e78..745c4b2fc46 100644 --- a/src/extras/gizmo/gizmo.js +++ b/src/extras/gizmo/gizmo.js @@ -254,6 +254,13 @@ class Gizmo extends EventHandler { */ _pointerId = -1; + /** + * Internal flag to track if the gizmo has been destroyed. + * + * @private + */ + _destroyed = false; + /** * The graph nodes attached to the gizmo. * @@ -392,6 +399,15 @@ class Gizmo extends EventHandler { this._layer = layer; this._camera.layers = this._camera.layers.concat(this._layer.id); + // move the gizmo meshes to the new layer + const ids = [layer.id]; + this.root.forEach((node) => { + const { render } = /** @type {Entity} */ (node); + if (render) { + render.layers = ids; + } + }); + this.enabled = this._enabled; } @@ -784,6 +800,12 @@ class Gizmo extends EventHandler { * gizmo.destroy(); */ destroy() { + // destroy can be called again, e.g. by the app destroy event after the user + if (this._destroyed) { + return; + } + this._destroyed = true; + this.detach(); this._releaseCameraLayer(); diff --git a/src/extras/gizmo/rotate-gizmo.js b/src/extras/gizmo/rotate-gizmo.js index c00d40e0cf4..811214d8dd4 100644 --- a/src/extras/gizmo/rotate-gizmo.js +++ b/src/extras/gizmo/rotate-gizmo.js @@ -430,6 +430,33 @@ class RotateGizmo extends TransformGizmo { return this._guideAngleLines[0].thickness; } + /** + * Sets the gizmo render layer. + * + * @type {Layer} + */ + set layer(layer) { + super.layer = layer; + + // the guide lines are not children of the gizmo root + const ids = [layer.id]; + for (let i = 0; i < this._guideAngleLines.length; i++) { + const { render } = this._guideAngleLines[i].entity; + if (render) { + render.layers = ids; + } + } + } + + /** + * Gets the gizmo render layer. + * + * @type {Layer} + */ + get layer() { + return super.layer; + } + /** * @type {boolean} * @deprecated Use {@link rotationMode} instead.