Bug 214393

Summary: [WebGL2] vector-dynamic-indexing is failing a subtest on iOS
Product: WebKit Reporter: Dean Jackson <dino>
Component: WebGLAssignee: Kimmo Kinnunen <kkinnunen>
Status: RESOLVED MOVED    
Severity: Normal CC: bfulgham, dino, jdarpinian, justin_fan, kbr, kkinnunen, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 223934    
Bug Blocks:    
Attachments:
Description Flags
Patch ews-feeder: commit-queue-

Dean Jackson
Reported 2020-07-15 19:51:36 PDT
The test webgl/2.0.0/conformance2/glsl3/vector-dynamic-indexing.html is failing on iOS. In particular, the part of the test that is: test: Index an inout parameter passed to an user-defined function with an index with side effects #version 300 es precision mediump float; out vec4 my_FragColor; uniform int u_zero; int sideEffectCounter = 0; void foo(inout float f) { float g = f + 2.5; modf(g, f); } int funcWithSideEffects() { sideEffectCounter++; return 2; } void main() { vec4 v = vec4(1.0, 2.0, 3.0, 4.0); foo(v[funcWithSideEffects()]); vec4 expected = vec4(1.0, 2.0, 5.0, 4.0); float f = 1.0 - distance(v, expected); if (sideEffectCounter != 1) { f = 0.0; } my_FragColor = vec4(1.0 - f, f, 0.0, 1.0); } It produces a red output, which indicates that the f value is always 0. It should be 1.
Attachments
Patch (129.47 KB, patch)
2021-09-06 05:59 PDT, Kimmo Kinnunen 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2020-07-15 19:51:46 PDT
<rdar://problem/65641563>
Dean Jackson
Comment 2 2020-07-15 19:52:12 PDT
This also fails on macOS 11 (Apple Silicon)
Dean Jackson
Comment 3 2020-07-15 20:01:49 PDT
I believe the sideEffectCounter variable is ending up with a value of 2 somehow.
Dean Jackson
Comment 4 2020-07-15 20:09:24 PDT
Yes. It gets set to 2. Weird. The translated shader source (on macOS apple silicon) is: #version 410 out vec4 _umy_FragColor; uniform int _uu_zero; int _usideEffectCounter = 0; void _ufoo(inout float _uf){ float _ug = (_uf + 2.5); modf(_ug, _uf); } int _ufuncWithSideEffects(){ (_usideEffectCounter++); return 2; } void main(){ (_umy_FragColor = vec4(0.0, 0.0, 0.0, 0.0)); vec4 _uv = vec4(1.0, 2.0, 3.0, 4.0); _ufoo(_uv[int(clamp(float(_ufuncWithSideEffects()), 0.0, float(3)))]); vec4 _uexpected = vec4(1.0, 2.0, 5.0, 4.0); float _uf = (1.0 - distance(_uv, _uexpected)); if ((_usideEffectCounter != 1)) { (_uf = 0.0); } (_umy_FragColor = vec4((1.0 - _uf), _uf, 0.0, 1.0)); } But even in this form the funcWithSideEffects() is only called once. For some reason it is being called twice in the iOS GL implementation.
Kenneth Russell
Comment 5 2020-07-16 12:53:31 PDT
Sounds like a bug in the GLSL compiler, perhaps in the implementation of the clamp operator when it's expanding out the AST. It sounds like it's duplicating the AST node which makes that call, rather than assigning its value to a temporary and referencing that temporary multiple times. Could you try modifying your ANGLE build to pass SH_CLAMP_WITH_USER_DEFINED_INT_CLAMP_FUNCTION for: ShArrayIndexClampingStrategy ArrayIndexClampingStrategy when passing the ShBuiltInResources to ANGLE's internal GLSL compiler instance? Does that work around the bug?
Kimmo Kinnunen
Comment 6 2021-09-06 04:40:08 PDT
Currently fixed in specific OpenGL driver versions. Currently iOS will use Metal, which does not have this bug. Will update the expectation in bug 223934
Kimmo Kinnunen
Comment 7 2021-09-06 05:59:11 PDT
Created attachment 437406 [details] Patch
Brent Fulgham
Comment 8 2022-02-10 19:48:30 PST
The fix for this issue was needed outside the WebKit project, therefore this is being resolved as 'Moved'. This should now be fixed in shipping software.
Note You need to log in before you can comment on or make changes to this bug.