Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Box3#applyMatrix4() no longer supports projection transformation matrices #14655

Closed
6 tasks done
razh opened this issue Aug 7, 2018 · 3 comments
Closed
6 tasks done
Labels

Comments

@razh
Copy link
Contributor

razh commented Aug 7, 2018

Description of the problem

#13993 introduced an optimization to Box3#applyMatrix4() that only supports affine transforms.

screen shot 2018-08-06 at 9 24 21 pm

CodePen

  • geometry.applyMatrix() - The geometry is transformed with applyMatrix().
  • r93 Box3#applyMatrix4() & r92 Box3#applyMatrix4() - A Box3 instance is constructed from the geometry. A non-affine matrix is then applied.

For BoxGeometry, it is expected is that both operations result in equivalent Box3 instances.

The r92 version transformed each Box3 vertex, then used setFromPoints() to resize to fit.

See #6039 for a similar issue with a Vector3#applyMatrix4() optimization.

Three.js version
  • Dev
  • r95
  • r94
  • r93
Browser
  • All of them
OS
  • All of them
@WestLangley
Copy link
Collaborator

@razh Thank you for providing a well-written and clear demo.

I support reverting #13993.

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 7, 2018

/cc @mikialex

@mikialex
Copy link
Contributor

mikialex commented Aug 7, 2018

I haven't consider the case that matrix is not affined, for consistency it should be revert

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants