New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 643075 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

linkage.varing.rules.struct_type_mismatch_* fail on MacOSX

Project Member Reported by jie.a.c...@intel.com, Sep 1 2016

Issue description

The cause is that Mac compiler doesn't support varing for variables of struct type. It's buggy to add 'out' prior to any struct variable.


 
A basic idea to walk around it in ANGLE is to expand all fields in struct flat recursively like this:

diff --git a/sdk/tests/deqp/data/gles3/shaders/linkage.test b/sdk/tests/deqp/data/gles3/shaders/linkage.test
index 5becf60..28095bd 100644
--- a/sdk/tests/deqp/data/gles3/shaders/linkage.test
+++ b/sdk/tests/deqp/data/gles3/shaders/linkage.test
@@ -381,22 +381,30 @@ group varying "Varying linkage"
                                #version 300 es
                                ${VERTEX_DECLARATIONS}
                                struct S { highp float a; highp vec2 b; };
-                               out S var;
+                               out highp float S_var_a;
+                               out highp vec2 S_var_b;
+                               S var_impl;
                                void main()
                                {
-                                       var.a = 2.0;
-                                       var.b = vec2(1.0, 0.0);
+                                       var_impl.a = 2.0;
+                                       var_impl.b = vec2(1.0, 0.0);
                                        ${VERTEX_OUTPUT}
+                                       S_var_a = var_impl.a;
+                                       S_var_b = var_impl.b;
                                }
                        ""
                        fragment ""
                                #version 300 es
                                ${FRAGMENT_DECLARATIONS}
                                struct S { highp float a; highp vec3 b; };
-                               in S var;
+                               in highp float S_var_a;
+                               in highp vec3 S_var_b;
+                               S var_impl;
                                void main()
                                {
-                                       ${FRAG_COLOR} = vec4(var.a, var.b);
+                                       var_impl.a = S_var_a;
+                                       var_impl.b = S_var_b;
+                                       ${FRAG_COLOR} = vec4(var_impl.a, var_impl.b);
                                }
                        ""
                end


Is it worth a try?
According to Jie's investigation, this is a general MacOSX issue on all GPU vendors. Ken and/or Zhenyao, could you ping Apple guys (through Dean?) to resolve this issue?
 
If that is infeasible or too slow, Jie can take a look and try to workaround this issue in Angle. 

Comment 3 by zmo@chromium.org, Sep 1 2016

Nah, this test passes on my desktop Mac (AMD Radeon HD - FirePro D500), mac retina laptop (AMD), mac laptop with Intel HD 6000.

On which platforms you saw this failure?

The only failure I saw in linkage.html is differing_interpolation_2, which is due to "centroid" matching rule.
It's a fairly new MacBookPro we have just bought. And it has dual GPUs, Intel and AMD, which can be switched arbitrarily, and either of which can reproduce this failure.

Comment 5 by zmo@chromium.org, Sep 2 2016

Can you provide the about:gpu info?
Chrome version  Chrome/55.0.2847.0
Operating system        Mac OS X 10.11.6
Software rendering list version 11.10
Driver bug list version 8.90
ANGLE commit id d5da505da77f
2D graphics backend     Skia/55 13a7eee2504e7deb0e27ed3e69a787696d57b037
Command Line Args       --enable-unsafe-es3-apis --flag-switches-begin --flag-switches-end
Driver Information
Initialization time     71
In-process GPU  false
Sandboxed       true
GPU0    VENDOR = 0x1002, DEVICE= 0x6821 *ACTIVE*
GPU1    VENDOR = 0x8086, DEVICE= 0x0d26
Optimus false
AMD switchable  true
Driver vendor
Driver version  1.42.15
Driver date
Pixel shader version    4.10
Vertex shader version   4.10
Max. MSAA samples       8
Machine model name      MacBookPro
Machine model version   11.5
GL_VENDOR       ATI Technologies Inc.
GL_RENDERER     AMD Radeon R9 M370X OpenGL Engine
GL_VERSION      4.1 ATI-1.42.15
GL_EXTENSIONS   GL_ARB_blend_func_extended GL_ARB_draw_buffers_blend GL_ARB_draw_indirect GL_ARB_ES2_compatibility GL_ARB_explicit_attrib_location GL_ARB_gpu_shader_fp64 GL_ARB_gpu_shader5 GL_ARB_instanced_arrays GL_ARB_internalformat_query GL_ARB_occlusion_query2 GL_ARB_sample_shading GL_ARB_sampler_objects GL_ARB_separate_shader_objects GL_ARB_shader_bit_encoding GL_ARB_shader_subroutine GL_ARB_shading_language_include GL_ARB_tessellation_shader GL_ARB_texture_buffer_object_rgb32 GL_ARB_texture_cube_map_array GL_ARB_texture_gather GL_ARB_texture_query_lod GL_ARB_texture_rgb10_a2ui GL_ARB_texture_storage GL_ARB_texture_swizzle GL_ARB_timer_query GL_ARB_transform_feedback2 GL_ARB_transform_feedback3 GL_ARB_vertex_attrib_64bit GL_ARB_vertex_type_2_10_10_10_rev GL_ARB_viewport_array GL_EXT_debug_label GL_EXT_debug_marker GL_EXT_depth_bounds_test GL_EXT_texture_compression_s3tc GL_EXT_texture_filter_anisotropic GL_EXT_texture_mirror_clamp GL_EXT_texture_sRGB_decode GL_APPLE_client_storage GL_APPLE_container_object_shareable GL_APPLE_flush_render GL_APPLE_object_purgeable GL_APPLE_rgb_422 GL_APPLE_row_bytes GL_APPLE_texture_range GL_ATI_texture_mirror_once GL_NV_texture_barrier
Disabled Extensions
Window system binding vendor
Window system binding version
Window system binding extensions
Direct rendering        Yes
Reset notification strategy     0x0000
GPU process crash count 0
Compositor Information
Tile Update Mode        Zero-copy
Partial Raster  Enabled
GpuMemoryBuffers Status
ATC     Software only
ATCIA   Software only
DXT1    Software only
DXT5    Software only
ETC1    Software only
R_8     GPU_READ_CPU_READ_WRITE, GPU_READ_CPU_READ_WRITE_PERSISTENT
BGR_565 Software only
RGBA_4444       Software only
RGBX_8888       Software only
RGBA_8888       GPU_READ, SCANOUT
BGRX_8888       GPU_READ, SCANOUT
BGRA_8888       GPU_READ, SCANOUT, GPU_READ_CPU_READ_WRITE, GPU_READ_CPU_READ_WRITE_PERSISTENT
YVU_420 Software only
YUV_420_BIPLANAR        GPU_READ_CPU_READ_WRITE, GPU_READ_CPU_READ_WRITE_PERSISTENT
UYVY_422        GPU_READ_CPU_READ_WRITE, GPU_READ_CPU_READ_WRITE_PERSISTENT


Comment 7 by zmo@chromium.org, Sep 2 2016

I have the same laptop. In canary, the failure is as I described, but on ToT, it's what you saw.  It is a regression in Chrome, most likely in ANGLE shader translator.  Can you bisect (or debug) a little?

Comment 8 by zmo@chromium.org, Sep 2 2016

Labels: -Pri-3 Pri-2
Okay, what's the version number of your canary?

Comment 10 by zmo@chromium.org, Sep 2 2016

55.0.2846.4, the latest update
I have failed to bisect it. All builds were good. Actually I had found this issue over 2 weeks when I reported it. So it must be something wrong with my build. The major difference probably was 'debug' and 'release'. So I tried my release build of the same TOT, and it turned to be good. So far most likely it has been a debug build only bug.
The offending assert:

"err: uniqueId(139):     ! Assert failed in uniqueId(139): mUniqueId != 0

Assertion failed: (mUniqueId != 0), function uniqueId, file ../../third_party/angle/src/compiler/translator/Types.h, line 139."

It came from RegenerateStructName.
https://chromium.googlesource.com/angle/angle/+/master/src/compiler/translator/RegenerateStructNames.cpp#25

The TStructure without uniqueId was created while inserting initialization code for struct variables. See https://chromium.googlesource.com/angle/angle/+/master/src/compiler/translator/InitializeVariables.cpp#160
It looks we should have generated unique id for it here. There is another similar case https://chromium.googlesource.com/angle/angle/+/master/src/compiler/translator/Initialize.cpp#484
Should we enforce all such TStructure must have a valid unique id? I think it at least does no harm. 
Project Member

Comment 14 by bugdroid1@chromium.org, Sep 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/6a25746c016b87739606b3d669a565e84d567a80

commit 6a25746c016b87739606b3d669a565e84d567a80
Author: jchen10 <jie.a.chen@intel.com>
Date: Tue Sep 06 00:56:08 2016

Generate uniqueId for all TStructure

If not present, ASSERT error may happen in debug build.

BUG= chromium:643075 

Change-Id: Ia57e3771ab4d2861aefc04287fbbce85232f1f4d
Reviewed-on: https://chromium-review.googlesource.com/381315
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>

[modify] https://crrev.com/6a25746c016b87739606b3d669a565e84d567a80/src/compiler/translator/Types.h
[modify] https://crrev.com/6a25746c016b87739606b3d669a565e84d567a80/src/compiler/translator/ParseContext.cpp
[modify] https://crrev.com/6a25746c016b87739606b3d669a565e84d567a80/src/compiler/translator/Types.cpp

Status: Fixed (was: Assigned)
Project Member

Comment 16 by bugdroid1@chromium.org, Sep 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9bb4e60b43e03ec97398628839dab490c41170d1

commit 9bb4e60b43e03ec97398628839dab490c41170d1
Author: jmadill <jmadill@chromium.org>
Date: Wed Sep 07 02:35:51 2016

Roll ANGLE a4e6f07..5a7e20e

https://chromium.googlesource.com/angle/angle.git/+log/a4e6f07..5a7e20e

BUG= chromium:643075 , chromium:642227 , chromium:638323 , chromium:644057 , 351528 , chromium:593024 

TBR=geofflang@chromium.org

TEST=bots

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2312243003
Cr-Commit-Position: refs/heads/master@{#416828}

[modify] https://crrev.com/9bb4e60b43e03ec97398628839dab490c41170d1/DEPS

Sign in to add a comment