Skip to content

Conversation

@GuoBinyong
Copy link

The following facilitation methods have been added:

  • Matrix4#fromMatrix3() :Transitions from Matrix3 to Matrix4 are a common requirement, but there is currently no API for this, so this method has been added
  • Quaternion#fromEuler(euler: Euler)
    • Although there is 'Euler#getQuaternion()', it returns a new instance of 'Quaternion', which is not conducive to reusing the instance

Copy link
Contributor

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

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

@GuoBinyong Many thanks for the contributions very appreciated.

Even though it is some extra work for you, it might be best to split them into separate PR for quickest resolution.

  • New public methods require documentation updates, and mention in docs/whats-new.md
  • At least a simple test case for the new method is also required.

* @param matrix3
* @returns self
*/
fromMatrix3(matrix3: Readonly<NumericArray>): this {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine, though it does add a little to the size of the Matrix4 class for every user.

Copy link
Contributor

Choose a reason for hiding this comment

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

As you can see in the CI log, you need to run yarn lint fix

You are welcome to join our OpenJS slack channel, see https://openvisualization.org for the link

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