Bug 231490 - https://tankionline.com/play/ html5 engine not working: crashes. (Metal shader not working)
Summary: https://tankionline.com/play/ html5 engine not working: crashes. (Metal shade...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: Safari 15
Hardware: All iOS 14
: P2 Critical
Assignee: Kyle Piddington
URL: https://tankionline.com/play
Keywords: InRadar
: 233096 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-10-10 11:42 PDT by starfleetcaptain1927
Modified: 2022-03-04 00:36 PST (History)
13 users (show)

See Also:


Attachments
Patch (8.17 KB, patch)
2021-10-11 15:20 PDT, Kyle Piddington
no flags Details | Formatted Diff | Diff
Patch (13.12 KB, patch)
2021-10-11 15:56 PDT, Kyle Piddington
no flags Details | Formatted Diff | Diff
Patch for landing (13.03 KB, patch)
2021-10-14 13:34 PDT, Kyle Piddington
no flags Details | Formatted Diff | Diff
Hi sorry new bug :) when trying to join a battle it throws this error. The ID changes every few seconds. Probably the 3D rendering resources (1.10 MB, image/png)
2021-10-14 16:44 PDT, starfleetcaptain1927
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description starfleetcaptain1927 2021-10-10 11:42:22 PDT
https://tankionline.com/play/ 
html5 engine not working: crashes. (Metal shader not working)

iPadOS 15 and macOS 11 webkit engine.
Comment 1 Radar WebKit Bug Importer 2021-10-10 13:20:53 PDT
<rdar://problem/84078788>
Comment 2 Kyle Piddington 2021-10-11 10:42:05 PDT
Hello! Thank you for reporting this.

The issue we're having is caused by an unused output variable. The shader in question:
//
// #version 300 es
// 			#ifdef GL_FRAGMENT_PRECISION_HIGH
// 				precision highp float;
// 				precision highp int;
// 			#else
// 				precision mediump float;
// 				precision mediump int;
// 			#endif
// 			
// 			#define varying in
// 			#define texture2D texture
// 			#define textureCube texture
// 			
// 			out vec4 outColor0;
// 			#define gl_FragColor outColor0
// 			
// 
// 			void main() {
// 			}
// 		

Doesn't actually do anything with outColor0. However, we still generated an initialization instruction to zero it out, despite not including this output variable from the shader. 

Thanks for catching this, it's a real bug and we should fix it up.
Comment 3 Kyle Piddington 2021-10-11 15:20:54 PDT
Created attachment 440847 [details]
Patch
Comment 4 Kenneth Russell 2021-10-11 15:24:02 PDT
Comment on attachment 440847 [details]
Patch

Looks good to me assuming all tests pass. Would you be able to put up the new test as a pull request against the WebGL conformance suite? r+
Comment 5 Dean Jackson 2021-10-11 15:25:08 PDT
Comment on attachment 440847 [details]
Patch

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

> LayoutTests/webgl/pending/resources/webgl_test_files/conformance2/glsl3/empty-shader-with-output.html:27
> +out vec4 outColor0;
> +#define gl_FragColor outColor0

I don't understand this.
Comment 6 Kyle Piddington 2021-10-11 15:25:41 PDT
Comment on attachment 440847 [details]
Patch

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

>> LayoutTests/webgl/pending/resources/webgl_test_files/conformance2/glsl3/empty-shader-with-output.html:27
>> +#define gl_FragColor outColor0
> 
> I don't understand this.

We can probably drop this, it was directly from the shader in this bug.
Comment 7 Kyle Piddington 2021-10-11 15:56:27 PDT
Created attachment 440849 [details]
Patch
Comment 8 starfleetcaptain1927 2021-10-11 15:59:22 PDT
Glad to help! You guys are awesome :)

(In reply to Kyle Piddington from comment #2)
> Hello! Thank you for reporting this.
> 
> The issue we're having is caused by an unused output variable. The shader in
> question:
> //
> // #version 300 es
> // 			#ifdef GL_FRAGMENT_PRECISION_HIGH
> // 				precision highp float;
> // 				precision highp int;
> // 			#else
> // 				precision mediump float;
> // 				precision mediump int;
> // 			#endif
> // 			
> // 			#define varying in
> // 			#define texture2D texture
> // 			#define textureCube texture
> // 			
> // 			out vec4 outColor0;
> // 			#define gl_FragColor outColor0
> // 			
> // 
> // 			void main() {
> // 			}
> // 		
> 
> Doesn't actually do anything with outColor0. However, we still generated an
> initialization instruction to zero it out, despite not including this output
> variable from the shader. 
> 
> Thanks for catching this, it's a real bug and we should fix it up.
Comment 9 Kenneth Russell 2021-10-11 16:16:49 PDT
Comment on attachment 440849 [details]
Patch

The code just changed to add a new argument to RemoveInactiveInterfaceVariables. Is there a test covering this change?
Comment 10 Kyle Piddington 2021-10-11 16:18:26 PDT
(In reply to Kenneth Russell from comment #9)
> Comment on attachment 440849 [details]
> Patch
> 
> The code just changed to add a new argument to
> RemoveInactiveInterfaceVariables. Is there a test covering this change?

Hey Ken,

This new test should still cover this change, as well as running our existing test suite. Moving RemoveInactiveInterfaceVariables to after rewritePipelines broke some existing tests, hence the new updates. 

Though you remind me that I need to update the changelog as well!
Comment 11 EWS Watchlist 2021-10-11 16:44:23 PDT
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Comment 12 Kenneth Russell 2021-10-11 22:36:01 PDT
Comment on attachment 440849 [details]
Patch

Still r+ once the commit message is updated. Glad tests caught the problem with the first patch.
Comment 13 Kyle Piddington 2021-10-14 13:34:30 PDT
Created attachment 441270 [details]
Patch for landing
Comment 14 EWS 2021-10-14 14:14:34 PDT
Committed r284197 (243011@main): <https://commits.webkit.org/243011@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 441270 [details].
Comment 15 starfleetcaptain1927 2021-10-14 16:44:23 PDT
Created attachment 441306 [details]
Hi sorry new bug :) when trying to join a battle it throws this error. The ID changes every few seconds. Probably the 3D rendering resources

Hi sorry new bug :) when trying to join a battle it throws this error. The ID changes every few seconds. Probably the 3D rendering resources
Comment 16 Kenneth Russell 2021-10-14 16:50:00 PDT
Please file a new bug rather than modifying this old one. Thanks.
Comment 17 Kimmo Kinnunen 2021-10-28 00:28:27 PDT
I think those errors are cross-origin resource loads failing.
Comment 18 Kimmo Kinnunen 2021-11-15 00:21:52 PST
*** Bug 233096 has been marked as a duplicate of this bug. ***