Bug 210994 - REGRESSION (r256784?): Shadertoy demo no longer works in Safari
Summary: REGRESSION (r256784?): Shadertoy demo no longer works in Safari
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kenneth Russell
URL:
Keywords: InRadar
: 133632 133639 141240 (view as bug list)
Depends on: 205483
Blocks: 126404 209740 210524 211156 211971 212181
  Show dependency treegraph
 
Reported: 2020-04-24 14:12 PDT by Dean Jackson
Modified: 2020-08-31 12:41 PDT (History)
15 users (show)

See Also:


Attachments
Patch (78.79 KB, patch)
2020-04-28 21:59 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Patch (82.58 KB, patch)
2020-04-28 22:36 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2020-04-24 14:12:57 PDT
Two shadertoys that worked on Catalina system safari but no longer work on trunk builds.

https://www.shadertoy.com/view/lltSR8
https://www.shadertoy.com/view/XtVXzV
Comment 1 Dean Jackson 2020-04-24 14:13:19 PDT
rdar://61791518
Comment 2 Kenneth Russell 2020-04-24 14:31:16 PDT
Hard to click the trac link in the description:
https://trac.webkit.org/r256784
Comment 3 Kenneth Russell 2020-04-24 14:32:07 PDT
I'll investigate.
Comment 4 Kenneth Russell 2020-04-24 18:31:17 PDT
Not sure what's going on yet. For https://www.shadertoy.com/view/lltSR8 , ToT with the ANGLE backend reports to the console:

[Log] WebGL (2.0=false): F32 Textures: yes, F16 Textures: yes, Depth Textures: yes, Anisotropic Textures: yes, MRT: yes, Render to 32F: yes, Shader Texture LOD: yes, Texture Units: 16, Max Texture Size: 16384, Max Render Buffer Size: 8192, Max Cubemap Size: 16384 (piLibs.js, line 487)
[Log] TypeError: null is not an object (evaluating 'this.mAudioContext.resume') (pgWatch.js, line 1648)
(anonymous function) — effect.js:3625
(anonymous function) — pgWatch.js:705
(anonymous function) — pgWatch.js:1629
dataLoadShader — pgWatch.js:1960
(anonymous function) — pgWatch.js:2052
Comment 5 Kenneth Russell 2020-04-26 16:38:14 PDT
ToT with USE_ANGLE=0 also fails to render this example. The console output:

[Log] WebGL (2.0=false): F32 Textures: yes, F16 Textures: yes, Depth Textures: yes, Anisotropic Textures: yes, MRT: yes, Render to 32F: yes, Shader Texture LOD: yes, Texture Units: 16, Max Texture Size: 16384, Max Render Buffer Size: 8192, Max Cubemap Size: 16384 (piLibs.js, line 487)
[Log] TypeError: null is not an object (evaluating 'this.mAudioContext.resume') (pgWatch.js, line 1648)

Not sure why Web Audio is failing to initialize on ShaderToy on ToT.

Actually - most if not all WebGL content is broken on ToT with ANGLE disabled. The WebGL Aquarium renders black, for example. Maybe the recent texture related refactorings broke the non-ANGLE path.
Comment 6 Kenneth Russell 2020-04-27 13:53:55 PDT
Trying to run a bisect for this to narrow down possible causes. Finding the following:

./Tools/Scripts/bisect-builds -a x86_64 -p mac-catalina -s 236784 -e 256784

Testing revision 253641...
Starting SafariForWebKitDevelopment with DYLD_FRAMEWORK_PATH set to point to built WebKit in /var/folders/4j/b231sqks3ts88_ls1h2069jm002z4b/T/tmpzfkm8r/Release.
dyld: Symbol not found: _OBJC_CLASS_$__WKInspectorDebuggableInfo
  Referenced from: /System/Library/PrivateFrameworks/WebInspector.framework/Versions/A/WebInspector
  Expected in: /var/folders/4j/b231sqks3ts88_ls1h2069jm002z4b/T/tmpzfkm8r/Release/WebKit.framework/Versions/A/WebKit
 in /System/Library/PrivateFrameworks/WebInspector.framework/Versions/A/WebInspector

Is this expected? Am I probing back too far in the build archives?
Comment 7 Kenneth Russell 2020-04-27 13:54:33 PDT
(Note: I'm running locally on Catalina - 10.15.4.)
Comment 8 Kenneth Russell 2020-04-27 13:56:44 PDT
Farther along in the build archive:

Testing revision 254930...
Starting SafariForWebKitDevelopment with DYLD_FRAMEWORK_PATH set to point to built WebKit in /var/folders/4j/b231sqks3ts88_ls1h2069jm002z4b/T/tmpzfkm8r/Release.
2020-04-27 13:55:06.076 SafariForWebKitDevelopment[9189:3323409] -[BrowserWKView _negotiatedLegacyTLS]: unrecognized selector sent to instance 0x7feabc1cd760

The browser window doesn't open, though Safari starts.
Comment 9 Kenneth Russell 2020-04-27 13:58:07 PDT
Good news and perhaps "never mind" - the build at r255689 runs fine.
Comment 10 Kenneth Russell 2020-04-27 14:17:32 PDT
This bisect:

./Tools/Scripts/bisect-builds -a x86_64 -p mac-catalina -s 256700 -e 256800

indicates that the regression happened between r256776 and r256784.
Comment 11 Kenneth Russell 2020-04-27 14:20:09 PDT
https://trac.webkit.org/log/webkit/?revs=256776-256784

Yes, the commits point to this having been broken in https://trac.webkit.org/browser/webkit/?rev=256784 .

Let me look into the WebGL conformance suite results around canvas-to-WebGL-texture uploads. I have a feeling something was broken there and that it's the cause of this and other failures.
Comment 12 Kenneth Russell 2020-04-27 16:09:37 PDT
A ToT build at r260783 runs all of conformance/textures under  https://www.khronos.org/registry/webgl/conformance-suites/1.0.3/webgl-conformance-tests.html cleanly, except for texture-npot-video, which isn't related to this failure.

Looking more directly into the Shadertoy example.
Comment 13 Kenneth Russell 2020-04-27 16:15:55 PDT
This simpler example:

https://www.shadertoy.com/view/4slGDB

fails to render in the Minibrowser in WebKit1, but renders correctly in the Minibrowser in WebKit2.

The problem in the Minibrowser in WebKit1 does appear to be the WebAudio failure - that exception seems to break all Shadertoy examples, regardless of whether they actually use Web Audio.
Comment 14 Kenneth Russell 2020-04-27 16:35:19 PDT
Whatever the WebKit1 WebAudio issue is, it seems unrelated. Debugging https://www.shadertoy.com/view/lltSR8 inside WebKit2 in the MiniBrowser now.
Comment 15 Kenneth Russell 2020-04-27 18:08:50 PDT
The bug is that enabling the OES_texture_float extension in WebGL 1.0 contexts no longer causes the floating point texture formats to become color-renderable. This is caught by optional cases in conformance-suites/1.0.3/conformance/extensions/oes-texture-float.html .

This functionality is exposed via an extension in ANGLE. That extension will need to be enabled, and the internal format of format=GL_RGBA, type=GL_FLOAT, internalformat=GL_RGBA textures changed to GL_RGBA32F. Similarly for GL_RGB textures, as well as the half-float formats (for oes-texture-half-float.html).
Comment 16 Kenneth Russell 2020-04-28 21:59:31 PDT
Created attachment 397923 [details]
Patch
Comment 17 Kenneth Russell 2020-04-28 22:01:03 PDT
The attached fix implements the WebGL 1.0 extensions EXT_color_buffer_half_float and WEBGL_color_buffer_float, and the WebGL 2.0 extension EXT_color_buffer_float. It restores implicit renderability of floating-point textures with the ANGLE backend. This fixes the Shadertoy examples and progresses several WebGL conformance tests.
Comment 18 Kenneth Russell 2020-04-28 22:13:00 PDT
Comment on attachment 397923 [details]
Patch

Will investigate the CMake failures.
Comment 19 Kenneth Russell 2020-04-28 22:36:06 PDT
Created attachment 397926 [details]
Patch
Comment 20 Kenneth Russell 2020-04-28 22:36:33 PDT
Comment on attachment 397926 [details]
Patch

Fixed CMake builds and a couple of other missed Xcode-related files.
Comment 21 Dean Jackson 2020-04-29 11:51:59 PDT
Comment on attachment 397926 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397926&action=review

> Source/WebCore/html/canvas/OESTextureFloat.cpp:40
> +    context.graphicsContextGL()->getExtensions().ensureEnabled("GL_CHROMIUM_color_buffer_float_rgb"_s);
> +    context.graphicsContextGL()->getExtensions().ensureEnabled("GL_CHROMIUM_color_buffer_float_rgba"_s);

I wonder if we should USE(ANGLE) here, although it doesn't hurt that it is missing.
Comment 22 EWS 2020-04-29 12:01:55 PDT
Committed r260908: <https://trac.webkit.org/changeset/260908>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 397926 [details].
Comment 23 Kenneth Russell 2020-04-29 12:44:19 PDT
Comment on attachment 397926 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397926&action=review

>> Source/WebCore/html/canvas/OESTextureFloat.cpp:40
>> +    context.graphicsContextGL()->getExtensions().ensureEnabled("GL_CHROMIUM_color_buffer_float_rgba"_s);
> 
> I wonder if we should USE(ANGLE) here, although it doesn't hurt that it is missing.

None of the other backends will fail because of the absence of these extensions, so I figured the fewer ifdefs, the better.
Comment 24 Kenneth Russell 2020-08-31 12:41:19 PDT
*** Bug 133632 has been marked as a duplicate of this bug. ***
Comment 25 Kenneth Russell 2020-08-31 12:41:38 PDT
*** Bug 133639 has been marked as a duplicate of this bug. ***
Comment 26 Kenneth Russell 2020-08-31 12:41:53 PDT
*** Bug 141240 has been marked as a duplicate of this bug. ***