Remove optionalTarget: Clean up #13522
Conversation
src/core/BufferGeometry.js
Outdated
| @@ -278,15 +278,21 @@ BufferGeometry.prototype = Object.assign( Object.create( EventDispatcher.prototy | |||
|
|
|||
| center: function () { | |||
|
|
|||
| this.computeBoundingBox(); | |||
There was a problem hiding this comment.
Hmm, this change can break user code if they did this:
var center1 = geometry1.center();
var center2 = geometry2.center();There was a problem hiding this comment.
Oh, I see. I guess we have to create a new object in center().
There was a problem hiding this comment.
Okay, changed the code.
Yeah, sounds reasonable. |
|
I've only found one example ( |
|
BTW: |
examples/css3d_molecules.html
Outdated
| var offset = geometryAtoms.center(); | ||
| geometryBonds.translate( offset.x, offset.y, offset.z ); | ||
| geometryAtoms.center(); | ||
| geometryBonds.center(); |
There was a problem hiding this comment.
Does that produce the same offset for both atoms and bonds?
There was a problem hiding this comment.
The visual result is the same.
BTW: I've missed webgl_loader_pdb. Applied the same change there, too.
There was a problem hiding this comment.
Sorry. How you can center the atoms and bonds separately makes no sense to me.
There was a problem hiding this comment.
I guess that works because PDBLoader builds both geometries from the same vertex data.
There was a problem hiding this comment.
I would inline the center() code, if you have to, and not rely on that.
|
Please file API changes in a separate PR. |
|
Okay, i undo the change to |
|
Thanks... Not to derail, but do you know how to collapse these commits into a single 'Remove optionalTarget: clean up' commit, so all the intermediary changes -- and reverted changes -- do not appear in the file commit history? |
|
Okay, i made a force push. The old commits are now gone. I also inlined the code in the PDB examples. Hope, it's now okay like that 😅 |
|
Thanks! |
Some clean up to avoid warnings...