RESOLVED FIXED 227596
REGRESSION (r279466): [Big Sur] webgl/1.0.3/conformance & webgl/2.0.0/conformance are failing
https://bugs.webkit.org/show_bug.cgi?id=227596
Summary REGRESSION (r279466): [Big Sur] webgl/1.0.3/conformance & webgl/2.0.0/confor...
ayumi_kojima
Reported 2021-07-01 14:03:48 PDT
webgl/1.0.3/conformance/glsl/functions/glsl-function-atan-xy.html webgl/1.0.3/conformance/ogles/GL/atan/atan_009_to_012.html webgl/2.0.0/conformance/glsl/functions/glsl-function-atan-xy.html webgl/2.0.0/conformance/ogles/GL/atan/atan_009_to_012.html History: https://results.webkit.org/?suite=layout-tests&suite=layout-tests&suite=layout-tests&suite=layout-tests&test=webgl%2F1.0.3%2Fconformance%2Fglsl%2Ffunctions%2Fglsl-function-atan-xy.html&test=webgl%2F1.0.3%2Fconformance%2Fogles%2FGL%2Fatan%2Fatan_009_to_012.html&test=webgl%2F2.0.0%2Fconformance%2Fglsl%2Ffunctions%2Fglsl-function-atan-xy.html&test=webgl%2F2.0.0%2Fconformance%2Fogles%2FGL%2Fatan%2Fatan_009_to_012.html It looks like the tests have started failing at r279466 https://build.webkit.org/results/Apple-BigSur-Release-WK1-Tests/r279475%20(3797)/results.html --- /Volumes/Data/worker/bigsur-release-tests-wk1/build/layout-test-results/webgl/1.0.3/conformance/glsl/functions/glsl-function-atan-xy-expected.txt +++ /Volumes/Data/worker/bigsur-release-tests-wk1/build/layout-test-results/webgl/1.0.3/conformance/glsl/functions/glsl-function-atan-xy-actual.txt @@ -1,5 +1,30 @@ This test runs the WebGL Test listed below in an iframe and reports PASS or FAIL. Test: ../../../resources/webgl_test_files/conformance/glsl/functions/glsl-function-atan-xy.html -[ PASS ] All tests passed +[ 1: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 2: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 3: FAIL ] images are different: at (0,0): ref=(187,0,0,255) test=(255,0,0,255) tolerance=5 +[ 4: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 5: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 6: FAIL ] images are different: at (0,0): ref=(187,187,0,255) test=(255,255,0,255) tolerance=5 +[ 7: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 8: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 9: FAIL ] images are different: at (0,0): ref=(187,192,187,255) test=(255,255,255,255) tolerance=5 +[ 10: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 11: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 12: FAIL ] images are different: at (0,0): ref=(137,192,187,191) test=(137,255,255,255) tolerance=5 +[ 13: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 14: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 15: PASS ] images are the same +[ 16: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 17: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 18: PASS ] images are the same +[ 19: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 20: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 21: FAIL ] images are different: at (0,0): ref=(187,192,186,255) test=(186,255,185,255) tolerance=5 +[ 22: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 23: PASS ] getError was expected value: NO_ERROR : no errors from draw +[ 24: FAIL ] images are different: at (0,0): ref=(137,192,186,191) test=(137,255,185,190) tolerance=5 +[ 25: PASS ] successfullyParsed is true +[ FAIL ] 6 failures reported
Attachments
Patch (2.05 KB, patch)
2021-07-01 16:57 PDT, Kyle Piddington
no flags
Patch (16.96 KB, patch)
2021-07-02 17:34 PDT, Kyle Piddington
no flags
Patch (17.48 KB, patch)
2021-07-02 17:39 PDT, Kyle Piddington
no flags
Patch for landing (17.43 KB, patch)
2021-07-06 11:52 PDT, Kyle Piddington
no flags
Radar WebKit Bug Importer
Comment 1 2021-07-01 14:06:33 PDT
Kenneth Russell
Comment 2 2021-07-01 15:49:06 PDT
Let's mark these as expected failures in TestExpectations and follow up. If just the arctangent function has exceptionally poor precision when fastMathEnabled is used, we could plausibly increase the tolerance on these specific WebGL conformance tests. I think that we shouldn't revert the changes from Bug 227226 . Without fastMathEnabled, it seems likely that a significant amount of WebGL content will be slower with ANGLE's Metal backend than the OpenGL backend. Would it be feasible or appropriate for Apple engineers to also file a bug against Metal to understand why precision of this one trigonometric function is so poor?
Kyle Piddington
Comment 3 2021-07-01 16:57:07 PDT
Kenneth Russell
Comment 4 2021-07-01 17:35:28 PDT
Comment on attachment 432749 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432749&action=review This seems like a good solution; didn't realize these options could be changed per-shader. r+ once this builds and passes tests. > Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/mtl_utils.mm:603 > + //Fastmath on downlevel Intel causes some issues (Webkit bug 227596) Space after //.
Alexey Proskuryakov
Comment 5 2021-07-01 18:43:20 PDT
Comment on attachment 432749 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432749&action=review > Source/ThirdParty/ANGLE/ChangeLog:7 > + Proposed workaround: Disable fastmath on shaders when atan or invariants are used > + on Intel systems This patch seems to only possibly enable fastMath, never disable it?
Kenneth Russell
Comment 6 2021-07-01 18:47:41 PDT
Comment on attachment 432749 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432749&action=review >> Source/ThirdParty/ANGLE/ChangeLog:7 >> + on Intel systems > > This patch seems to only possibly enable fastMath, never disable it? It's disabled either if the shader uses atan, or uses the invariant qualifier - am I misreading the code below?
Kyle Piddington
Comment 7 2021-07-02 17:34:34 PDT
EWS Watchlist
Comment 8 2021-07-02 17:35:32 PDT
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Kyle Piddington
Comment 9 2021-07-02 17:39:19 PDT
John Cunningham
Comment 10 2021-07-02 17:44:11 PDT
Comment on attachment 432836 [details] Patch LGTM
Kenneth Russell
Comment 11 2021-07-04 18:31:59 PDT
Comment on attachment 432836 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432836&action=review Looks good. r+ also based on johncunningham@'s review. One small comment. Note to save time that you don't need to request review on subsequent patches - you can just change the ChangeLogs to say "Reviewed by Kenneth Russell." and cq+. > Source/ThirdParty/ANGLE/include/platform/FeaturesMtl.h:133 > + Feature intelDisableFastMathWorkaround = { This feature doesn't need to end in "Workaround", and this name is confusing to read - it sounds like it's disabling a workaround related to fast math. intelDisableFastMath would be clearer in my opinion. > Source/ThirdParty/ANGLE/include/platform/FeaturesMtl.h:134 > + "intel_disable_fast_math_workarounds", FeatureCategory::MetalWorkarounds, Please name the same as the feature - workaround instead of workarounds in this case.
Kyle Piddington
Comment 12 2021-07-06 11:52:02 PDT
Created attachment 432956 [details] Patch for landing
EWS
Comment 13 2021-07-06 12:33:17 PDT
Committed r279606 (239429@main): <https://commits.webkit.org/239429@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 432956 [details].
Note You need to log in before you can comment on or make changes to this bug.