Revert "Vulkan: readPixels improvement - use DynamicBuffer"
This reverts commit 82a420bb9af221e659b9510aed6791a87ac97b25.
Reason for revert: breaks readPixels on Android, probably because memory there is not host coherent by default.
Original change's description:
> Vulkan: readPixels improvement - use DynamicBuffer
>
> - Instead of using a new Staging image everytime, we now
> just use a DynamicBuffer that should be bit less wasteful.
>
> Bug: angleproject:2480
>
> Change-Id: I09edfb16f6ec49a0ca320d9e5dd9ce34e9dbf99e
> Reviewed-on: https://chromium-review.googlesource.com/1059452
> Commit-Queue: Luc Ferron <lucferron@chromium.org>
> Reviewed-by: Jamie Madill <jmadill@chromium.org>
> Reviewed-by: Geoff Lang <geofflang@chromium.org>
TBR=geofflang@chromium.org,jmadill@chromium.org,lucferron@chromium.org
Change-Id: Id7c493915ec7131f5202a7224267a79cd53007d2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: angleproject:2480
Reviewed-on: https://chromium-review.googlesource.com/1064770
Reviewed-by: Yuly Novikov <ynovikov@chromium.org>
Commit-Queue: Yuly Novikov <ynovikov@chromium.org>
diff --git a/src/libANGLE/renderer/vulkan/FramebufferVk.cpp b/src/libANGLE/renderer/vulkan/FramebufferVk.cpp
index e5cb11a..c1151a0 100644
--- a/src/libANGLE/renderer/vulkan/FramebufferVk.cpp
+++ b/src/libANGLE/renderer/vulkan/FramebufferVk.cpp
@@ -31,8 +31,6 @@
namespace
{
-constexpr size_t kMinReadPixelsBufferSize = 128000;
-
const gl::InternalFormat &GetReadAttachmentInfo(const gl::Context *context,
RenderTargetVk *renderTarget)
{
@@ -61,10 +59,8 @@
mRenderPassDesc(),
mFramebuffer(),
mActiveColorComponents(0),
- mMaskedClearDescriptorSet(VK_NULL_HANDLE),
- mReadPixelsBuffer(VK_BUFFER_USAGE_TRANSFER_DST_BIT, kMinReadPixelsBufferSize)
+ mMaskedClearDescriptorSet(VK_NULL_HANDLE)
{
- mReadPixelsBuffer.init(1);
}
FramebufferVk::FramebufferVk(const gl::FramebufferState &state, WindowSurfaceVk *backbuffer)
@@ -73,10 +69,8 @@
mRenderPassDesc(),
mFramebuffer(),
mActiveColorComponents(0),
- mMaskedClearDescriptorSet(VK_NULL_HANDLE),
- mReadPixelsBuffer(VK_BUFFER_USAGE_TRANSFER_DST_BIT, kMinReadPixelsBufferSize)
+ mMaskedClearDescriptorSet(VK_NULL_HANDLE)
{
- mReadPixelsBuffer.init(1);
}
FramebufferVk::~FramebufferVk()
@@ -85,13 +79,10 @@
void FramebufferVk::destroy(const gl::Context *context)
{
- ContextVk *contextVk = vk::GetImpl(context);
- RendererVk *renderer = contextVk->getRenderer();
+ RendererVk *renderer = vk::GetImpl(context)->getRenderer();
renderer->releaseResource(*this, &mFramebuffer);
renderer->releaseResource(*this, &mMaskedClearUniformBuffer.buffer);
renderer->releaseResource(*this, &mMaskedClearUniformBuffer.memory);
-
- mReadPixelsBuffer.destroy(contextVk->getDevice());
}
gl::Error FramebufferVk::discard(const gl::Context *context,
@@ -296,10 +287,8 @@
}
const gl::State &glState = context->getGLState();
- RendererVk *renderer = vk::GetImpl(context)->getRenderer();
-
- vk::CommandBuffer *commandBuffer = nullptr;
- ANGLE_TRY(beginWriteResource(renderer, &commandBuffer));
+ RenderTargetVk *renderTarget = getColorReadRenderTarget();
+ ASSERT(renderTarget);
const gl::PixelPackState &packState = context->getGLState().getPackState();
const gl::InternalFormat &sizedFormatInfo = gl::GetInternalFormatInfo(format, type);
@@ -316,18 +305,17 @@
(clippedArea.y - area.y) * outputPitch;
PackPixelsParams params;
- params.area = clippedArea;
+ params.area = area;
params.format = format;
params.type = type;
params.outputPitch = outputPitch;
params.packBuffer = glState.getTargetBuffer(gl::BufferBinding::PixelPack);
params.pack = glState.getPackState();
- ANGLE_TRY(ReadPixelsFromRenderTarget(context, clippedArea, params, mReadPixelsBuffer,
- getColorReadRenderTarget(), commandBuffer,
- reinterpret_cast<uint8_t *>(pixels) + outputSkipBytes));
- mReadPixelsBuffer.releaseRetainedBuffers(renderer);
- return gl::NoError();
+ vk::CommandBuffer *commandBuffer = nullptr;
+ ANGLE_TRY(beginWriteResource(vk::GetImpl(context)->getRenderer(), &commandBuffer));
+ return ReadPixelsFromRenderTarget(context, clippedArea, params, renderTarget, commandBuffer,
+ reinterpret_cast<uint8_t *>(pixels) + outputSkipBytes);
}
RenderTargetVk *FramebufferVk::getColorReadRenderTarget()
@@ -814,52 +802,48 @@
gl::Error ReadPixelsFromRenderTarget(const gl::Context *context,
const gl::Rectangle &area,
const PackPixelsParams &packPixelsParams,
- vk::DynamicBuffer &dynamicBuffer,
RenderTargetVk *renderTarget,
vk::CommandBuffer *commandBuffer,
void *pixels)
{
- RendererVk *renderer = vk::GetImpl(context)->getRenderer();
+ ContextVk *contextVk = vk::GetImpl(context);
+ RendererVk *renderer = contextVk->getRenderer();
+ VkDevice device = renderer->getDevice();
- vk::ImageHelper *renderTargetImage = renderTarget->image;
- const angle::Format &angleFormat = renderTargetImage->getFormat().textureFormat();
- VkBuffer bufferHandle = VK_NULL_HANDLE;
- uint8_t *readPixelBuffer = nullptr;
- bool newBufferAllocated = false;
- uint32_t stagingOffset = 0;
- size_t allocationSize = area.width * angleFormat.pixelBytes * area.height;
+ vk::ImageHelper stagingImage;
+ ANGLE_TRY(stagingImage.init2DStaging(
+ device, renderer->getMemoryProperties(), renderTarget->image->getFormat(),
+ gl::Extents(area.width, area.height, 1), vk::StagingUsage::Read));
- dynamicBuffer.allocate(renderer, allocationSize, &readPixelBuffer, &bufferHandle,
- &stagingOffset, &newBufferAllocated);
+ stagingImage.changeLayoutWithStages(VK_IMAGE_ASPECT_COLOR_BIT, VK_IMAGE_LAYOUT_GENERAL,
+ VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
+ VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, commandBuffer);
- VkBufferImageCopy region;
- region.bufferImageHeight = area.height;
- region.bufferOffset = static_cast<VkDeviceSize>(stagingOffset);
- region.bufferRowLength = area.width;
- region.imageExtent.width = area.width;
- region.imageExtent.height = area.height;
- region.imageExtent.depth = 1;
- region.imageOffset.x = area.x;
- region.imageOffset.y = area.y;
- region.imageOffset.z = 0;
- region.imageSubresource.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
- region.imageSubresource.baseArrayLayer = 0;
- region.imageSubresource.layerCount = 1;
- region.imageSubresource.mipLevel = 0;
+ vk::ImageHelper::Copy(renderTarget->image, &stagingImage, gl::Offset(area.x, area.y, 0),
+ gl::Offset(), gl::Extents(area.width, area.height, 1),
+ VK_IMAGE_ASPECT_COLOR_BIT, commandBuffer);
- renderTargetImage->changeLayoutWithStages(
- VK_IMAGE_ASPECT_COLOR_BIT, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
- VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, commandBuffer);
-
- commandBuffer->copyImageToBuffer(renderTargetImage->getImage(),
- renderTargetImage->getCurrentLayout(), bufferHandle, 1,
- ®ion);
// Triggers a full finish.
// TODO(jmadill): Don't block on asynchronous readback.
ANGLE_TRY(renderer->finish(context));
- PackPixels(packPixelsParams, angleFormat, area.width * angleFormat.pixelBytes, readPixelBuffer,
- reinterpret_cast<uint8_t *>(pixels));
+ // TODO(jmadill): parameters
+ uint8_t *mapPointer = nullptr;
+ ANGLE_TRY(stagingImage.getDeviceMemory().map(device, 0, stagingImage.getAllocatedMemorySize(),
+ 0, &mapPointer));
+
+ const angle::Format &angleFormat = renderTarget->image->getFormat().textureFormat();
+
+ // Get the staging image pitch and use it to pack the pixels later.
+ VkSubresourceLayout subresourceLayout;
+ stagingImage.getImage().getSubresourceLayout(device, VK_IMAGE_ASPECT_COLOR_BIT, 0, 0,
+ &subresourceLayout);
+
+ PackPixels(packPixelsParams, angleFormat, static_cast<int>(subresourceLayout.rowPitch),
+ mapPointer, reinterpret_cast<uint8_t *>(pixels));
+
+ stagingImage.getDeviceMemory().unmap(device);
+ renderer->releaseObject(renderer->getCurrentQueueSerial(), &stagingImage);
return vk::NoError();
}