search

GoogleChrome / omnitone

Spatial Audio Rendering on the web.
https://googlechrome.github.io/omnitone
Apache License 2.0
855 stars 114 forks source link

Update TOA rotation #89

Closed hoch closed 6 years ago

hoch commented 6 years ago

PTAL.

As discussed in here, this PR reverses the rotation direction.

I rather not do this at this point unless we can maintain the rotation direction.

drewbitllama commented 6 years ago

The issue is that currently hoa and toa rotate in opposite ways given the same threejs matrix. This change ensures they rotate in the way. On Tue, Mar 13, 2018 at 2:12 PM Hongchan Choi notifications@github.com wrote:

@hoch https://github.com/hoch requested your review on: GoogleChrome/omnitone#89 https://github.com/GoogleChrome/omnitone/pull/89 Update TOA rotation.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/GoogleChrome/omnitone/pull/89#event-1519641426, or mute the thread https://github.com/notifications/unsubscribe-auth/AGDsNgd8xjmxIoaAEVoOIcRaBSc26C0wks5teDYkgaJpZM4Spdbz .

hoch commented 6 years ago

Yes, being able to use the matrix from Three.js is great, but this change breaks other pages that have been using Omnitone this way. Can we consider introducing the new APIs for the better Three.js support?

  1. I think we can go further than changing the sign in the formula. (accessing camera directly?)
  2. For the short-term solution, I suggest to use a helper function to do this conversion.
drewbitllama commented 6 years ago

Ok sgtm On Tue, Mar 13, 2018 at 2:19 PM Hongchan Choi notifications@github.com wrote:

Yes, being able to use the matrix from Three.js is great, but this change breaks other pages that have been using Omnitone this way. Can we consider introducing the new APIs for the better Three.js support?

  1. I think we can go further than changing the sign in the formula. (accessing camera directly?)
  2. For the short-term solution, I suggest to use a helper function to do this conversion.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/GoogleChrome/omnitone/pull/89#issuecomment-372822245, or mute the thread https://github.com/notifications/unsubscribe-auth/AGDsNtemU1tlwdxL6pJcPmQamPBsDMuJks5teDfzgaJpZM4Spdbz .

hoch commented 6 years ago

Based on this discussion, we decided to bring this change even if it might change the behavior of some pages out there.

  1. This is to make the rotation mechanism identical between FOA and HOA renderer.
  2. With this change, the rotator directly can take the matrix from Three.js.

@drewbitllama WDYT?