From 84ded60a1c8e6fb8b0c0bb8bd1bdf14536fc6ec6 Mon Sep 17 00:00:00 2001 From: Christopher Pruijsen Date: Wed, 14 Oct 2026 18:01:12 +0100 Subject: [PATCH 1/2] perf(Prop3D): only recompute the matrix when the transform changes computeMatrix() used the prop's global mtime to decide whether to rebuild the matrix, so any modified() call - for example setVisibility() - made it stale and forced a recompute, both eagerly through the onModified subscription and lazily on the next access. Track the modification time of the transform inputs (origin, position, scale, rotation, userMatrix) in a dedicated transformsMTime and use it as the staleness check. The onModified subscription is kept so isIdentity stays up to date when a transform actually changes. fix #3378 --- Sources/Rendering/Core/Prop3D/index.js | 35 +++++++---- .../Core/Prop3D/test/testComputeMatrix.js | 60 +++++++++++++++++++ 2 files changed, 85 insertions(+), 10 deletions(-) create mode 100644 Sources/Rendering/Core/Prop3D/test/testComputeMatrix.js diff --git a/Sources/Rendering/Core/Prop3D/index.js b/Sources/Rendering/Core/Prop3D/index.js index 841c231510a..6294f3c8ba3 100644 --- a/Sources/Rendering/Core/Prop3D/index.js +++ b/Sources/Rendering/Core/Prop3D/index.js @@ -15,11 +15,23 @@ function vtkProp3D(publicAPI, model) { // Set our className model.classHierarchy.push('vtkProp3D'); + // Track the modification time of the transform inputs (origin, position, + // scale, rotation, userMatrix) separately so computeMatrix() does not run + // on every modified() event, e.g. when only the visibility changes. + // The _onXXXChanged hooks run before the setter's own modified() call. + function transformsModified() { + model.transformsMTime.modified(); + publicAPI.modified(); + } + model._onOriginChanged = () => model.transformsMTime.modified(); + model._onPositionChanged = () => model.transformsMTime.modified(); + model._onScaleChanged = () => model.transformsMTime.modified(); + publicAPI.addPosition = (deltaXYZ) => { model.position = model.position.map( (value, index) => value + deltaXYZ[index] ); - publicAPI.modified(); + transformsModified(); }; publicAPI.getOrientationWXYZ = () => { @@ -42,7 +54,7 @@ function vtkProp3D(publicAPI, model) { model.rotation, vtkMath.radiansFromDegrees(val) ); - publicAPI.modified(); + transformsModified(); }; publicAPI.rotateY = (val) => { @@ -54,7 +66,7 @@ function vtkProp3D(publicAPI, model) { model.rotation, vtkMath.radiansFromDegrees(val) ); - publicAPI.modified(); + transformsModified(); }; publicAPI.rotateZ = (val) => { @@ -66,7 +78,7 @@ function vtkProp3D(publicAPI, model) { model.rotation, vtkMath.radiansFromDegrees(val) ); - publicAPI.modified(); + transformsModified(); }; publicAPI.rotateWXYZ = (degrees, x, y, z) => { @@ -83,7 +95,7 @@ function vtkProp3D(publicAPI, model) { const quatMat = new Float64Array(16); mat4.fromQuat(quatMat, q); mat4.multiply(model.rotation, model.rotation, quatMat); - publicAPI.modified(); + transformsModified(); }; publicAPI.rotateQuaternion = (orientationQuaternion) => { @@ -96,7 +108,7 @@ function vtkProp3D(publicAPI, model) { orientationQuaternion ); mat4.multiply(model.rotation, model.rotation, oriQuatMat); - publicAPI.modified(); + transformsModified(); }; publicAPI.setOrientation = (x, y, z) => { @@ -112,7 +124,7 @@ function vtkProp3D(publicAPI, model) { publicAPI.rotateZ(z); publicAPI.rotateX(x); publicAPI.rotateY(y); - publicAPI.modified(); + transformsModified(); return true; }; @@ -121,7 +133,7 @@ function vtkProp3D(publicAPI, model) { mat4.fromQuat(rotation, q); if (!vtkMath.areMatricesEqual(rotation, model.rotation)) { model.rotation = rotation; - publicAPI.modified(); + transformsModified(); return true; } return false; @@ -132,7 +144,7 @@ function vtkProp3D(publicAPI, model) { return false; } mat4.copy(model.userMatrix, matrix); - publicAPI.modified(); + transformsModified(); return true; }; @@ -143,7 +155,7 @@ function vtkProp3D(publicAPI, model) { publicAPI.computeMatrix = () => { // check whether or not need to rebuild the matrix - if (publicAPI.getMTime() > model.matrixMTime.getMTime()) { + if (model.transformsMTime.getMTime() > model.matrixMTime.getMTime()) { mat4.identity(model.matrix); if (model.userMatrix) { mat4.multiply(model.matrix, model.matrix, model.userMatrix); @@ -303,6 +315,7 @@ const DEFAULT_VALUES = { cachedProp3D: null, isIdentity: true, matrixMTime: null, + transformsMTime: null, }; // ---------------------------------------------------------------------------- @@ -315,6 +328,8 @@ export function extend(publicAPI, model, initialValues = {}) { model.matrixMTime = {}; macro.obj(model.matrixMTime); + model.transformsMTime = {}; + macro.obj(model.transformsMTime); // Build VTK API macro.get(publicAPI, model, ['isIdentity']); diff --git a/Sources/Rendering/Core/Prop3D/test/testComputeMatrix.js b/Sources/Rendering/Core/Prop3D/test/testComputeMatrix.js new file mode 100644 index 00000000000..10e67a12330 --- /dev/null +++ b/Sources/Rendering/Core/Prop3D/test/testComputeMatrix.js @@ -0,0 +1,60 @@ +import { it, expect } from 'vitest'; + +import vtkActor from 'vtk.js/Sources/Rendering/Core/Actor'; + +// The actor matrix only depends on the transform state (position, origin, +// scale, orientation, userMatrix). Recomputing it on any other modification +// (e.g. setVisibility) is wasted work. +it('Test computeMatrix only runs on transform change', () => { + const actor = vtkActor.newInstance(); + const getMatrixMTime = () => + actor.getReferenceByName('matrixMTime').getMTime(); + + // Build the matrix once so it is up to date + actor.getMatrix(); + let mtime = getMatrixMTime(); + + // Modifications unrelated to the transform must not trigger a matrix + // recomputation, neither eagerly through onModified nor lazily through + // getMatrix()/computeMatrix(). + actor.setVisibility(false); + actor.setPickable(false); + actor.getProperty().setOpacity(0.5); + actor.getMatrix(); + actor.computeMatrix(); + expect(getMatrixMTime()).toBe(mtime); + + // Transform changes must still recompute the matrix + actor.setPosition(1, 2, 3); + expect(getMatrixMTime()).toBeGreaterThan(mtime); + expect(actor.getIsIdentity()).toBe(false); + const matrix = actor.getMatrix(); + expect(matrix[3]).toBe(1); + expect(matrix[7]).toBe(2); + expect(matrix[11]).toBe(3); + + mtime = getMatrixMTime(); + actor.setOrigin(1, 0, 0); + expect(getMatrixMTime()).toBeGreaterThan(mtime); + + mtime = getMatrixMTime(); + actor.setScale(2, 2, 2); + expect(getMatrixMTime()).toBeGreaterThan(mtime); + + mtime = getMatrixMTime(); + actor.rotateZ(45); + expect(getMatrixMTime()).toBeGreaterThan(mtime); + expect(actor.getIsIdentity()).toBe(false); + + // Resetting the orientation to zero must reset the rotation even though + // rotateX/Y/Z early-return on a null angle. + actor.setPosition(0, 0, 0); + actor.setOrigin(0, 0, 0); + actor.setScale(1, 1, 1); + actor.setOrientation(30, 0, 0); + expect(actor.getIsIdentity()).toBe(false); + mtime = getMatrixMTime(); + actor.setOrientation(0, 0, 0); + expect(getMatrixMTime()).toBeGreaterThan(mtime); + expect(actor.getIsIdentity()).toBe(true); +}); From 17466daa54fb41cf1ba2bfaf2dc689b50094dd0a Mon Sep 17 00:00:00 2001 From: Christopher Pruijsen Date: Tue, 20 Oct 2026 10:20:35 +0100 Subject: [PATCH 2/2] fix(Prop3D): expose transformsModified() for in-place transform edits --- Sources/Rendering/Core/Prop3D/index.d.ts | 10 ++++++++++ Sources/Rendering/Core/Prop3D/index.js | 2 +- .../Rendering/Core/Prop3D/test/testComputeMatrix.js | 5 +++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Sources/Rendering/Core/Prop3D/index.d.ts b/Sources/Rendering/Core/Prop3D/index.d.ts index 488378db565..e538c3a4e20 100755 --- a/Sources/Rendering/Core/Prop3D/index.d.ts +++ b/Sources/Rendering/Core/Prop3D/index.d.ts @@ -135,6 +135,8 @@ export interface vtkProp3D extends vtkProp { /** * Get the transformation matrix set for your own use. + * The matrix is returned by reference: if you modify it in place, call + * transformsModified() so the actor matrix gets recomputed. */ getUserMatrix(): mat4; @@ -279,6 +281,14 @@ export interface vtkProp3D extends vtkProp { */ computeMatrix(): void; + /** + * Mark the transform (origin, position, scale, rotation, userMatrix) as + * modified. Call it after changing one of these in place, e.g. through + * getUserMatrix() or getReferenceByName(), so computeMatrix() rebuilds the + * matrix. + */ + transformsModified(): void; + /** * Set the actor property for the specified mapper input port, which defaults to 0 * @param {vtkObject} property diff --git a/Sources/Rendering/Core/Prop3D/index.js b/Sources/Rendering/Core/Prop3D/index.js index 6294f3c8ba3..a83dbc066fe 100644 --- a/Sources/Rendering/Core/Prop3D/index.js +++ b/Sources/Rendering/Core/Prop3D/index.js @@ -23,6 +23,7 @@ function vtkProp3D(publicAPI, model) { model.transformsMTime.modified(); publicAPI.modified(); } + publicAPI.transformsModified = transformsModified; model._onOriginChanged = () => model.transformsMTime.modified(); model._onPositionChanged = () => model.transformsMTime.modified(); model._onScaleChanged = () => model.transformsMTime.modified(); @@ -310,7 +311,6 @@ const DEFAULT_VALUES = { properties: [], userMatrix: null, - userMatrixMTime: null, cachedProp3D: null, isIdentity: true, diff --git a/Sources/Rendering/Core/Prop3D/test/testComputeMatrix.js b/Sources/Rendering/Core/Prop3D/test/testComputeMatrix.js index 10e67a12330..59063d87534 100644 --- a/Sources/Rendering/Core/Prop3D/test/testComputeMatrix.js +++ b/Sources/Rendering/Core/Prop3D/test/testComputeMatrix.js @@ -57,4 +57,9 @@ it('Test computeMatrix only runs on transform change', () => { actor.setOrientation(0, 0, 0); expect(getMatrixMTime()).toBeGreaterThan(mtime); expect(actor.getIsIdentity()).toBe(true); + + // In-place edits of the user matrix need an explicit transformsModified() + actor.getUserMatrix()[12] = 5; + actor.transformsModified(); + expect(actor.getMatrix()[3]).toBe(5); });