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

DirectionalLightHelper breaks when pointing down the Object3D.DefaultUp axis. #11436

Closed
3 of 13 tasks
manthrax opened this issue Jun 3, 2017 · 8 comments
Closed
3 of 13 tasks

Comments

@manthrax
Copy link
Contributor

manthrax commented Jun 3, 2017

Description of the problem

DirectionalLightHelper breaks when pointing down the DefaultUp axis.

This is related to:
#11241

I made a JSFiddle showing some of this new counterintuitive behavior, and also what seems like it might be a bug in DirectionalLightHelper as well...

First I set Object3D.DefaultUp.set(0,0,1)

Then I make a DirectionalLight, and a DirectionalLightHelper moving along the X axis, to eventually line up looking down the Z (DefaultUp) axis.

At the point that it is looking exactly down that axis, the DirectionalLightHelper breaks, and its values are full of NaNs...

Additionally when the NaN problem kicks in, I check for it, and flash the cube...

Let me know if any of this is unclear... of if I'm just using DirectionalLightHelper wrong?

Additionally, the DirectionalLightHelper, doesn't seem to be aiming down the correct axis that the light is pointing along... I animate a cube at the halfway point between light and 0,0,0, to illustrate that point...

http://jsfiddle.net/5wgjvLnz/19/

Three.js version
  • Dev
  • r85
  • ...
Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
OS
  • All of them
  • Windows
  • macOS
  • Linux
  • Android
  • iOS
Hardware Requirements (graphics card, VR Device, ...)
@WestLangley
Copy link
Collaborator

WestLangley commented Jun 4, 2017

Please try with this change to Matrix4.lookAt():

// eye and target are in the same vertical

if ( Math.abs( up.z ) === 1 ) {

	z.x += 0.0001;

} else {

	z.z += 0.0001;

}

@manthrax
Copy link
Contributor Author

manthrax commented Jun 4, 2017

Yep that fixes it. This also sorta nails down another implication of DefaultUp, namely that it can't be an arbitrary vector.. it needs to be x, y, or z exclusively.. in which case it almost seems to make more sense that it just be declared as an axis.. like DefaultUp = "z".

Thanks for checking it out @WestLangley.

@WestLangley
Copy link
Collaborator

This also sorta nails down another implication of DefaultUp, namely that it can't be an arbitrary vector

Why do you say that?

@mrdoob
Copy link
Owner

mrdoob commented Jun 13, 2017

Additionally, the DirectionalLightHelper, doesn't seem to be aiming down the correct axis that the light is pointing along...

Commenting this line fixes that:

light.lookAt(light.target.position);

http://jsfiddle.net/5wgjvLnz/20/

@manthrax
Copy link
Contributor Author

manthrax commented Jun 14, 2017

@WestLangley If up.z = 1.01, would that check fail, and still yield a zero length cross product?

@mrdoob yeah. it fixes it, but doesn't explain why making a directionalLight lookAt something should break the helper. I think what you're telling me is that lookAt on a directionalLight is not intended to be used to aim the light.. which I get. Additionally, a side effect of that incorrect usage of lookAt is that any attached directionalLightHelper will be affected by the lookAt...

For instance, light.lookAt(new THREE.Vector3(0,0,0)); shows the same behavior. Like.. I wonder if lookAt on a directionLight should throw a warning or something?

@WestLangley
Copy link
Collaborator

up.z = 1.01

up must be unit length.

but doesn't explain why making a directionalLight lookAt something should break the helper.

Yes it does. The helper relies on the world matrix of the light.

@manthrax
Copy link
Contributor Author

manthrax commented Jun 17, 2017

Right. So the helper is broken for cases where one has modified the directional lights transform?

@WestLangley
Copy link
Collaborator

It does not make sense to modify the directional light's transform. Hence, I would not consider the helper as broken.

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

No branches or pull requests

3 participants