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

Camera: Properly update world matrix prior to use #20224

Merged
merged 1 commit into from
Oct 26, 2020

Conversation

WestLangley
Copy link
Collaborator

No description provided.

@WestLangley WestLangley added this to the r121 milestone Aug 30, 2020
@WestLangley
Copy link
Collaborator Author

@Mugen87 I think this completes the remaining use cases. :-)

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 21, 2020

@WestLangley What do you think about introducing default parameters to updateWorldMatrix()? I mean:

updateWorldMatrix: function ( updateParents = true, updateChildren = false ) {

This seems to be a proper default for a world matrix update routine.

@WestLangley
Copy link
Collaborator Author

@Mugen87

The method has implied default values of false and false. I did that intentionally when I implemented the method.

If you would like default values specified explicitly, I would suggest:

updateWorldMatrix: function ( updateParents = false, updateChildren = false ) {

I think @donmccurdy would be particularly happy with that decision. :-)

This makes it clear that it is the user's responsibility to traverse the tree based on the user's use case.

@DefinitelyMaybe
Copy link
Contributor

This makes it clear that it is the user's responsibility to traverse the tree based on the user's use case.

that sounds like the way to go IMO

@WestLangley
Copy link
Collaborator Author

Again, let's ensure the method returns the correct answer as a first step. We can improve it later.

@mrdoob mrdoob modified the milestones: r121, r122 Sep 29, 2020
@WestLangley
Copy link
Collaborator Author

This PR is a follow-up to #19969.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 26, 2020

@mrdoob Since #19969 was merged, this PR should go in, too. Unlike #20221, this PR only improves the already existing world matrix computation.

@mrdoob mrdoob merged commit 1323906 into mrdoob:dev Oct 26, 2020
@mrdoob
Copy link
Owner

mrdoob commented Oct 26, 2020

Thanks!

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.

None yet

5 participants