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

Add ability to set/get light "direction" explicitly #13373

Closed
3 tasks done
bhouston opened this issue Feb 20, 2018 · 37 comments
Closed
3 tasks done

Add ability to set/get light "direction" explicitly #13373

bhouston opened this issue Feb 20, 2018 · 37 comments

Comments

@bhouston
Copy link
Contributor

Description of the problem

I'd like to add to the Directional Light and Spot light the ability to set/get the light direction in an explicit fashion. Preferrably light.direction = ... or var direction = light.direction. Right now both the DirectionalLight and SpotLight are really hard to figure out where they are pointing, it isn't one line of code but a bunch. I would like to a generic accessor that hides this complexity.

Three.js version
  • Dev
Browser
  • All of them
OS
  • All of them
Hardware Requirements (graphics card, VR Device, ...)
@bhouston bhouston changed the title Add ability to set light "direction" explicitly Add ability to set/get light "direction" explicitly Feb 20, 2018
@WestLangley
Copy link
Collaborator

Right now both the DirectionalLight and SpotLight are really hard to figure out where they are pointing

They are pointing at light.target -- but you must add light.target to the scene, or call:

light.target.updateMatrixWorld();

If you want to be able to set the direction, then I expect we would have to remove light.target.

@looeee
Copy link
Collaborator

looeee commented Feb 21, 2018

I made a similar request a couple of months ago, that we switch to setting the direction of the light via the rotation / quaternion.

At the time I was told that the problem with this was that the light shadow requires a target to work, and that this is the reason the lights have targets.

If this is indeed the case, could we add onRotationChange type method to the lights that updates the target position when they are rotated, and possibly make the target invisible to the user?

@Usnul
Copy link
Contributor

Usnul commented Feb 28, 2018

@looee can you link the issue?

I have an adapter in my code specifically to address this issue as well.

@WestLangley
Copy link
Collaborator

WestLangley commented Feb 28, 2018

At the time I was told that the problem with this was that the light shadow requires a target to work, and that this is the reason the lights have targets.

Someone may have said that, but my recollection is the reason lights have targets is so the user can set the target to be an object in the scene, and the light will follow it. That is why the target is an Object3D, and not a Vector3.

@looeee
Copy link
Collaborator

looeee commented Feb 28, 2018

My apologies, it seems I remembered wrongly

#10220 (comment)

The comment from @WestLangley was that lights require a position for shadows, which makes more sense.

@Usnul
Copy link
Contributor

Usnul commented Feb 28, 2018

@WestLangley
Is light a meaningful primitive by itself in three.js? if so, it should be usable independently, and usable fully. Do you agree? As it stands - directional light can not be utilized fully without introducing a target.

Similar argument can (and was made by me at least once before) with respect to directional light's position - the fact that you "need" a position because we can't code up shadow camera's position without it is a fairly artificial limitation, I believe i'm not the only one with a wrapper in their code to lift these limitations.

I hope i don't come off too rude. I simply wish to understand where design/implementation boundary lies. If something is "so", does it imply "intended so" or not.

@WestLangley
Copy link
Collaborator

WestLangley commented Feb 28, 2018

@Usnul Sorry. I misread the comment, and since my reply was very misleading, I did the unthinkable and edited my comment after-the-fact.

@Usnul
Copy link
Contributor

Usnul commented Feb 28, 2018

@WestLangley How scandalous, you rascal!

but seriously, is this intended design or an accident/legacy

@WestLangley
Copy link
Collaborator

@Usnul It was an intended design for the reasons I stated. However, I do recall @mrdoob showing interest in removing target in the past.

@mrdoob mrdoob added this to the rXX milestone Mar 1, 2018
@mrdoob
Copy link
Owner

mrdoob commented Mar 1, 2018

Removing target is definitely something I'm interested in.

Not sure if direction make thing easier though. Couldn't one achieve the same thing with this?

var light = new THREE.DirectionalLight();
light.lookAt( new THREE.Vector3( 1, 1, 1 ) );

@Usnul
Copy link
Contributor

Usnul commented Mar 2, 2018 via email

@WestLangley
Copy link
Collaborator

@Usnul What specifically do you recommend as a new API for SpotLight and DirectionalLight?

@Usnul
Copy link
Contributor

Usnul commented Mar 2, 2018

Specifically, spot can work well with position and direction, directed light should ignore position attribute, instead relying entirely on orientation (rotation).

@Usnul
Copy link
Contributor

Usnul commented Mar 2, 2018

As a user of the API, i am not interested in how “shadow camera” is implemented, and the fact that internally there are clipping planes aligned on the vector of orientation for said camera. In most cases I would not bother setting position at all, letting it default to 0,0,0; I would only set the direction, using lookAt most likely

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 2, 2018

I like how Unity handles directional light. There is no target or direction vector, you just apply a rotation to change the direction of the light. When rotation is set to (0,0,0), the effective light direction is (0,0,1).

@WestLangley
Copy link
Collaborator

A Directional light is an Object3D, and consequently it has a position. You are suggesting we ignore the position and use rotation, instead.

rotation and quaternion are not appropriate for specifying a direction; they are only appropriate for specifying an orientation.

A 3D direction has two degrees of freedom ( heading and pitch, for example ).

An orientation has three degrees of freedom ( heading, pitch, and yaw ).

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 2, 2018

Well, my terminology could be better, yeah 😅

Okay, then you define an orientation and derive the direction vector from it. You know, i mean this:

lightDirection.set( 0, 0, 1 ).applyQuaternion( light.quaternion );

@Usnul
Copy link
Contributor

Usnul commented Mar 2, 2018

@WestLangley
Fully with you. You asked for specific suggestion; my suggestion acknowledges the convenience of Object3D and the attributes rotation and position. The fact that we have surplus information is tolerable to me

@WestLangley
Copy link
Collaborator

WestLangley commented Mar 2, 2018

@Mugen87 @Usnul But we have to support shadow frustums. They require a location, and they also (I think) should be right-side-up. That is, not tilted.

The current API supports what we need.

I am interested in your alternate APIs, but I have not heard anything yet detailed enough to be implementable. (At least I don't think I have. :) )

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 2, 2018

I thought we want to get rid of target, right? So i still don't understand what blocks the Unity approach. Instead of:

} else if ( light.isDirectionalLight ) {
var uniforms = cache.get( light );
uniforms.color.copy( light.color ).multiplyScalar( light.intensity );
uniforms.direction.setFromMatrixPosition( light.matrixWorld );
vector3.setFromMatrixPosition( light.target.matrixWorld );
uniforms.direction.sub( vector3 );
uniforms.direction.transformDirection( viewMatrix );

why not:

light.matrixWorld.decompose( position, quaternion, scale );
uniforms.direction.set( 0, 0, 1 ).applyQuaternion( quaternion );
uniforms.direction.transformDirection( viewMatrix );

Wouldn't something similar also work for the shadowCamera in WebGLShadowMap?

@WestLangley
Copy link
Collaborator

Fine, so far...

Wouldn't something similar also work for the shadowCamera in WebGLShadowMap?

You tell me... :)

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 2, 2018

......Maybe i should call Unity and ask how they implemented their directional light 😆.

@WestLangley
Copy link
Collaborator

I just do not see how this can be implemented without position.

Maybe light.direction and shadowCamera.position. That way, if there are no shadows, then a position is not required.

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 2, 2018

Oh, i've never suggested to neglect the position property. I just like to remove target...

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 2, 2018

Instead of

_lookTarget.setFromMatrixPosition( light.target.matrixWorld );
shadowCamera.lookAt( _lookTarget );
shadowCamera.updateMatrixWorld();

why not just

shadowCamera.matrixWorld.copy( light.matrixWorld );

@WestLangley
Copy link
Collaborator

WestLangley commented Mar 2, 2018

why not just

shadowCamera.matrixWorld.copy( light.matrixWorld );

Because that will cause the frustum to be rotated on its look-direction.

--

OK. So light.position and light.quaternion could work. But we need special logic to keep the shadow frustum right-side-up. (There are many quaternions that yield the same direction.) The orientation of the frustum will also be an issue if we implement projective texturing.

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 3, 2018

I'm stuck here, sorry. I don't understand "Because that will cause the frustum to be rotated on its look-direction.". I think i have to make the change and then visualize the shadow camera to see the difference.

@WestLangley
Copy link
Collaborator

WestLangley commented Mar 3, 2018

What I was thinking was... If you take an arbitrary transformation matrix, it is associated with a particular quaternion. A quaternion applied to any object, such as a frustum shape, will change its "look direction", but will not necessarily keep it "right side up".

That is why we use object.lookAt( target ). It uses the direction to the target plus the object's up vector to orient the object.

@Usnul
Copy link
Contributor

Usnul commented Mar 3, 2018

Alright, all this talk of APIs got me confused. I get it now. I think :)
In concrete terms, I don’t think that position is needed, indeed I believe it to be harmful. Why? Because you can find best near and far clipping planes without it. Position of the camera is also trivial by extension. How do you find near and far planes? Simple - analyse your scene and determine the extents of it along the light’s direction. It’s quite cheap too, if you use a spatial index. That’s what I have implemented, now I don’t need to think about shadow camera at all, it just works, and it produces the best possible results.

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 4, 2018

@WestLangley Maybe we can do this instead of using a predefined target vector:

_lookTarget.copy( _lightPositionWorld ).add( _lightDirectionWorld );
shadowCamera.lookAt( _lookTarget );

@WestLangley
Copy link
Collaborator

@Usnul Some users have a tight shadow frustum focused on just part of the scene, in which case location information is required -- either in the form of a target position, light position, or shadow camera position.

@Mugen87 If you want, you can try hacking something up for directional light to see if you can replace target with quaternion or direction. Lights can be children of other (rotated) objects, so you need take that into consideration. Lights also have helpers and shadow frustums that need to be properly positioned and oriented.

@Usnul
Copy link
Contributor

Usnul commented Mar 5, 2018

@WestLangley
That is true. And i acknowledge that. However, this moves usability further and further into domain of experts. What I propose is a solution which requires no tweaking on the user's part. Set the desired direction and the engine takes care of the rest.

I genuinely am curious, do you believe that shadows being generated automatically to fit the scene to be an inferior design?

Let's say I create a directed light and move it about the scene - just because I can. I changed the "position" (whatever that means for a directed light), but why does the orientation of the light changes? Say i have a pencil in the scene, and I light it up directly from the tip along the length, and I adjust my shadow camera to only fit that small circular shadow. Now I change the pencil's position, light changes rotation, shadow camera now clips the shadow which is no longer a small circle.

I just don't get it, ...I'm sorry for being so thick?

@WestLangley
Copy link
Collaborator

Shadow quality degrades as the shadow frustum increases in size. One option is to bake shadows for static elements. The shadow camera frustum can then be tight, and track only the dynamic object(s).

@looeee
Copy link
Collaborator

looeee commented Jul 26, 2018

Adding this from #14557 since this is the main thread. Three.js is the outlier regarding having a targeted directional light. Here's a comparison to some other apps / frameworks:

My opinion is that when basically every other app or framework has basic functionality like this, we should aim to support it as well, and my preference would be to add a second directional light with a name such as FreeDirectionalLight. The direction can be set via the .rotation, while the .position can be used for the shadow frustum.

@donmccurdy
Copy link
Collaborator

The direction can be set via the .rotation, while the .position can be used for the shadow frustum.

Nit on terminology — direction is a fixed vector, e.g. always +Z, but can be transformed by the node's rotation.

FWIW we are accomplishing this in GLTFLoader with:

lightNode = new THREE.DirectionalLight( color );
lightNode.target.position.set( 0, 0, 1 );
lightNode.add( lightNode.target );

@looeee
Copy link
Collaborator

looeee commented Jul 26, 2018

direction is a fixed vector

I meant "direction of the light rays" - as in the end result of any transformations or calculations that are done.

@looeee
Copy link
Collaborator

looeee commented Jul 26, 2018

lightNode = new THREE.DirectionalLight( color );
lightNode.target.position.set( 0, 0, 1 );
lightNode.add( lightNode.target );

That's actually a really neat workaround 😁

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

No branches or pull requests

7 participants