Skip to content

Fix modeltransform#3538

Open
sankhesh wants to merge 4 commits into
Kitware:masterfrom
sankhesh:fix-modeltransform
Open

Fix modeltransform#3538
sankhesh wants to merge 4 commits into
Kitware:masterfrom
sankhesh:fix-modeltransform

Conversation

@sankhesh

Copy link
Copy Markdown
Collaborator

Context

When applying the model transform matrix, it was observed that the matrix was being applied in the view space leading to unintended orientation specific artifacts. This change fixes the order of matrix multiplication for the transform and adds testing to ensure that the transform is applied camera-orientation-agnostically in the world space.

Results

Changes

  • Documentation and TypeScript definitions were updated to match those changes

PR and Code Checklist

  • semantic-release commit messages
  • Run npm run reformat to have correctly formatted code

Testing

  • This change adds or fixes unit tests
  • Tested environment:
    • vtk.js:
    • OS:
    • Browser:

The model transform matrix was being post-multiplied causing the
transform to be applied in in camera space rather than world space. This
change fixes making the transform orientation-invariant regardless of
camera rotation.

@finetjul finetjul left a comment

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.

Why setModelTransformMatrix and getModelTransformMatrix are not generated with macro.setGet ?

Comment thread Sources/Rendering/Core/Camera/index.js Outdated
@sankhesh sankhesh force-pushed the fix-modeltransform branch from 1a92c52 to 0e9ff2a Compare June 26, 2026 16:50

@finetjul finetjul left a comment

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.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants