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

Editor: Focus will frame target in view. #13165

Merged
merged 4 commits into from
Feb 27, 2018

Conversation

Adam4lexander
Copy link
Contributor

This changes the EditorControls.focus behaviour in a few ways:

  1. Will translate the camera to a distance proportional to the bounding box size of the target to frame it in the viewport.

  2. The camera will only translate when focusing instead of rotating due to a lookAt call. I think this behaviour is more standard in 3d programs and I find the lookAt method to be a little disorienting.

  3. Fixed a bug where the camera couldn't focus on empties which don't have geometries, such as lights or groups.

…on target, will not rotate. Fixed a bug where empty's couldn't be focused on.
…o focus on target, will not rotate. Fixed a bug where empty's couldn't be focused on."

This reverts commit 65a7e28.
…on target, will not rotate. Fixed a bug where empty's couldn't be focused on.
@mrdoob mrdoob changed the title Focus will frame target in view. Editor: Focus will frame target in view. Jan 24, 2018
@@ -37,8 +37,36 @@ THREE.EditorControls = function ( object, domElement ) {

this.focus = function ( target ) {

if ( target === undefined || !target.isObject3D ) {
Copy link
Owner

@mrdoob mrdoob Jan 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... groups, lights and cameras extend Object3D so I'm not sure about the last check here...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the idea, this is only guarding against some random non-3d JavaScript object being passed in. The camera should focus on lights and cameras and groups too.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, what random non-3d JavaScript object? And how could it get there?

Copy link
Contributor Author

@Adam4lexander Adam4lexander Feb 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example with the code: EditorControls.focus( 7 );
Maybe I'm being too protective... I can take it out if you think its not necessary?

I think it would only get there if the user made a mistake. So maybe it would be better for it to error in that case.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we don't do type checking in general. I would take it out 👍

@mrdoob mrdoob added this to the r91 milestone Jan 24, 2018
@mrdoob
Copy link
Owner

mrdoob commented Feb 14, 2018

@Adam4lexander
Copy link
Contributor Author

Done. isObject3D check is removed

@mrdoob mrdoob merged commit 0a7fb46 into mrdoob:dev Feb 27, 2018
@mrdoob
Copy link
Owner

mrdoob commented Feb 27, 2018

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Feb 27, 2018

FYI I "mrdoobified" the code in 689306f 😇

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.

2 participants