Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions Sources/Rendering/Core/Prop3D/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you would need to expose it publicly because anyone can "getXXXByReference" (or getUserMatrix) and modify the fields manually.
If they do so, they must call transformsModified().

Please document getUserMatrix() accordingly

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 = () => {
Expand All @@ -42,7 +54,7 @@ function vtkProp3D(publicAPI, model) {
model.rotation,
vtkMath.radiansFromDegrees(val)
);
publicAPI.modified();
transformsModified();
};

publicAPI.rotateY = (val) => {
Expand All @@ -54,7 +66,7 @@ function vtkProp3D(publicAPI, model) {
model.rotation,
vtkMath.radiansFromDegrees(val)
);
publicAPI.modified();
transformsModified();
};

publicAPI.rotateZ = (val) => {
Expand All @@ -66,7 +78,7 @@ function vtkProp3D(publicAPI, model) {
model.rotation,
vtkMath.radiansFromDegrees(val)
);
publicAPI.modified();
transformsModified();
};

publicAPI.rotateWXYZ = (degrees, x, y, z) => {
Expand All @@ -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) => {
Expand All @@ -96,7 +108,7 @@ function vtkProp3D(publicAPI, model) {
orientationQuaternion
);
mat4.multiply(model.rotation, model.rotation, oriQuatMat);
publicAPI.modified();
transformsModified();
};

publicAPI.setOrientation = (x, y, z) => {
Expand All @@ -112,7 +124,7 @@ function vtkProp3D(publicAPI, model) {
publicAPI.rotateZ(z);
publicAPI.rotateX(x);
publicAPI.rotateY(y);
publicAPI.modified();
transformsModified();
return true;
};

Expand All @@ -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;
Expand All @@ -132,7 +144,7 @@ function vtkProp3D(publicAPI, model) {
return false;
}
mat4.copy(model.userMatrix, matrix);
publicAPI.modified();
transformsModified();
return true;
};

Expand All @@ -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);
Expand Down Expand Up @@ -303,6 +315,7 @@ const DEFAULT_VALUES = {
cachedProp3D: null,
isIdentity: true,
matrixMTime: null,
transformsMTime: null,
};

// ----------------------------------------------------------------------------
Expand All @@ -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']);
Expand Down
60 changes: 60 additions & 0 deletions Sources/Rendering/Core/Prop3D/test/testComputeMatrix.js
Original file line number Diff line number Diff line change
@@ -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);
});