Commit edeaa835 by Jamie Madill Committed by Commit Bot

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: 's avatarLuc Ferron <lucferron@chromium.org>
parent caaff169
......@@ -50,7 +50,6 @@ ContextVk::ContextVk(const gl::ContextState &state, RendererVk *renderer)
: ContextImpl(state),
mRenderer(renderer),
mCurrentDrawMode(gl::PrimitiveMode::InvalidEnum),
mDynamicDescriptorPool(),
mTexturesDirty(false),
mVertexArrayBindingHasChanged(false),
mClearColorMask(kAllColorChannelsMask)
......@@ -66,7 +65,11 @@ ContextVk::~ContextVk()
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::getIncompleteTexture(const gl::Context *context,
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});
vk::DescriptorPoolSizes imageSamplerPoolSize;
imageSamplerPoolSize.push_back(
{VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER,
mRenderer->getMaxActiveTextures() * vk::kDefaultDescriptorPoolMaxSets});
ANGLE_TRY(mDynamicDescriptorPool.init(getDevice(), poolSizes));
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 @@ gl::Error ContextVk::memoryBarrierByRegion(const gl::Context *context, GLbitfiel
return gl::InternalError();
}
vk::DynamicDescriptorPool *ContextVk::getDynamicDescriptorPool()
vk::DynamicDescriptorPool *ContextVk::getDynamicDescriptorPool(uint32_t descriptorSetIndex)
{
return &mDynamicDescriptorPool;
return &mDynamicDescriptorPools[descriptorSetIndex];
}
const VkClearValue &ContextVk::getClearColorValue() const
......
......@@ -154,7 +154,7 @@ class ContextVk : public ContextImpl
void invalidateCurrentPipeline();
vk::DynamicDescriptorPool *getDynamicDescriptorPool();
vk::DynamicDescriptorPool *getDynamicDescriptorPool(uint32_t descriptorSetIndex);
const VkClearValue &getClearColorValue() const;
const VkClearValue &getClearDepthStencilValue() const;
......@@ -182,9 +182,9 @@ class ContextVk : public ContextImpl
// 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;
......
......@@ -741,7 +741,8 @@ Serial ProgramVk::getFragmentModuleSerial() const
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())
......
......@@ -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 @@ namespace
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 main()
}
}
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 @@ TEST_P(VulkanUniformUpdatesTest, DescriptorPoolUpdates)
// 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 @@ TEST_P(VulkanUniformUpdatesTest, DescriptorPoolUpdates)
}
}
// 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
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment