Skip to content

A
angle
Commit 2f3f4141 by Luc Ferron Committed by Commit Bot
Options

Vulkan: Fix texture completeness issues

The fix is to skip updates that are queued that are not valid for the current image description. We keep these around in case they are used in a later usage of the same texture but we issue a warning telling the user that memory will be used indefinitely until they use that data. Bug: angleproject:2596 Change-Id: I8c20fffbd473ae8e2e9d2123a49b675b824a9bf6 Reviewed-on: https://chromium-review.googlesource.com/1078913Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Commit-Queue: Luc Ferron <lucferron@chromium.org>
parent 8ae09b0e
Showing with 143 additions and 20 deletions
src/libANGLE/renderer/vulkan/TextureVk.cpp
......@@ -83,6 +83,20 @@ void PixelBuffer::release(RendererVk *renderer)
mStagingBuffer.release(renderer);
}
void PixelBuffer::removeStagedUpdates(const gl::ImageIndex &index)
{
// Find any staged updates for this index and removes them from the pending list.
uint32_t levelIndex = static_cast<uint32_t>(index.getLevelIndex());
uint32_t layerIndex = static_cast<uint32_t>(index.getLayerIndex());
auto removeIfStatement = [levelIndex, layerIndex](SubresourceUpdate &update) {
return update.copyRegion.imageSubresource.mipLevel == levelIndex &&
update.copyRegion.imageSubresource.baseArrayLayer == layerIndex;
};
mSubresourceUpdates.erase(
std::remove_if(mSubresourceUpdates.begin(), mSubresourceUpdates.end(), removeIfStatement),
mSubresourceUpdates.end());
}
gl::Error PixelBuffer::stageSubresourceUpdate(ContextVk *contextVk,
const gl::ImageIndex &index,
const gl::Extents &extents,
......@@ -250,6 +264,7 @@ gl::Error PixelBuffer::allocate(RendererVk *renderer,
}
vk::Error PixelBuffer::flushUpdatesToImage(RendererVk *renderer,
uint32_t levelCount,
vk::ImageHelper *image,
vk::CommandBuffer *commandBuffer)
{
......@@ -260,10 +275,22 @@ vk::Error PixelBuffer::flushUpdatesToImage(RendererVk *renderer,
ANGLE_TRY(mStagingBuffer.flush(renderer->getDevice()));
std::vector<SubresourceUpdate> updatesToKeep;
for (const SubresourceUpdate &update : mSubresourceUpdates)
{
ASSERT(update.bufferHandle != VK_NULL_HANDLE);
const uint32_t updateMipLevel = update.copyRegion.imageSubresource.mipLevel;
// It's possible we've accumulated updates that are no longer applicable if the image has
// never been flushed but the image description has changed. Check if this level exist for
// this image.
if (updateMipLevel >= levelCount)
{
updatesToKeep.emplace_back(update);
continue;
}
// Conservatively flush all writes to the image. We could use a more restricted barrier.
// Do not move this above the for loop, otherwise multiple updates can have race conditions
// and not be applied correctly as seen i:
......@@ -276,8 +303,18 @@ vk::Error PixelBuffer::flushUpdatesToImage(RendererVk *renderer,
image->getCurrentLayout(), 1, &update.copyRegion);
}
mSubresourceUpdates.clear();
mStagingBuffer.releaseRetainedBuffers(renderer);
// Only remove the updates that were actually applied to the image.
mSubresourceUpdates = std::move(updatesToKeep);
if (mSubresourceUpdates.empty())
{
mStagingBuffer.releaseRetainedBuffers(renderer);
}
else
{
WARN() << "Internal Vulkan bufffer could not be released. This is likely due to having "
"extra images defined in the Texture.";
}
return vk::NoError();
}
......@@ -413,6 +450,10 @@ gl::Error TextureVk::setImage(const gl::Context *context,
ContextVk *contextVk = vk::GetImpl(context);
RendererVk *renderer = contextVk->getRenderer();
// If there is any staged changes for this index, we can remove them since we're going to
// override them with this call.
mPixelBuffer.removeStagedUpdates(index);
// Convert internalFormat to sized internal format.
const gl::InternalFormat &formatInfo = gl::GetInternalFormatInfo(internalFormat, type);
......@@ -720,6 +761,8 @@ gl::Error TextureVk::generateMipmapWithCPU(const gl::Context *context)
ANGLE_TRY(renderer->finish(context));
const uint32_t levelCount = getLevelCount();
// We now have the base level available to be manipulated in the baseLevelBuffer pointer.
// Generate all the missing mipmaps with the slow path. We can optimize with vkCmdBlitImage
// later.
......@@ -734,7 +777,7 @@ gl::Error TextureVk::generateMipmapWithCPU(const gl::Context *context)
sourceRowPitch, baseLevelBuffers + bufferOffset));
}
mPixelBuffer.flushUpdatesToImage(renderer, &mImage, commandBuffer);
mPixelBuffer.flushUpdatesToImage(renderer, levelCount, &mImage, commandBuffer);
return gl::NoError();
}
......@@ -828,18 +871,19 @@ vk::Error TextureVk::ensureImageInitialized(RendererVk *renderer)
vk::CommandBuffer *commandBuffer = nullptr;
ANGLE_TRY(getCommandBufferForWrite(renderer, &commandBuffer));
const gl::ImageDesc &baseLevelDesc = mState.getBaseLevelDesc();
const gl::Extents &baseLevelExtents = baseLevelDesc.size;
const uint32_t levelCount = getLevelCount();
if (!mImage.valid())
{
const gl::ImageDesc &baseLevelDesc = mState.getBaseLevelDesc();
const vk::Format &format =
renderer->getFormat(baseLevelDesc.format.info->sizedInternalFormat);
const gl::Extents &extents = baseLevelDesc.size;
const uint32_t levelCount = getLevelCount();
ANGLE_TRY(initImage(renderer, format, extents, levelCount, commandBuffer));
ANGLE_TRY(initImage(renderer, format, baseLevelExtents, levelCount, commandBuffer));
}
ANGLE_TRY(mPixelBuffer.flushUpdatesToImage(renderer, &mImage, commandBuffer));
ANGLE_TRY(mPixelBuffer.flushUpdatesToImage(renderer, levelCount, &mImage, commandBuffer));
return vk::NoError();
}
......
src/libANGLE/renderer/vulkan/TextureVk.h
......@@ -26,6 +26,8 @@ class PixelBuffer final : angle::NonCopyable
void release(RendererVk *renderer);
void removeStagedUpdates(const gl::ImageIndex &index);
gl::Error stageSubresourceUpdate(ContextVk *contextVk,
const gl::ImageIndex &index,
const gl::Extents &extents,
......@@ -60,6 +62,7 @@ class PixelBuffer final : angle::NonCopyable
bool *newBufferAllocatedOut);
vk::Error flushUpdatesToImage(RendererVk *renderer,
uint32_t levelCount,
vk::ImageHelper *image,
vk::CommandBuffer *commandBuffer);
......
src/tests/deqp_support/deqp_gles2_test_expectations.txt
......@@ -238,8 +238,6 @@
2593 VULKAN : dEQP-GLES2.functional.shaders.invariance.mediump.loop_4 = SKIP
2594 VULKAN : dEQP-GLES2.functional.shaders.fragdata.valid_static_index = SKIP
2595 VULKAN : dEQP-GLES2.functional.shaders.random.all_features.fragment* = SKIP
2596 VULKAN : dEQP-GLES2.functional.texture.completeness.2d.extra_level = SKIP
2596 VULKAN : dEQP-GLES2.functional.texture.completeness.cube.extra_level = SKIP
2597 VULKAN : dEQP-GLES2.functional.fbo.render.color_clear.* = SKIP
2597 VULKAN : dEQP-GLES2.functional.fbo.render.stencil_clear.tex2d_rgb_stencil_index8 = SKIP
2597 VULKAN : dEQP-GLES2.functional.fbo.render.color.* = SKIP
......
src/tests/gl_tests/MipmapTest.cpp
......@@ -60,8 +60,8 @@ class MipmapTest : public BaseMipmapTest
mTextureCube(0),
mLevelZeroBlueInitData(nullptr),
mLevelZeroWhiteInitData(nullptr),
mLevelOneInitData(nullptr),
mLevelTwoInitData(nullptr),
mLevelOneGreenInitData(nullptr),
mLevelTwoRedInitData(nullptr),
mOffscreenFramebuffer(0)
{
setWindowWidth(128);
......@@ -138,8 +138,10 @@ class MipmapTest : public BaseMipmapTest
mLevelZeroBlueInitData = createRGBInitData(getWindowWidth(), getWindowHeight(), 0, 0, 255); // Blue
mLevelZeroWhiteInitData = createRGBInitData(getWindowWidth(), getWindowHeight(), 255, 255, 255); // White
mLevelOneInitData = createRGBInitData((getWindowWidth() / 2), (getWindowHeight() / 2), 0, 255, 0); // Green
mLevelTwoInitData = createRGBInitData((getWindowWidth() / 4), (getWindowHeight() / 4), 255, 0, 0); // Red
mLevelOneGreenInitData =
createRGBInitData((getWindowWidth() / 2), (getWindowHeight() / 2), 0, 255, 0); // Green
mLevelTwoRedInitData =
createRGBInitData((getWindowWidth() / 4), (getWindowHeight() / 4), 255, 0, 0); // Red
glGenFramebuffers(1, &mOffscreenFramebuffer);
glGenTextures(1, &mTexture2D);
......@@ -177,8 +179,8 @@ class MipmapTest : public BaseMipmapTest
SafeDeleteArray(mLevelZeroBlueInitData);
SafeDeleteArray(mLevelZeroWhiteInitData);
SafeDeleteArray(mLevelOneInitData);
SafeDeleteArray(mLevelTwoInitData);
SafeDeleteArray(mLevelOneGreenInitData);
SafeDeleteArray(mLevelTwoRedInitData);
ANGLETest::TearDown();
}
......@@ -219,8 +221,8 @@ class MipmapTest : public BaseMipmapTest
GLubyte* mLevelZeroBlueInitData;
GLubyte* mLevelZeroWhiteInitData;
GLubyte* mLevelOneInitData;
GLubyte* mLevelTwoInitData;
GLubyte *mLevelOneGreenInitData;
GLubyte *mLevelTwoRedInitData;
private:
GLuint mOffscreenFramebuffer;
......@@ -444,7 +446,8 @@ TEST_P(MipmapTest, DISABLED_ThreeLevelsInitData)
}
// Pass in level one init data.
glTexImage2D(GL_TEXTURE_2D, 1, GL_RGB, getWindowWidth() / 2, getWindowHeight() / 2, 0, GL_RGB, GL_UNSIGNED_BYTE, mLevelOneInitData);
glTexImage2D(GL_TEXTURE_2D, 1, GL_RGB, getWindowWidth() / 2, getWindowHeight() / 2, 0, GL_RGB,
GL_UNSIGNED_BYTE, mLevelOneGreenInitData);
ASSERT_GL_NO_ERROR();
// Draw a full-sized quad, and check it's blue.
......@@ -467,7 +470,8 @@ TEST_P(MipmapTest, DISABLED_ThreeLevelsInitData)
EXPECT_PIXEL_COLOR_EQ(getWindowWidth() / 8, getWindowHeight() / 8, GLColor::black);
// Pass in level two init data.
glTexImage2D(GL_TEXTURE_2D, 2, GL_RGB, getWindowWidth() / 4, getWindowHeight() / 4, 0, GL_RGB, GL_UNSIGNED_BYTE, mLevelTwoInitData);
glTexImage2D(GL_TEXTURE_2D, 2, GL_RGB, getWindowWidth() / 4, getWindowHeight() / 4, 0, GL_RGB,
GL_UNSIGNED_BYTE, mLevelTwoRedInitData);
ASSERT_GL_NO_ERROR();
// Draw a full-sized quad, and check it's blue.
......@@ -660,6 +664,80 @@ TEST_P(MipmapTest, RenderOntoLevelZeroAfterGenerateMipmap)
EXPECT_PIXEL_COLOR_EQ(getWindowWidth() / 8, getWindowHeight() / 8, GLColor::green);
}
// This test defines a valid mipchain manually, with an extra level that's unused on the first few
// draws. Later on, it redefines the whole mipchain but this time, uses the last mip that was
// already uploaded before. The test expects that mip to be usable.
TEST_P(MipmapTest, DefineValidExtraLevelAndUseItLater)
{
glBindTexture(GL_TEXTURE_2D, mTexture2D);
GLubyte *levels[] = {mLevelZeroBlueInitData, mLevelOneGreenInitData, mLevelTwoRedInitData};
int maxLevel = 1 + floor(log2(std::max(getWindowWidth(), getWindowHeight())));
for (int i = 0; i < maxLevel; i++)
{
glTexImage2D(GL_TEXTURE_2D, i, GL_RGB, getWindowWidth() >> i, getWindowHeight() >> i, 0,
GL_RGB, GL_UNSIGNED_BYTE, levels[i % 3]);
}
// Define an extra level that won't be used for now
GLubyte *magentaExtraLevelData =
createRGBInitData(getWindowWidth() * 2, getWindowHeight() * 2, 255, 0, 255);
glTexImage2D(GL_TEXTURE_2D, maxLevel, GL_RGB, 1, 1, 0, GL_RGB, GL_UNSIGNED_BYTE,
magentaExtraLevelData);
ASSERT_GL_NO_ERROR();
// Enable mipmaps.
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR_MIPMAP_NEAREST);
// Draw a full-sized quad using mip 0, and check it's blue.
clearAndDrawQuad(m2DProgram, getWindowWidth(), getWindowHeight());
EXPECT_PIXEL_COLOR_EQ(getWindowWidth() / 2, getWindowHeight() / 2, GLColor::blue);
// Draw a full-sized quad using mip 1, and check it's green.
clearAndDrawQuad(m2DProgram, getWindowWidth() / 2, getWindowHeight() / 2);
EXPECT_PIXEL_COLOR_EQ(getWindowWidth() / 4, getWindowHeight() / 4, GLColor::green);
// Draw a full-sized quad using mip 2, and check it's red.
clearAndDrawQuad(m2DProgram, getWindowWidth() / 4, getWindowHeight() / 4);
EXPECT_PIXEL_COLOR_EQ(getWindowWidth() / 8, getWindowHeight() / 8, GLColor::red);
// Draw a full-sized quad using the last mip, and check it's green.
clearAndDrawQuad(m2DProgram, 1, 1);
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
// Now redefine everything above level 8 to be a mipcomplete chain again.
GLubyte *levelDoubleSizeYellowInitData =
createRGBInitData(getWindowWidth() * 2, getWindowHeight() * 2, 255, 255, 0);
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, getWindowWidth() * 2, getWindowHeight() * 2, 0, GL_RGB,
GL_UNSIGNED_BYTE, levelDoubleSizeYellowInitData); // 256
for (int i = 0; i < maxLevel - 1; i++)
{
glTexImage2D(GL_TEXTURE_2D, i + 1, GL_RGB, getWindowWidth() >> i, getWindowHeight() >> i, 0,
GL_RGB, GL_UNSIGNED_BYTE, levels[i % 3]);
}
// At this point we have a valid mip chain, the last level being magenta if we draw 1x1 pixel.
clearAndDrawQuad(m2DProgram, 1, 1);
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::magenta);
// Draw a full-sized quad using mip 0, and check it's yellow.
clearAndDrawQuad(m2DProgram, getWindowWidth() * 2, getWindowHeight() * 2);
EXPECT_PIXEL_COLOR_EQ(getWindowWidth() / 2, getWindowHeight() / 2, GLColor::yellow);
// Draw a full-sized quad using mip 1, and check it's blue.
clearAndDrawQuad(m2DProgram, getWindowWidth(), getWindowHeight());
EXPECT_PIXEL_COLOR_EQ(getWindowWidth() / 4, getWindowHeight() / 4, GLColor::blue);
// Draw a full-sized quad using mip 2, and check it's green.
clearAndDrawQuad(m2DProgram, getWindowWidth() / 2, getWindowHeight() / 2);
EXPECT_PIXEL_COLOR_EQ(getWindowWidth() / 8, getWindowHeight() / 8, GLColor::green);
}
// Regression test for a bug that cause mipmaps to only generate using the top left corner as input.
TEST_P(MipmapTest, MipMapGenerationD3D9Bug)
{
......
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