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

Support reflections in world transform #12787

Merged
merged 1 commit into from
Dec 2, 2017

Conversation

WestLangley
Copy link
Collaborator

As proposed, and discussed, in #12720.

@mrdoob
Copy link
Owner

mrdoob commented Dec 2, 2017

Sweet!

@mrdoob mrdoob merged commit 01111ad into mrdoob:dev Dec 2, 2017
@mrdoob
Copy link
Owner

mrdoob commented Dec 2, 2017

Thanks!

@looeee
Copy link
Collaborator

looeee commented Dec 6, 2017

I've tested this with a couple of models that were showing problems and it works great! Thanks @WestLangley 😄

@vincentfretin
Copy link
Contributor

This change write lots of warnings, see #13188

@yiakwy
Copy link

yiakwy commented Jan 29, 2018

@mrdoob , @looeee, @WestLangley . I think at least we need to check the determinant values and matrix type before further processing

getNormalMatrix: function ( matrix4 ) {
...
...
  // verify the dimension of matrix4
  if (matrixl4.length !== 16) {
     throw Error("Wrong Dimension of Matrix!")
   }

   return this.setFromMatrix4( matrix4 ).getInverse( this ).transpose();

},

setFromMatrix4 will return a matrix of type Matrix3

	setFromMatrix4: function ( m ) {

		var me = m.elements;

		this.set(

			me[ 0 ], me[ 4 ], me[ 8 ],
			me[ 1 ], me[ 5 ], me[ 9 ],
			me[ 2 ], me[ 6 ], me[ 10 ]

		);

		return this;

	},

The line 3400 in three.module.js getInverse should be at least replaced by

Math.abs(det) < +(1+"e-" + places) // a common numeric trick in js or es6

console.warning(msg) => log.warning(msg) // when log level lower than warning, mute it.

The original code in getinverse

		if ( det === 0 ) {

			var msg = "THREE.Matrix3: .getInverse() can't invert matrix, determinant is 0";

			if ( throwOnDegenerate === true ) {

				throw new Error( msg );

			} else {
				log.warn( msg ); // when log level lower than warning, mute it.

			}

			return this.identity();

		}

is numerically wrong. The precision call also be Math.EPILON which should have been set by polyfills.js

@WestLangley
Copy link
Collaborator Author

@yiakwy If you believe there is a three.js bug, please open a new ticket and include a live example demonstrating the issue.

We do not verify or validate function args in three.js.

Also, line numbers change with each build. Avoid referencing build file line numbers in your post.

Thanks.

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.

6 participants