PannerNode.rolloffFactor setter should clamp value to [0, 1] when distanceModel is "linear": - https://www.w3.org/TR/webaudio/#dom-pannernode-rollofffactor
Created attachment 406814 [details] Patch
Comment on attachment 406814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406814&action=review > Source/WebCore/platform/audio/Distance.cpp:72 > + auto clampedRolloffFactor = std::min(m_rolloffFactor, 1.0); This says clamped but it only does the 1.0 half of the clamping. I’d expect to see std::max or std::clamp here.
Comment on attachment 406814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406814&action=review >> Source/WebCore/platform/audio/Distance.cpp:72 >> + auto clampedRolloffFactor = std::min(m_rolloffFactor, 1.0); > > This says clamped but it only does the 1.0 half of the clamping. I’d expect to see std::max or std::clamp here. to handle the 0.0 minimum.
(In reply to Darin Adler from comment #3) > Comment on attachment 406814 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=406814&action=review > > >> Source/WebCore/platform/audio/Distance.cpp:72 > >> + auto clampedRolloffFactor = std::min(m_rolloffFactor, 1.0); > > > > This says clamped but it only does the 1.0 half of the clamping. I’d expect to see std::max or std::clamp here. > > to handle the 0.0 minimum. We already throw an exception in the setter if the JS tries to set a negative value. Therefore, we only need to make sure the value is no more than 1.
(In reply to Chris Dumez from comment #4) > (In reply to Darin Adler from comment #3) > > Comment on attachment 406814 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=406814&action=review > > > > >> Source/WebCore/platform/audio/Distance.cpp:72 > > >> + auto clampedRolloffFactor = std::min(m_rolloffFactor, 1.0); > > > > > > This says clamped but it only does the 1.0 half of the clamping. I’d expect to see std::max or std::clamp here. > > > > to handle the 0.0 minimum. > > We already throw an exception in the setter if the JS tries to set a > negative value. Therefore, we only need to make sure the value is no more > than 1. From https://www.w3.org/TR/webaudio/#dom-pannernode-rollofffactor: "A RangeError exception MUST be thrown if this is set to a negative value."
Comment on attachment 406814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406814&action=review > Source/WebCore/Modules/webaudio/PannerNode.cpp:282 > return Exception { RangeError, "rolloffFactor cannot be set to a negative value"_s }; Throwing happens here if value is negative.
Created attachment 406819 [details] Patch
Committed r265838: <https://trac.webkit.org/changeset/265838> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406819 [details].
<rdar://problem/67360120>