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

Optional makeEmpty in THREE.Box3 #10474

Merged
merged 7 commits into from
Jan 12, 2017
Merged

Optional makeEmpty in THREE.Box3 #10474

merged 7 commits into from
Jan 12, 2017

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Dec 27, 2016

Hi.
This solve problems with to collect parallel objects(no relationship) in THREE.Box3.
Otherwise would have to create a Box3 for each object or add all in an single Object3D

var meshes = [mesh1, mesh2, ..[n].. ]
var bbox = new THREE.Box3();

for(var i = 0; i < meshes.length; i++) {
	bbox.setFromObject( meshes[i], false ); // false = not call `makeEmpty`
}

@WestLangley
Copy link
Collaborator

Instead, why not add the method

Box3.expandByObject( object )

and rewrite setFromObject() to call the new method.

@sunag
Copy link
Collaborator Author

sunag commented Dec 28, 2016

and rewrite setFromObject() to call the new method.

@WestLangley added!

@@ -92,8 +94,6 @@ Box3.prototype = {

setFromPoints: function ( points ) {

this.makeEmpty();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you remove makeEmpty() here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sry my distraction... I will add.

@WestLangley
Copy link
Collaborator

/ping @Mugen87 code

/ping @looeee docs

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 28, 2016

I'm not 100% sure about the consistency of this change. Box3 .setFromObject() will be the only method that allows an optional empty operation. But what about the other .setFrom*() methods?

@WestLangley
Copy link
Collaborator

@Mugen87 We have expandBy*() and setFrom*() now. See the coments above, and ignore the post title.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 28, 2016

I see...

@mrdoob mrdoob merged commit 59e929e into mrdoob:dev Jan 12, 2017
@mrdoob
Copy link
Owner

mrdoob commented Jan 12, 2017

Thanks!

@calvin
Copy link

calvin commented Feb 28, 2017

Need to update the docs for the new .expandByObject() method.

calvin added a commit to calvin/DefinitelyTyped that referenced this pull request Feb 28, 2017
Not a documented method yet, but have been included in the latest build (r84) of the library.

See mrdoob/three.js#10474 for more info.
mhegazy pushed a commit to DefinitelyTyped/DefinitelyTyped that referenced this pull request Mar 10, 2017
Not a documented method yet, but have been included in the latest build (r84) of the library.

See mrdoob/three.js#10474 for more info.
@sunag sunag deleted the 84dev branch June 3, 2019 23:00
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.

5 participants