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

Convert the 'populateQuaternion' function to 'quaternion' attribute #19

Merged
merged 1 commit into from
Mar 21, 2017

Conversation

pozdnyakov
Copy link

@pozdnyakov pozdnyakov commented Mar 20, 2017

The attribute will be more convenient to use.

The main reason for the previous using of BYOB was the potential GC issues, now BYOB here seems to be unnecessary: w3c/sensors#153 (comment)


Preview | Diff

The attribute will be more convenient to use.

The main reason for the previous using of BYOB was the potential GC issues, now BYOB here seems to be unnecessary:  w3c/sensors#153 (comment)
@kenchris
Copy link
Contributor

OK, we will probably want populate methods later though :) lgtm

@pozdnyakov
Copy link
Author

OK, we will probably want populate methods later though :)

IMO populate still makes sense for matrices as matrix can contain translation data before calling populateMatrix, so that after the call the matrix will contain both translation and orientation.
For quaternions populate is useless.

@kenchris
Copy link
Contributor

But are they? If it makes sense getting different quats in different shader instances, it would not be. I am not sure about those use-cases but they should cover some of the same as matrices

@pozdnyakov
Copy link
Author

not sure I understood you correctly, what I mean is that matrix can contain both translation and rotation data (pls see example 5 at https://w3c.github.io/webvr/spec/latest/#interface-vrpose), so if translation elements are already predefined, populateMatrix call won't erase them but just introduce rotation data to the given matrix, this is quite convenient.

@alexshalamov
Copy link

lgtm. We can add populateMatrix(Float32Array or DOMMatrix) later. I wish DOMMatrix would expose Float32Array internal buffer, so it is easy and free to convert them back and forth, making them compatible with WebGL.

@alexshalamov
Copy link

@pozdnyakov We should also document that if transformation matrix that was provided to populateMatrix contains scaling / rotation, those values will be overwritten, only translation values would be kept intact.

@pozdnyakov pozdnyakov merged commit 6d90a42 into w3c:gh-pages Mar 21, 2017
@pozdnyakov pozdnyakov deleted the quaternion_attribute branch March 21, 2017 14:12
@pozdnyakov
Copy link
Author

Reverted this with 7d7f697 as exporting Float32Array as property makes the sensor data mutable

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