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

PannerNode.refDistance shouldn't throw when it's set to 0. #1105

Closed
trzecieu opened this issue Dec 7, 2016 · 5 comments
Closed

PannerNode.refDistance shouldn't throw when it's set to 0. #1105

trzecieu opened this issue Dec 7, 2016 · 5 comments

Comments

@trzecieu
Copy link

trzecieu commented Dec 7, 2016

Hi,
Changes introduced in #1064
contradict with OpenAL specification: https://www.openal.org/documentation/openal-1.1-specification.pdf
Where AL_REFERENCE_DISTANCE takes [0, any] ($Table 4-12)

I understand that proposed changes prevent to have strange result for exponential mode, but in case of linear mode it's safe to have 0 as a value.

I brought OpenGL as a reference, because when a game is ported with Emscripten to ASM.js/WebAssembly, and it uses OpenAL then it will throw an exception.

CC:
@rtoy
@padenot

@rtoy
Copy link
Member

rtoy commented Dec 7, 2016

First, I would say WebAudio is not OpenAL, but it is certainly inspired by OpenAL.

It's probably possible to give definite meanings for all of these distance models, but does anyone actually use a ref distance of 0?

@trzecieu
Copy link
Author

trzecieu commented Dec 7, 2016

OpenAL and linear model works well for 2D games.
I don't know if any other company uses this combination, and definitely I can't say if somebody has published HTML5 game.
Major concern refers to games that are already published. Our tech stack can adjust to new situation.

In case of removing setVelocity from PannerNode it was slightly different. It was nicely documented and we have seen messages about deprecation. Removing possibility to set reference distance to 0 was sudden.

@rtoy
Copy link
Member

rtoy commented Dec 7, 2016

I'm not really opposed to reverting this change. Just now need to come up with appropriate formulas for the corner cases for the inverse and exponential models. And since you found this, it seems clear that someone sets refDistance to 0 as part of their application.

Added WG review label for wider review.

@rtoy
Copy link
Member

rtoy commented Feb 2, 2017

Update formulas to allow 0.

@trzecieu
Copy link
Author

trzecieu commented Feb 11, 2017

Thank you! Feel free to close it.
Update: I created a PR #1147 that changes index.html, but most likely it's not only one required change. Please let me know if I can help somehow.

@rtoy rtoy closed this as completed in e629c03 Mar 30, 2017
rtoy added a commit that referenced this issue Mar 30, 2017
Fix #1105: Allow refDistance to be 0 for PannerNode
MXEBot pushed a commit to mirror/chromium that referenced this issue Apr 13, 2017
Reverts the change that required refDistance to be strictly positive.
This was updated in the spec in issue
WebAudio/web-audio-api#1105, fixed by
WebAudio/web-audio-api#1150

BUG=672156
TEST=Panner/panner-distance-clamping.html

Review-Url: https://codereview.chromium.org/2807313002
Cr-Commit-Position: refs/heads/master@{#464178}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants