Replace imageAtomicExchange with imageAtomicCompSwap in vertex shader

Recently the test was changed in order to replace a imageAtomicAdd for
a imageAtomicExchange, as the test can't assume that the vertex shader
will be executed exactly one per vertex shader (see VK-GL-CTS issue
1910).

But imageAtomicExchange is also problematic because in some
architecture/driver, specially tile-based, the test can't also assume
that all the vertex shader executions would finish before
rasterization starts (so fragment shader execution). So when using
imageAtomicExchange could happend that a vertex shader execution would
override the value that the fragment shader already started to update.

Due this two restrictions, this patch changes the atomic operation on
the vertex shader to imageAtomicCompSwap, that sets the value of the
image at the vertex shader only if it is the initial value (so
zero). That would prevent any possible execution when the
rasterization already started to override the value of such image.

This patch also adds a comment on the code explaining the situation,
to avoid any possible future change to a simpler atomic operation by
mistake.

Components: OpenGL ES

VK-GL-CTS issue: 1997

Affects:
KHR-GLES31.core.shader_image_load_store.advanced-allStages-oneImage

Change-Id: Icbc34662f33839a1ced9f728cee45a1198e4b495

diff --git a/external/openglcts/modules/gles31/es31cShaderImageLoadStoreTests.cpp b/external/openglcts/modules/gles31/es31cShaderImageLoadStoreTests.cpp
index 5176bec..da55550 100644 (file)
--- a/external/openglcts/modules/gles31/es31cShaderImageLoadStoreTests.cpp
+++ b/external/openglcts/modules/gles31/es31cShaderImageLoadStoreTests.cpp
@@ -4175,10 +4175,28 @@ class AdvancedAllStagesOneImage : public ShaderImageLoadStoreBase
                const int kSize = 64;
                if (!IsVSFSAvailable(1, 1) || !IsImageAtomicSupported())
                        return NOT_SUPPORTED;
+                /* Note that we use imageAtomicCompSwap on the vertex
+                 * shader on purpose, for two reasons:
+                 *
+                 * * Test can't assume that the vertex shader will be
+                 *   executed exactly once per vertex. So the test
+                 *   can't use imageAtomicAdd as it is not possible to
+                 *   known in advance the final value (see khronos
+                 *   issue #1910)
+                 *
+                 * * Test can't assume that all the vertex shader
+                 *   executions will be executed before rasterization
+                 *   (so fragment shader) starts, specially on tile
+                 *   based GPUs. So the test can't use
+                 *   imageAtomicExchange, as it could happen that a
+                 *   vertex shader execution overrides the values
+                 *   being updated by the frament shader (see khronos
+                 *   issue #1997)
+                 */
                const char* const glsl_vs =
                        NL "layout(location = 0) in vec4 i_position;" NL
                           "layout(r32ui, binding = 3) coherent uniform uimage2D g_image;" NL "void main() {" NL
-                          "  gl_Position = i_position;" NL "  imageAtomicExchange(g_image, ivec2(0, gl_VertexID), 100u);" NL "}";
+                          "  gl_Position = i_position;" NL "  imageAtomicCompSwap(g_image, ivec2(0, gl_VertexID), 0u, 100u);" NL "}";
                const char* const glsl_fs =
                        NL "#define KSIZE 64" NL "layout(r32ui, binding = 3) coherent uniform uimage2D g_image;" NL
                           "void main() {" NL "  imageAtomicAdd(g_image, ivec2(0, int(gl_FragCoord.x) & 0x03), 0x1u);" NL "}";