Bug 231490 – https://tankionline.com/play/ html5 engine not working: crashes. (Metal shader not working)

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

https://tankionline.com/play/ 
html5 engine not working: crashes. (Metal shader not working)

iPadOS 15 and macOS 11 webkit engine.

[Radar WebKit Bug Importer]

<rdar://problem/84078788>

[Kyle Piddington]

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.

[Kyle Piddington]

Created attachment 440847 [details]
Patch

[Kenneth Russell]

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+

[Dean Jackson]

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.

[Kyle Piddington]

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.

[Kyle Piddington]

Created attachment 440849 [details]
Patch

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.

[Kenneth Russell]

Comment on attachment 440849 [details]
Patch

The code just changed to add a new argument to RemoveInactiveInterfaceVariables. Is there a test covering this change?

[Kyle Piddington]

(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!

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE

[Kenneth Russell]

Comment on attachment 440849 [details]
Patch

Still r+ once the commit message is updated. Glad tests caught the problem with the first patch.

[Kyle Piddington]

Created attachment 441270 [details]
Patch for landing

[EWS]

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].

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

[Kenneth Russell]

Please file a new bug rather than modifying this old one. Thanks.

[Kimmo Kinnunen]

I think those errors are cross-origin resource loads failing.

[Kimmo Kinnunen]

*** Bug 233096 has been marked as a duplicate of this bug. ***