Fix the DispatchCompute error

src/libANGLE/renderer/d3d/d3d11/StateManager11.cpp

@@ -771,24 +771,19 @@ void StateManager11::setShaderResourceInternal(gl::ShaderType shaderType,
     {
         ID3D11DeviceContext *deviceContext = mRenderer->getDeviceContext();
         ID3D11ShaderResourceView *srvPtr   = srv ? srv->get() : nullptr;
-        switch (shaderType)
-        {
-            case gl::ShaderType::Vertex:
         if (srvPtr)
         {
             uintptr_t resource = reinterpret_cast<uintptr_t>(GetViewResource(srvPtr));
-                    unsetConflictingUAVs(gl::PipelineType::GraphicsPipeline,
-                                         gl::ShaderType::Compute, resource, nullptr);
+            unsetConflictingUAVs(gl::PipelineType::GraphicsPipeline, gl::ShaderType::Compute,
+                                 resource, nullptr);
         }
+
+        switch (shaderType)
+        {
+            case gl::ShaderType::Vertex:
                 deviceContext->VSSetShaderResources(resourceSlot, 1, &srvPtr);
                 break;
             case gl::ShaderType::Fragment:
-                if (srvPtr)
-                {
-                    uintptr_t resource = reinterpret_cast<uintptr_t>(GetViewResource(srvPtr));
-                    unsetConflictingUAVs(gl::PipelineType::GraphicsPipeline,
-                                         gl::ShaderType::Compute, resource, nullptr);
-                }
                 deviceContext->PSSetShaderResources(resourceSlot, 1, &srvPtr);
                 break;
             case gl::ShaderType::Compute:
@@ -824,6 +819,8 @@ void StateManager11::setUnorderedAccessViewInternal(gl::ShaderType shaderType,
                                  resource, nullptr, false);
             unsetConflictingSRVs(gl::PipelineType::ComputePipeline, gl::ShaderType::Fragment,
                                  resource, nullptr, false);
+            unsetConflictingSRVs(gl::PipelineType::ComputePipeline, gl::ShaderType::Compute,
+                                 resource, nullptr, false);
         }
         deviceContext->CSSetUnorderedAccessViews(resourceSlot, 1, &uavPtr, nullptr);
 
@@ -1782,82 +1779,6 @@ angle::Result StateManager11::onMakeCurrent(const gl::Context *context)
     return angle::Result::Continue;
 }
 
-angle::Result StateManager11::clearSRVs(gl::ShaderType shaderType,
-                                        size_t rangeStart,
-                                        size_t rangeEnd)
-{
-    if (rangeStart == rangeEnd)
-    {
-        return angle::Result::Continue;
-    }
-
-    auto *currentSRVs = getSRVCache(shaderType);
-    gl::Range<size_t> clearRange(rangeStart, std::min(rangeEnd, currentSRVs->highestUsed()));
-    if (clearRange.empty())
-    {
-        return angle::Result::Continue;
-    }
-
-    ID3D11DeviceContext *deviceContext = mRenderer->getDeviceContext();
-    switch (shaderType)
-    {
-        case gl::ShaderType::Vertex:
-            deviceContext->VSSetShaderResources(static_cast<unsigned int>(clearRange.low()),
-                                                static_cast<unsigned int>(clearRange.length()),
-                                                &mNullSRVs[0]);
-            break;
-        case gl::ShaderType::Fragment:
-            deviceContext->PSSetShaderResources(static_cast<unsigned int>(clearRange.low()),
-                                                static_cast<unsigned int>(clearRange.length()),
-                                                &mNullSRVs[0]);
-            break;
-        case gl::ShaderType::Compute:
-            deviceContext->CSSetShaderResources(static_cast<unsigned int>(clearRange.low()),
-                                                static_cast<unsigned int>(clearRange.length()),
-                                                &mNullSRVs[0]);
-            break;
-        default:
-            UNREACHABLE();
-            break;
-    }
-
-    for (size_t samplerIndex : clearRange)
-    {
-        currentSRVs->update(samplerIndex, nullptr);
-    }
-
-    return angle::Result::Continue;
-}
-
-angle::Result StateManager11::clearUAVs(gl::ShaderType shaderType,
-                                        size_t rangeStart,
-                                        size_t rangeEnd)
-{
-    ASSERT(shaderType == gl::ShaderType::Compute);
-    if (rangeStart == rangeEnd)
-    {
-        return angle::Result::Continue;
-    }
-
-    gl::Range<size_t> clearRange(rangeStart, std::min(rangeEnd, mCurComputeUAVs.highestUsed()));
-    if (clearRange.empty())
-    {
-        return angle::Result::Continue;
-    }
-
-    ID3D11DeviceContext *deviceContext = mRenderer->getDeviceContext();
-    deviceContext->CSSetUnorderedAccessViews(static_cast<unsigned int>(clearRange.low()),
-                                             static_cast<unsigned int>(clearRange.length()),
-                                             &mNullUAVs[0], nullptr);
-
-    for (size_t index : clearRange)
-    {
-        mCurComputeUAVs.update(index, nullptr);
-    }
-
-    return angle::Result::Continue;
-}
-
 void StateManager11::unsetConflictingView(gl::PipelineType pipeline,
                                           ID3D11View *view,
                                           bool isRenderTarget)
@@ -2848,15 +2769,6 @@ angle::Result StateManager11::applyTexturesForSRVs(const gl::Context *context,
         ANGLE_TRY(setTextureForImage(context, shaderType, readonlyImageIndex, true, imageUnit));
     }
 
-    size_t samplerCount = caps.maxShaderTextureImageUnits[shaderType];
-    size_t readonlyImageCount =
-        context->getClientVersion() >= gl::Version(3, 1) ? caps.maxImageUnits : 0;
-
-    // Samplers and readonly images share the SRVs here, their range is
-    // [0, max(samplerRange.high(), readonlyImageRange.high()).
-    ANGLE_TRY(clearSRVs(shaderType, std::max(samplerRange.high(), readonlyImageRange.high()),
-                        samplerCount + readonlyImageCount));
-
     return angle::Result::Continue;
 }
 
@@ -2882,18 +2794,11 @@ angle::Result StateManager11::applyTexturesForUAVs(const gl::Context *context,
         ANGLE_TRY(setTextureForImage(context, shaderType, imageIndex, false, imageUnit));
     }
 
-    size_t imageCount = caps.maxImageUnits;
-    ANGLE_TRY(clearUAVs(shaderType, imageRange.high(), imageCount));
-
     return angle::Result::Continue;
 }
 
 angle::Result StateManager11::syncTexturesForCompute(const gl::Context *context)
 {
-    // applyTexturesForUAVs must be earlier than applyTexturesForSRVs since we need to do clearUVAs
-    // before set resources to SRVs. Otherwise, it will report the following error:
-    // ID3D11DeviceContext::CSSetShaderResources: Resource being set to CS shader resource slot 0 is
-    // still bound on output! Forcing to NULL.
     ANGLE_TRY(applyTexturesForUAVs(context, gl::ShaderType::Compute));
     ANGLE_TRY(applyTexturesForSRVs(context, gl::ShaderType::Compute));
     return angle::Result::Continue;
@@ -2908,6 +2813,21 @@ angle::Result StateManager11::setTextureForImage(const gl::Context *context,
     TextureD3D *textureImpl = nullptr;
     if (!imageUnit.texture.get())
     {
+        // The texture is used in shader. However, there is no resource binding to it. We
+        // should clear the corresponding UAV/SRV in case the previous view type is a buffer not a
+        // texture. Otherwise, below error will be reported. The Unordered Access View dimension
+        // declared in the shader code (TEXTURE2D) does not match the view type bound to slot 0
+        // of the Compute Shader unit (BUFFER).
+        if (readonly)
+        {
+            setShaderResourceInternal<d3d11::ShaderResourceView>(type, static_cast<UINT>(index),
+                                                                 nullptr);
+        }
+        else
+        {
+            setUnorderedAccessViewInternal<d3d11::UnorderedAccessView>(
+                type, static_cast<UINT>(index), nullptr);
+        }
         return angle::Result::Continue;
     }
 
@@ -3718,9 +3638,20 @@ angle::Result StateManager11::syncShaderStorageBuffersForShader(const gl::Contex
          blockIndex++)
     {
         GLuint binding = program->getShaderStorageBlockBinding(static_cast<GLuint>(blockIndex));
+        const unsigned int registerIndex = mProgramD3D->getShaderStorageBufferRegisterIndex(
+            static_cast<GLuint>(blockIndex), shaderType);
+        // It means this block is active but not statically used.
+        if (registerIndex == GL_INVALID_INDEX)
+        {
+            continue;
+        }
         const auto &shaderStorageBuffer = glState.getIndexedShaderStorageBuffer(binding);
         if (shaderStorageBuffer.get() == nullptr)
         {
+            // We didn't see a driver error like atomic buffer did. But theoretically, the same
+            // thing should be done.
+            setUnorderedAccessViewInternal<d3d11::UnorderedAccessView>(shaderType, registerIndex,
+                                                                       nullptr);
             continue;
         }
 
@@ -3750,29 +3681,11 @@ angle::Result StateManager11::syncShaderStorageBuffersForShader(const gl::Contex
         ANGLE_TRY(bufferStorage->getRawUAVRange(context, shaderStorageBuffer.getOffset(), viewSize,
                                                 &uavPtr));
 
-        // We need to make sure that resource being set to UnorderedAccessView slot |registerIndex|
-        // is not bound on SRV.
-        if (uavPtr)
-        {
-            unsetConflictingView(gl::GetPipelineType(shaderType), uavPtr->get(), false);
-        }
-
-        const unsigned int registerIndex = mProgramD3D->getShaderStorageBufferRegisterIndex(
-            static_cast<GLuint>(blockIndex), shaderType);
-
-        // It means this block is active but not statically used.
-        if (registerIndex == GL_INVALID_INDEX)
-        {
-            continue;
-        }
-
         switch (shaderType)
         {
             case gl::ShaderType::Compute:
             {
-                ID3D11UnorderedAccessView *uav     = uavPtr->get();
-                ID3D11DeviceContext *deviceContext = mRenderer->getDeviceContext();
-                deviceContext->CSSetUnorderedAccessViews(registerIndex, 1, &uav, nullptr);
+                setUnorderedAccessViewInternal(shaderType, registerIndex, uavPtr);
                 break;
             }
 
@@ -3831,9 +3744,18 @@ angle::Result StateManager11::syncAtomicCounterBuffersForShader(const gl::Contex
     {
         GLuint binding     = atomicCounterBuffer.binding;
         const auto &buffer = glState.getIndexedAtomicCounterBuffer(binding);
-
+        const unsigned int registerIndex =
+            mProgramD3D->getAtomicCounterBufferRegisterIndex(binding, shaderType);
+        ASSERT(registerIndex != GL_INVALID_INDEX);
         if (buffer.get() == nullptr)
         {
+            // The atomic counter is used in shader. However, there is no buffer binding to it. We
+            // should clear the corresponding UAV in case the previous view type is a texture not a
+            // buffer. Otherwise, below error will be reported. The Unordered Access View dimension
+            // declared in the shader code (BUFFER) does not match the view type bound to slot 0
+            // of the Compute Shader unit (TEXTURE2D).
+            setUnorderedAccessViewInternal<d3d11::UnorderedAccessView>(shaderType, registerIndex,
+                                                                       nullptr);
             continue;
         }
 
@@ -3847,21 +3769,9 @@ angle::Result StateManager11::syncAtomicCounterBuffersForShader(const gl::Contex
         d3d11::UnorderedAccessView *uavPtr = nullptr;
         ANGLE_TRY(bufferStorage->getRawUAVRange(context, buffer.getOffset(), viewSize, &uavPtr));
 
-        // We need to make sure that resource being set to UnorderedAccessView slot |registerIndex|
-        // is not bound on SRV.
-        if (uavPtr)
-        {
-            unsetConflictingView(gl::GetPipelineType(shaderType), uavPtr->get(), false);
-        }
-
-        const unsigned int registerIndex =
-            mProgramD3D->getAtomicCounterBufferRegisterIndex(binding, shaderType);
-
         if (shaderType == gl::ShaderType::Compute)
         {
-            ID3D11UnorderedAccessView *uav     = uavPtr->get();
-            ID3D11DeviceContext *deviceContext = mRenderer->getDeviceContext();
-            deviceContext->CSSetUnorderedAccessViews(registerIndex, 1, &uav, nullptr);
+            setUnorderedAccessViewInternal(shaderType, registerIndex, uavPtr);
         }
         else
         {

src/libANGLE/renderer/d3d/d3d11/StateManager11.h

@@ -354,9 +354,6 @@ class StateManager11 final : angle::NonCopyable
                                      bool readonly,
                                      const gl::ImageUnit &imageUnit);
 
-    // Faster than calling setTexture a jillion times
-    angle::Result clearSRVs(gl::ShaderType shaderType, size_t rangeStart, size_t rangeEnd);
-    angle::Result clearUAVs(gl::ShaderType shaderType, size_t rangeStart, size_t rangeEnd);
     void handleMultiviewDrawFramebufferChange(const gl::Context *context);
 
     angle::Result syncCurrentValueAttribs(

src/tests/gl_tests/ComputeShaderTest.cpp

@@ -371,7 +371,7 @@ layout(rgba32ui) uniform highp writeonly uimage2D imageOut;
 void main()
 {
     uvec3 temp = gl_NumWorkGroups;
-    imageStore(imageOut, ivec2(0), uvec4(temp, 0u));
+    imageStore(imageOut, ivec2(gl_GlobalInvocationID.xy), uvec4(temp, 0u));
 })";
 
     ANGLE_GL_COMPUTE_PROGRAM(program, kCS);
@@ -381,6 +381,184 @@ void main()
     EXPECT_GL_NO_ERROR();
 }
 
+// That that bind UAV with type buffer to slot 0, then bind UAV with type image to slot 0, then
+// buffer again. The test runs well.
+TEST_P(ComputeShaderTest, BufferImageBuffer)
+{
+    // See http://anglebug.com/3536
+    ANGLE_SKIP_TEST_IF(IsOpenGL() && IsIntel() && IsWindows());
+
+    constexpr char kCS0[] = R"(#version 310 es
+layout(local_size_x=1, local_size_y=1, local_size_z=1) in;
+layout(binding = 0, offset = 4) uniform atomic_uint ac[2];
+void main()
+{
+    atomicCounterIncrement(ac[0]);
+    atomicCounterDecrement(ac[1]);
+})";
+
+    ANGLE_GL_COMPUTE_PROGRAM(program0, kCS0);
+    glUseProgram(program0);
+
+    unsigned int bufferData[3] = {11u, 4u, 4u};
+    GLBuffer atomicCounterBuffer;
+    glBindBuffer(GL_ATOMIC_COUNTER_BUFFER, atomicCounterBuffer);
+    glBufferData(GL_ATOMIC_COUNTER_BUFFER, sizeof(bufferData), bufferData, GL_STATIC_DRAW);
+
+    glBindBufferBase(GL_ATOMIC_COUNTER_BUFFER, 0, atomicCounterBuffer);
+
+    glDispatchCompute(1, 1, 1);
+    EXPECT_GL_NO_ERROR();
+
+    glMemoryBarrier(GL_ATOMIC_COUNTER_BARRIER_BIT);
+    void *mappedBuffer =
+        glMapBufferRange(GL_ATOMIC_COUNTER_BUFFER, 0, sizeof(GLuint) * 3, GL_MAP_READ_BIT);
+    memcpy(bufferData, mappedBuffer, sizeof(bufferData));
+    glUnmapBuffer(GL_ATOMIC_COUNTER_BUFFER);
+
+    EXPECT_EQ(11u, bufferData[0]);
+    EXPECT_EQ(5u, bufferData[1]);
+    EXPECT_EQ(3u, bufferData[2]);
+
+    constexpr char kCS1[] = R"(#version 310 es
+layout(local_size_x=4, local_size_y=3, local_size_z=2) in;
+layout(rgba32ui) uniform highp writeonly uimage2D imageOut;
+void main()
+{
+    uvec3 temp = gl_NumWorkGroups;
+    imageStore(imageOut, ivec2(gl_GlobalInvocationID.xy), uvec4(temp, 0u));
+})";
+
+    ANGLE_GL_COMPUTE_PROGRAM(program1, kCS1);
+
+    glUseProgram(program1);
+    glDispatchCompute(8, 4, 2);
+
+    glUseProgram(program0);
+    glDispatchCompute(1, 1, 1);
+    glMemoryBarrier(GL_ATOMIC_COUNTER_BARRIER_BIT);
+    mappedBuffer =
+        glMapBufferRange(GL_ATOMIC_COUNTER_BUFFER, 0, sizeof(GLuint) * 3, GL_MAP_READ_BIT);
+    memcpy(bufferData, mappedBuffer, sizeof(bufferData));
+    glUnmapBuffer(GL_ATOMIC_COUNTER_BUFFER);
+
+    EXPECT_EQ(11u, bufferData[0]);
+    EXPECT_EQ(6u, bufferData[1]);
+    EXPECT_EQ(2u, bufferData[2]);
+
+    EXPECT_GL_NO_ERROR();
+}
+
+// That that bind UAV with type image to slot 0, then bind UAV with type buffer to slot 0. The test
+// runs well.
+TEST_P(ComputeShaderTest, ImageAtomicCounterBuffer)
+{
+    constexpr char kCS0[] = R"(#version 310 es
+layout(local_size_x=1, local_size_y=1, local_size_z=1) in;
+layout(r32ui, binding = 0) writeonly uniform highp uimage2D uImage[2];
+void main()
+{
+    imageStore(uImage[0], ivec2(gl_LocalInvocationIndex, gl_WorkGroupID.x), uvec4(100, 0,
+0, 0));
+    imageStore(uImage[1], ivec2(gl_LocalInvocationIndex, gl_WorkGroupID.x), uvec4(100, 0,
+0, 0));
+})";
+
+    ANGLE_GL_COMPUTE_PROGRAM(program0, kCS0);
+    glUseProgram(program0);
+    int width = 1, height = 1;
+    GLuint inputValues[] = {200};
+    GLTexture mTexture[2];
+    glBindTexture(GL_TEXTURE_2D, mTexture[0]);
+    glTexStorage2D(GL_TEXTURE_2D, 1, GL_R32UI, width, height);
+    glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, width, height, GL_RED_INTEGER, GL_UNSIGNED_INT,
+                    inputValues);
+
+    glBindTexture(GL_TEXTURE_2D, mTexture[1]);
+    glTexStorage2D(GL_TEXTURE_2D, 1, GL_R32UI, width, height);
+    glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, width, height, GL_RED_INTEGER, GL_UNSIGNED_INT,
+                    inputValues);
+
+    glBindImageTexture(0, mTexture[0], 0, GL_FALSE, 0, GL_WRITE_ONLY, GL_R32UI);
+    glBindImageTexture(1, mTexture[1], 0, GL_FALSE, 0, GL_WRITE_ONLY, GL_R32UI);
+
+    glDispatchCompute(1, 1, 1);
+    EXPECT_GL_NO_ERROR();
+
+    constexpr char kCS1[] = R"(#version 310 es
+layout(local_size_x=1, local_size_y=1, local_size_z=1) in;
+layout(binding = 0, offset = 4) uniform atomic_uint ac[2];
+void main()
+{
+    atomicCounterIncrement(ac[0]);
+    atomicCounterDecrement(ac[1]);
+})";
+
+    ANGLE_GL_COMPUTE_PROGRAM(program1, kCS1);
+
+    glUseProgram(program1);
+    glDispatchCompute(1, 1, 1);
+    EXPECT_GL_NO_ERROR();
+}
+
+// That that bind UAV with type image to slot 0, then bind UAV with type buffer to slot 0. The test
+// runs well.
+TEST_P(ComputeShaderTest, ImageShaderStorageBuffer)
+{
+    constexpr char kCS0[] = R"(#version 310 es
+layout(local_size_x=1, local_size_y=1, local_size_z=1) in;
+layout(r32ui, binding = 0) writeonly uniform highp uimage2D uImage[2];
+void main()
+{
+    imageStore(uImage[0], ivec2(gl_LocalInvocationIndex, gl_WorkGroupID.x), uvec4(100, 0,
+0, 0));
+    imageStore(uImage[1], ivec2(gl_LocalInvocationIndex, gl_WorkGroupID.x), uvec4(100, 0,
+0, 0));
+})";
+
+    ANGLE_GL_COMPUTE_PROGRAM(program0, kCS0);
+    glUseProgram(program0);
+    int width = 1, height = 1;
+    GLuint inputValues[] = {200};
+    GLTexture mTexture[2];
+    glBindTexture(GL_TEXTURE_2D, mTexture[0]);
+    glTexStorage2D(GL_TEXTURE_2D, 1, GL_R32UI, width, height);
+    glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, width, height, GL_RED_INTEGER, GL_UNSIGNED_INT,
+                    inputValues);
+
+    glBindTexture(GL_TEXTURE_2D, mTexture[1]);
+    glTexStorage2D(GL_TEXTURE_2D, 1, GL_R32UI, width, height);
+    glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, width, height, GL_RED_INTEGER, GL_UNSIGNED_INT,
+                    inputValues);
+
+    glBindImageTexture(0, mTexture[0], 0, GL_FALSE, 0, GL_WRITE_ONLY, GL_R32UI);
+    glBindImageTexture(1, mTexture[1], 0, GL_FALSE, 0, GL_WRITE_ONLY, GL_R32UI);
+
+    glDispatchCompute(1, 1, 1);
+    EXPECT_GL_NO_ERROR();
+
+    constexpr char kCS1[] =
+        R"(#version 310 es
+ layout(local_size_x=1, local_size_y=1, local_size_z=1) in;
+ layout(std140, binding = 0) buffer blockOut {
+     uvec2 data;
+ } instanceOut;
+ layout(std140, binding = 1) buffer blockIn {
+     uvec2 data;
+ } instanceIn;
+ void main()
+ {
+     instanceOut.data = instanceIn.data;
+ }
+ )";
+
+    ANGLE_GL_COMPUTE_PROGRAM(program1, kCS1);
+
+    glUseProgram(program1);
+    glDispatchCompute(1, 1, 1);
+    EXPECT_GL_NO_ERROR();
+}
+
 // Basic test for DispatchComputeIndirect.
 TEST_P(ComputeShaderTest, DispatchComputeIndirect)
 {