Issue 5654 in angleproject: 'samplerBuffer' : Illegal use of reserved word

[ti… via monorail]

Status: Accepted
Owner: ----
CC: cnor...@google.com, ti...@google.com, jmad...@chromium.org
Components: CaptureReplay
OS: All
Priority: Medium
Renderer: Vulkan DesktopGL SwiftShader
Type: Defect

New issue 5654 by ti...@google.com: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654

Looking at Issue 3573: Vulkan: Texture buffers - it seems `EXT_texture_buffer` should be supported, but "Special Forces Group 2" is hitting a compilation error with one of it's shaders:

ANGLE : ERROR: 0:466: 'samplerBuffer' : Illegal use of reserved word
ANGLE : ERROR: 0:466: 'samplerBuffer' : syntax error

I'm able to recreate this with a simple test:

TEST_P(TextureBufferTestES31, SamplerBuffer)
{
ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_texture_buffer"));

constexpr char kVS[] =
"#version 310 es\n"
"in highp vec2 position;\n"
"uniform samplerBuffer buffer;\n"
"void main() {\n"
" gl_Position = vec4(position, 0, 1);\n"
"}";
constexpr char kFS[] =
"#version 310 es\n"
"uniform highp vec4 uniformColor;\n"
"out highp vec4 fragColor;"
"void main() {\n"
" fragColor = uniformColor;\n"
"}";

GLuint program = CompileProgram(kVS, kFS);
ASSERT_NE(0u, program);
}

Which results in:

shader compilation failed: ERROR: 0:3: 'samplerBuffer' : Illegal use of reserved word
ERROR: 0:3: 'samplerBuffer' : syntax error

../../src/tests/gl_tests/TextureTest.cpp:8677: Failure
Expected: (0u) != (program), actual: 0 vs 0
[ FAILED ] TextureBufferTestES31.SamplerBuffer/ES3_1_Vulkan, where GetParam() = ES3_1_Vulkan (550 ms)

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

[ti… via monorail]

Updates:
Owner: syou...@chromium.org

Comment #1 on issue 5654 by ti...@google.com: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c1

Shabi,

Since you're marked as the owner of Issue 3573, can you take the first pass at what may be going wrong here and any next steps?

[syous… via monorail]

Updates:
Owner: ----

Comment #2 on issue 5654 by syou...@chromium.org: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c2

This is a bug in the shader. It should use:

#extension GL_OES_texture_buffer : require

There is no ANGLE bug here AFAICT.

[ti… via monorail]

Comment #3 on issue 5654 by ti...@google.com: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c3

Thanks for the heads up where to look next. Updating the test to include that line does indeed fix the issue.

I've attached the app's shader that's failing. The interesting part is how it starts:

#version 310 es


#ifdef GL_EXT_texture_buffer
#extension GL_EXT_texture_buffer : enable

#endif

So they know they need the extension enabled, but GL_EXT_texture_buffer isn't being #define'ed. Updating the test to use GL_EXT_texture_buffer (rather than OES) passes just fine with the Vulkan back end (though the GLES backend has issues - it only like OES).

It appears we are exposing the extension correctly, but the app isn't enabling it correctly for some reason. Unfortunately, the frame that causes the issue moves around, so I'm not able to get a trace with RenderDoc to see what the shader looks like for the native driver (presumably the same, but with the extension enabled).

Attachments:
special_forces_group_2.shader.txt 29.5 KB

[ti… via monorail]

Updates:
Blocking: 5592

Comment #4 on issue 5654 by ti...@google.com: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c4

(No comment was entered for this change.)

[syous… via monorail]

Comment #5 on issue 5654 by syou...@chromium.org: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c5

Hilarious, what's the point of `#ifdef`ing the `#extension`, if you're not going to `#ifdef` the samplerBuffer?

That said, this is the wrong way of enabling extensions. The GL_EXT_texture_buffer token is not defined until the #extension does so, so this code never enables the extension.

The code should instead look something like this:

#extension GL_EXT_texture_buffer : enable

#ifdef GL_EXT_texture_buffer
// use samplerBuffer
#endif

If the extension is not there, GL_EXT_texture_buffer will not be defined. Can we contact the developer and let them know of the bug? For a start, they can just remove the #ifdef around the #extension.

[jmad… via monorail]

Comment #6 on issue 5654 by jmad...@chromium.org: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c6

Note that whatever the app does is correct and the spec should be warped to fit them. :) That's the unfortunate reality of Android.

[ti… via monorail]

Comment #7 on issue 5654 by ti...@google.com: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c7

Updating the renderer string to mimic Adreno using Ian's CL doesn't help; it's still missing the #define.

Ian's CL:
https://chromium-review.googlesource.com/c/angle/angle/+/2543505

I don't have a good way to prove that the shader being given to the native driver is "good", but I may be able to dump the shaders with a layer.

[ti… via monorail]

Comment #8 on issue 5654 by ti...@google.com: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c8

Cody pointed out that "Fortnite" is fixed by enabling exposeNonConformantExtensionsAndVersions:

adb shell setprop debug.angle.feature_overrides_enabled exposeNonConformantExtensionsAndVersions

This also fixes "Special Forces Group 2" by allowing the shader to compile successfully, even though the shader itself is the same (still missing the #define).

[syous… via monorail]

Comment #9 on issue 5654 by syou...@chromium.org: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c9

Probably with exposeNonConformantExtensionsAndVersions the shaders use #version 320 es

[ti… via monorail]

Comment #10 on issue 5654 by ti...@google.com: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c10

Same 310 as before, just no compilation error:

#version 310 es


#ifdef GL_EXT_texture_buffer
#extension GL_EXT_texture_buffer : enable

#endif

[ti… via monorail]

Comment #11 on issue 5654 by ti...@google.com: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c11

The block of code that's preventing GL_EXT_texture_buffer is:

bool HasTextureBufferSupport(const RendererVk *rendererVk)
{
...
const std::array<GLenum, 3> &optionalFormats2 = {
GL_RGB32F,
GL_RGB32I,
GL_RGB32UI,
};

for (GLenum formatGL : optionalFormats2)
{
if (!HasTexelBufferSupport(rendererVk, formatGL))
{
return false;
}
}

There's a workaround CL available here that changes this to log that the format isn't supported while still enabling the extension:
https://chromium-review.googlesource.com/c/angle/angle/+/2702838

I've also opened a Khronos issue to try and address this:
https://gitlab.khronos.org/vulkan/vulkan/-/issues/2532

[ti… via monorail]

Comment #12 on issue 5654 by ti...@google.com: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c12

Mohan has indicated that this is affecting PUBG also.

[ianel… via monorail]

Comment #13 on issue 5654 by ianel...@google.com: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c13

I tried that CL with PUBG Mobile, and the crash changed. When PUBG calls glLinkeProgram, ANGLE asserts:

FATAL: glslang_wrapper_utils.cpp:4829 (get): ! Assert failed in get (../../third_party/angle/src/libANGLE/renderer/glslang_wrapper_utils.cpp:4829): it != mData[shaderType].end()

[syous… via monorail]

Comment #14 on issue 5654 by syou...@chromium.org: 'samplerBuffer' : Illegal use of reserved word
https://bugs.chromium.org/p/angleproject/issues/detail?id=5654#c14

Mohan mentioned in another change they have a fix for that ASSERT in another situation Ian.