Bug 233529 - REGRESSION(r283855) [GTK][WPE] imported/w3c/web-platform-tests/webaudio/the-audio-api/the-pannernode-interface/panner-equalpower.html fails
Summary: REGRESSION(r283855) [GTK][WPE] imported/w3c/web-platform-tests/webaudio/the-a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Audio (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-11-26 14:33 PST by Arcady Goldmints-Orlov
Modified: 2021-11-30 08:03 PST (History)
14 users (show)

See Also:


Attachments
Patch (3.43 KB, patch)
2021-11-29 13:37 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Arcady Goldmints-Orlov 2021-11-26 14:33:51 PST
In r283855, EqualPowerPanner::pan() was modified to use vector instructions where available. This caused a regression on GTK and WPE in the test imported/w3c/web-platform-tests/webaudio/the-audio-api/the-pannernode-interface/panner-equalpower.html, presumably because of some problem in the implementation of VectorMath::multiplyByScalarThenAddToVector() which is implemented in the WebKit code for non-Apple platforms but uses an Apple library on Apple platforms.
Comment 1 Chris Dumez 2021-11-29 13:28:55 PST
(In reply to Arcady Goldmints-Orlov from comment #0)
> In r283855, EqualPowerPanner::pan() was modified to use vector instructions
> where available. This caused a regression on GTK and WPE in the test
> imported/w3c/web-platform-tests/webaudio/the-audio-api/the-pannernode-
> interface/panner-equalpower.html, presumably because of some problem in the
> implementation of VectorMath::multiplyByScalarThenAddToVector() which is
> implemented in the WebKit code for non-Apple platforms but uses an Apple
> library on Apple platforms.

What architecture does it fail on? ARM or Intel?
Comment 2 Chris Dumez 2021-11-29 13:31:05 PST
Ok, I can reproduce the following failure on Intel if I disable the `#if USE(ACCELERATE)` in VectorMath.cpp:
--- /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/layout-test-results/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-pannernode-interface/panner-equalpower-expected.txt
+++ /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/layout-test-results/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-pannernode-interface/panner-equalpower-actual.txt
@@ -14,7 +14,13 @@
 PASS   Mono: Left and right channels is identical to the array [expected array].
 PASS < [mono source=listener] All assertions passed. (total 1 assertions)
 PASS > [stereo source=listener] Source and listener at the same position
-PASS   Stereo: Left and right channels is identical to the array [expected array].
-PASS < [stereo source=listener] All assertions passed. (total 1 assertions)
-PASS # AUDIT TASK RUNNER FINISHED: 3 tasks ran successfully.
+FAIL X Stereo: Left and right channels expected to be equal to the array [expected array] but differs in 8025 places:
+	Index	Actual			Expected
+	[129]	-9.094678e-1	-9.937366e-1
+	[130]	-8.316806e-1	-9.996189e-1
+	[131]	-7.479766e-1	-9.983897e-1
+	[132]	-6.589520e-1	-9.900583e-1
+	...and 8021 more errors. assert_true: expected true got false
+FAIL < [stereo source=listener] 1 out of 1 assertions were failed. assert_true: expected true got false
+FAIL # AUDIT TASK RUNNER FINISHED: 1 out of 3 tasks were failed. assert_true: expected true got false

Seems to confirm that the Intel implementation for one of our vector math functions is somehow wrong. The results seem pretty different so it doesn't look like a simple precision issue.
Comment 3 Chris Dumez 2021-11-29 13:33:09 PST
Seems that this is wrong somehow:
```
void multiplyByScalarThenAddToVector(const float* inputVector1, float scalar, const float* inputVector2, float* outputVector, size_t numberOfElementsToProcess)
{
    multiplyByScalarThenAddToOutput(inputVector1, scalar, outputVector, numberOfElementsToProcess);
    add(outputVector, inputVector2, outputVector, numberOfElementsToProcess);
}
```

I am looking into it.
Comment 4 Chris Dumez 2021-11-29 13:37:46 PST
Created attachment 445333 [details]
Patch
Comment 5 Philippe Normand 2021-11-29 13:54:48 PST
Comment on attachment 445333 [details]
Patch

Thanks Arcady for spotting this issue and Chris for fixing it ;)
Comment 6 EWS 2021-11-29 14:38:48 PST
Committed r286264 (244627@main): <https://commits.webkit.org/244627@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 445333 [details].
Comment 7 Radar WebKit Bug Importer 2021-11-29 14:39:22 PST
<rdar://problem/85835819>
Comment 8 Arcady Goldmints-Orlov 2021-11-30 08:03:27 PST
*** Bug 217845 has been marked as a duplicate of this bug. ***