Vulkan: Fix mixed descriptor pool updates.
When updating the Uniforms and Textures pools at different rates we
could trigger an ANGLE bug. We would sometimes reset and free the
currently bound descriptors. We can fix this by using separate
descriptor pools for each descriptor set.
Bug: angleproject:2678
Change-Id: I605b558531e7745484e16156a3af5eac40ffcc79
Reviewed-on: https://chromium-review.googlesource.com/1110662
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Luc Ferron <lucferron@chromium.org>
diff --git a/src/libANGLE/renderer/vulkan/ContextVk.cpp b/src/libANGLE/renderer/vulkan/ContextVk.cpp
index e94ad19..70d67ed 100644
--- a/src/libANGLE/renderer/vulkan/ContextVk.cpp
+++ b/src/libANGLE/renderer/vulkan/ContextVk.cpp
@@ -50,7 +50,6 @@
: ContextImpl(state),
mRenderer(renderer),
mCurrentDrawMode(gl::PrimitiveMode::InvalidEnum),
- mDynamicDescriptorPool(),
mTexturesDirty(false),
mVertexArrayBindingHasChanged(false),
mClearColorMask(kAllColorChannelsMask)
@@ -66,7 +65,11 @@
void ContextVk::onDestroy(const gl::Context *context)
{
mIncompleteTextures.onDestroy(context);
- mDynamicDescriptorPool.destroy(getDevice());
+
+ for (vk::DynamicDescriptorPool &descriptorPool : mDynamicDescriptorPools)
+ {
+ descriptorPool.destroy(getDevice());
+ }
}
gl::Error ContextVk::getIncompleteTexture(const gl::Context *context,
@@ -81,13 +84,20 @@
gl::Error ContextVk::initialize()
{
// Note that this may reserve more sets than strictly necessary for a particular layout.
- vk::DescriptorPoolSizes poolSizes;
- poolSizes.push_back({VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC,
- GetUniformBufferDescriptorCount() * vk::kDefaultDescriptorPoolMaxSets});
- poolSizes.push_back({VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER,
- mRenderer->getMaxActiveTextures() * vk::kDefaultDescriptorPoolMaxSets});
+ vk::DescriptorPoolSizes uniformPoolSize;
+ uniformPoolSize.push_back(
+ {VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC,
+ GetUniformBufferDescriptorCount() * vk::kDefaultDescriptorPoolMaxSets});
- ANGLE_TRY(mDynamicDescriptorPool.init(getDevice(), poolSizes));
+ vk::DescriptorPoolSizes imageSamplerPoolSize;
+ imageSamplerPoolSize.push_back(
+ {VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER,
+ mRenderer->getMaxActiveTextures() * vk::kDefaultDescriptorPoolMaxSets});
+
+ ANGLE_TRY(
+ mDynamicDescriptorPools[kUniformsDescriptorSetIndex].init(getDevice(), uniformPoolSize));
+ ANGLE_TRY(mDynamicDescriptorPools[kTextureDescriptorSetIndex].init(getDevice(),
+ imageSamplerPoolSize));
mPipelineDesc.reset(new vk::PipelineDesc());
mPipelineDesc->initDefaults();
@@ -766,9 +776,9 @@
return gl::InternalError();
}
-vk::DynamicDescriptorPool *ContextVk::getDynamicDescriptorPool()
+vk::DynamicDescriptorPool *ContextVk::getDynamicDescriptorPool(uint32_t descriptorSetIndex)
{
- return &mDynamicDescriptorPool;
+ return &mDynamicDescriptorPools[descriptorSetIndex];
}
const VkClearValue &ContextVk::getClearColorValue() const
diff --git a/src/libANGLE/renderer/vulkan/ContextVk.h b/src/libANGLE/renderer/vulkan/ContextVk.h
index a73f4c6..df4642e 100644
--- a/src/libANGLE/renderer/vulkan/ContextVk.h
+++ b/src/libANGLE/renderer/vulkan/ContextVk.h
@@ -154,7 +154,7 @@
void invalidateCurrentPipeline();
- vk::DynamicDescriptorPool *getDynamicDescriptorPool();
+ vk::DynamicDescriptorPool *getDynamicDescriptorPool(uint32_t descriptorSetIndex);
const VkClearValue &getClearColorValue() const;
const VkClearValue &getClearDepthStencilValue() const;
@@ -182,9 +182,9 @@
// Kept in a pointer so allocations can be aligned, and structs can be portably packed.
std::unique_ptr<vk::PipelineDesc> mPipelineDesc;
- // The dynamic descriptor pool is externally sychronized, so cannot be accessed from different
- // threads simultaneously. Hence, we keep it in the ContextVk instead of the RendererVk.
- vk::DynamicDescriptorPool mDynamicDescriptorPool;
+ // The descriptor pools are externally sychronized, so cannot be accessed from different
+ // threads simultaneously. Hence, we keep them in the ContextVk instead of the RendererVk.
+ vk::DescriptorSetLayoutArray<vk::DynamicDescriptorPool> mDynamicDescriptorPools;
// Triggers adding dependencies to the command graph.
bool mTexturesDirty;
diff --git a/src/libANGLE/renderer/vulkan/ProgramVk.cpp b/src/libANGLE/renderer/vulkan/ProgramVk.cpp
index 7278ade..a462dba 100644
--- a/src/libANGLE/renderer/vulkan/ProgramVk.cpp
+++ b/src/libANGLE/renderer/vulkan/ProgramVk.cpp
@@ -741,7 +741,8 @@
vk::Error ProgramVk::allocateDescriptorSet(ContextVk *contextVk, uint32_t descriptorSetIndex)
{
// Write out to a new a descriptor set.
- vk::DynamicDescriptorPool *dynamicDescriptorPool = contextVk->getDynamicDescriptorPool();
+ vk::DynamicDescriptorPool *dynamicDescriptorPool =
+ contextVk->getDynamicDescriptorPool(descriptorSetIndex);
uint32_t potentialNewCount = descriptorSetIndex + 1;
if (potentialNewCount > mDescriptorSets.size())
diff --git a/src/tests/gl_tests/VulkanUniformUpdatesTest.cpp b/src/tests/gl_tests/VulkanUniformUpdatesTest.cpp
index 9aba555..c018038 100644
--- a/src/tests/gl_tests/VulkanUniformUpdatesTest.cpp
+++ b/src/tests/gl_tests/VulkanUniformUpdatesTest.cpp
@@ -18,7 +18,6 @@
#include "libANGLE/angletypes.h"
#include "libANGLE/renderer/vulkan/ContextVk.h"
#include "libANGLE/renderer/vulkan/ProgramVk.h"
-#include "libANGLE/renderer/vulkan/RendererVk.h"
#include "test_utils/gl_raii.h"
using namespace angle;
@@ -29,7 +28,7 @@
class VulkanUniformUpdatesTest : public ANGLETest
{
protected:
- rx::ContextVk *hackANGLE()
+ rx::ContextVk *hackANGLE() const
{
// Hack the angle!
const gl::Context *context = static_cast<gl::Context *>(getEGLWindow()->getContext());
@@ -83,6 +82,15 @@
}
}
+void InitTexture(GLColor color, GLTexture *texture)
+{
+ const std::vector<GLColor> colors(4, color);
+ glBindTexture(GL_TEXTURE_2D, *texture);
+ glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 2, 2, 0, GL_RGBA, GL_UNSIGNED_BYTE, colors.data());
+ glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+ glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+}
+
// Force uniform updates until the dynamic descriptor pool wraps into a new pool allocation.
TEST_P(VulkanUniformUpdatesTest, DescriptorPoolUpdates)
{
@@ -90,7 +98,7 @@
// Force a small limit on the max sets per pool to more easily trigger a new allocation.
constexpr uint32_t kMaxSetsForTesting = 32;
- rx::vk::DynamicDescriptorPool *dynamicDescriptorPool = hackANGLE()->getDynamicDescriptorPool();
+ rx::vk::DynamicDescriptorPool *dynamicDescriptorPool = hackANGLE()->getDynamicDescriptorPool(0);
dynamicDescriptorPool->setMaxSetsPerPoolForTesting(kMaxSetsForTesting);
// Initialize texture program.
@@ -120,6 +128,89 @@
}
}
+// Uniform updates along with Texture updates.
+TEST_P(VulkanUniformUpdatesTest, DescriptorPoolUniformAndTextureUpdates)
+{
+ ASSERT_TRUE(IsVulkan());
+
+ // Force a small limit on the max sets per pool to more easily trigger a new allocation.
+ constexpr uint32_t kMaxSetsForTesting = 32;
+ rx::vk::DynamicDescriptorPool *uniformPool =
+ hackANGLE()->getDynamicDescriptorPool(rx::kUniformsDescriptorSetIndex);
+ uniformPool->setMaxSetsPerPoolForTesting(kMaxSetsForTesting);
+ rx::vk::DynamicDescriptorPool *texturePool =
+ hackANGLE()->getDynamicDescriptorPool(rx::kTextureDescriptorSetIndex);
+ texturePool->setMaxSetsPerPoolForTesting(kMaxSetsForTesting);
+
+ // Initialize texture program.
+ constexpr char kVS[] = R"(attribute vec2 position;
+varying mediump vec2 texCoord;
+void main()
+{
+ gl_Position = vec4(position, 0, 1);
+ texCoord = position * 0.5 + vec2(0.5);
+})";
+
+ constexpr char kFS[] = R"(varying mediump vec2 texCoord;
+uniform sampler2D tex;
+uniform mediump vec4 colorMask;
+void main()
+{
+ gl_FragColor = texture2D(tex, texCoord) * colorMask;
+})";
+
+ ANGLE_GL_PROGRAM(program, kVS, kFS);
+ glUseProgram(program);
+
+ // Get uniform locations.
+ GLint texLoc = glGetUniformLocation(program, "tex");
+ ASSERT_NE(-1, texLoc);
+
+ GLint colorMaskLoc = glGetUniformLocation(program, "colorMask");
+ ASSERT_NE(-1, colorMaskLoc);
+
+ // Initialize white texture.
+ GLTexture whiteTexture;
+ InitTexture(GLColor::white, &whiteTexture);
+ ASSERT_GL_NO_ERROR();
+
+ glActiveTexture(GL_TEXTURE0);
+ glBindTexture(GL_TEXTURE_2D, whiteTexture);
+
+ // Initialize magenta texture.
+ GLTexture magentaTexture;
+ InitTexture(GLColor::magenta, &magentaTexture);
+ ASSERT_GL_NO_ERROR();
+
+ glActiveTexture(GL_TEXTURE1);
+ glBindTexture(GL_TEXTURE_2D, magentaTexture);
+
+ // Draw multiple times, each iteration will create a new descriptor set.
+ for (uint32_t iteration = 0; iteration < kMaxSetsForTesting * 2; ++iteration)
+ {
+ // Draw with white.
+ glUniform1i(texLoc, 0);
+ glUniform4f(colorMaskLoc, 1.0f, 1.0f, 1.0f, 1.0f);
+ drawQuad(program, "position", 0.5f, 1.0f, true);
+
+ // Draw with white masking out red.
+ glUniform4f(colorMaskLoc, 0.0f, 1.0f, 1.0f, 1.0f);
+ drawQuad(program, "position", 0.5f, 1.0f, true);
+
+ // Draw with magenta.
+ glUniform1i(texLoc, 1);
+ glUniform4f(colorMaskLoc, 1.0f, 1.0f, 1.0f, 1.0f);
+ drawQuad(program, "position", 0.5f, 1.0f, true);
+
+ // Draw with magenta masking out red.
+ glUniform4f(colorMaskLoc, 0.0f, 1.0f, 1.0f, 1.0f);
+ drawQuad(program, "position", 0.5f, 1.0f, true);
+
+ swapBuffers();
+ ASSERT_GL_NO_ERROR();
+ }
+}
+
ANGLE_INSTANTIATE_TEST(VulkanUniformUpdatesTest, ES2_VULKAN());
} // anonymous namespace
