Skip to content

Conversation

@natacha-beck
Copy link
Contributor

The idea here was to recenter the model to the origin if the original data is shifted. In this way the user data should appear at the center when it is loaded.
I tried to see all the edges case that can appear in the demo, but maybe I miss some.

Can you take a second look and let me know if you think it is ok.

<input id="axes-controls" type="checkbox">Display axes<br />
</p>
<p>
<div>
Copy link
Member

Choose a reason for hiding this comment

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

Missing two "s":

Origin corresponds to model's center

* ```
*/
viewer.modelCentric = function(model_centric) {
if (model_centric === undefined) {model_centric = false;}

Choose a reason for hiding this comment

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

Sorry if this is overly pedantic, but I really prefer to keep statements on separate lines:

if (model_centric === undefined) {
   model_centric = false;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I will fix it, and I'm kind of agree with you.
In fact I like used return if something but it is not supported in JS.

// Caculate the offset
var offset_centroid = new THREE.Vector3();
offset_centroid.copy(model.userData.model_center_offset);
if (model_centric === false) { offset_centroid.negate();}

Choose a reason for hiding this comment

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

Missed one here.

@rdvincent
Copy link

Looks good. Shall I merge?

@natacha-beck
Copy link
Contributor Author

If it seems ok for you yes Pierre had review too, so everything seems ok, I
will do a release next week.

On 27 May 2016 at 15:51, Robert D Vincent notifications@github.com wrote:

Looks good. Shall I merge?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#300 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAvddmmFFeHb8BGV8PvMXlUbUrJ0_J4Zks5qF0s5gaJpZM4Int1K
.

@rdvincent rdvincent merged commit b9abdb8 into aces:master May 27, 2016
@natacha-beck natacha-beck deleted the centric_rotation branch August 8, 2016 14:17
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.

3 participants