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

Improved SpotLightHelper #3470

Merged
merged 1 commit into from
May 13, 2013
Merged

Improved SpotLightHelper #3470

merged 1 commit into from
May 13, 2013

Conversation

WestLangley
Copy link
Collaborator

SpotLightHelper geometry has been simplified. It is now just a cone. (I took the liberty of removing the sphere at the tip of the cone as it was not needed -- the light cone is sufficient.)

SpotLightHelper now works with spotlights that have parents.

As in the previous version, SpotLightHelper.update() must be called inside the render loop.

As in the previous version, an instance of SpotLightHelper must be a child of the scene. (This is because SpotLightHelper.update() relies on Object3D.lookAt(), which does not have hierarchical support.)

@mrdoob mrdoob merged commit 3cb9146 into mrdoob:dev May 13, 2013
@mrdoob
Copy link
Owner

mrdoob commented May 13, 2013

Great!
However, the lightSphere was there to be able to be grabbed in the editor. Maybe it can be a small cone instead?

@gaitat
Copy link

gaitat commented May 13, 2013

In part, it should be fixing Issue #3392

@WestLangley
Copy link
Collaborator Author

However, the lightSphere was there to be able to be grabbed in the editor. Maybe it can be a small cone instead?

Do you want to un-merge this, or keep it, and change the editor to render some thingy that can be grabbed?

@mrdoob
Copy link
Owner

mrdoob commented May 13, 2013

I'll add something.

@WestLangley
Copy link
Collaborator Author

@arodic Thanks for the complement. Feel free to make any changes you see fit -- or make a request, and I'll be glad to have a look. It is true, we have made several attempts at refactoring the helpers recently. We need to see what works best, and then make them consistent.

Thinking outside of the box here... A grabby thingy is only needed by the editor. I wonder if a draggable sprite could be added to the scene by the editor? A user can then drag the sprite. That way the helpers can remain simple.

@arodic
Copy link
Contributor

arodic commented May 20, 2013

I thought you would only need helpers in editor-like environments. The helpers provide visible geometry for the objects that dont have their own geometry. Since geometry is also used for picking it would be nice to to have the picking geometry in the helpers as well (it doesn't have to be visible). For example it could be .picker attribute of the helper object.

Sure we can add picking geometry in the editor itself but I imagine any application that uses helpers will also need picking so it makes sense to bundle it all in the helper.

@WestLangley
Copy link
Collaborator Author

I imagine any application that uses helpers will also need picking

No way. Helpers are very useful for viewing parameter settings in lights and shadow maps, and for debugging geometries, normals, tangents, camera settings, for example -- none of which have anything to do with picking.

I am not opposed to adding a .picker attribute to the helpers, I just don't particularly like it. Helpers were not created for that purpose.

On the other hand, this is not a battle I want to fight, so whatever you want is fine with me. :-)

@mrdoob
Copy link
Owner

mrdoob commented May 20, 2013

I agree with @WestLangley. I think the editor should add the pickers itself.

@arodic
Copy link
Contributor

arodic commented May 20, 2013

Helpers were not created for that purpose.

I wasn't aware of that. They were used for picking in the editor when I first learned about them. Technically speaking, they can still be used for picking. All except this one.

On the other hand, this is not a battle I want to fight, so whatever you want is fine with me. :-)

Im sorry if my comment came out agressive. I most certainly dont want to fight or change something because I like it my way.

@WestLangley
Copy link
Collaborator Author

@arodic I did not take your comments as aggressive at all, and I apologize if my reply seemed as though I did.

BTW, thanks for all your recent contributions. They are awesome, IMHO.

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.

4 participants