Skip to content

Commit

Permalink
Vulkan: improve the ReadPixels implementation.
Browse files Browse the repository at this point in the history 
This adds support for more format conversions and removes a bogus
assert that prevented ReadPixels within beginFrame / endFrame.
This was tested with:

    backend_test_mac --api vulkan --gtest_filter=BackendTest.ReadPixels
  • Loading branch information
@prideout
prideout committed Oct 21, 2020
1 parent 239246e commit 548b28c
Show file tree
Hide file tree
Showing 8 changed files with 245 additions and 71 deletions.
128 changes: 105 additions & 23 deletions filament/backend/src/DataReshaper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,24 @@
namespace filament {
namespace backend {

// This little utility adds padding to multi-channel interleaved data by inserting dummy values, or
// discards trailing channels. This is useful for platforms that only accept 4-component data, since
// users often wish to submit (or receive) 3-component data.
// Provides an alpha value when expanding 3-channel images to 4-channel.
// Also used as a normalization scale when converting between numeric types.
template<typename componentType> inline componentType getMaxValue();

class DataReshaper {
public:
template<typename componentType, size_t srcChannelCount, size_t dstChannelCount,
componentType maxValue = std::numeric_limits<componentType>::max()>

// Adds padding to multi-channel interleaved data by inserting dummy values, or discards
// trailing channels. This is useful for platforms that only accept 4-component data, since
// users often wish to submit (or receive) 3-component data.
template<typename componentType, size_t srcChannelCount, size_t dstChannelCount>
static void reshape(void* dest, const void* src, size_t numSrcBytes) {
const componentType maxValue = getMaxValue<componentType>();
const componentType* in = (const componentType*) src;
componentType* out = (componentType*) dest;
const size_t srcWordCount = (numSrcBytes / sizeof(componentType)) / srcChannelCount;
const size_t width = (numSrcBytes / sizeof(componentType)) / srcChannelCount;
const int minChannelCount = filament::math::min(srcChannelCount, dstChannelCount);
for (size_t word = 0; word < srcWordCount; ++word) {
for (size_t column = 0; column < width; ++column) {
for (size_t channel = 0; channel < minChannelCount; ++channel) {
out[channel] = in[channel];
}
Expand All @@ -48,37 +53,114 @@ class DataReshaper {
}
}

template<typename componentType, size_t srcChannelCount, size_t dstChannelCount,
componentType maxValue = std::numeric_limits<componentType>::max()>
static void reshapeImage(uint8_t* dest, const uint8_t* src, size_t srcBytesPerRow,
size_t dstBytesPerRow, size_t height, bool swizzle03) {
const size_t srcWordCount = (srcBytesPerRow / sizeof(componentType)) / srcChannelCount;
const int minChannelCount = filament::math::min(srcChannelCount, dstChannelCount);
// Converts a 4-channel image of UBYTE, INT, UINT, or FLOAT to a different type.
template<typename dstComponentType, typename srcComponentType>
static void reshapeImage(uint8_t* dest, const uint8_t* src, size_t srcBytesPerRow,
size_t dstBytesPerRow, size_t dstChannelCount, size_t height, bool swizzle, bool flip) {
const size_t srcChannelCount = 4;
const dstComponentType dstMaxValue = getMaxValue<dstComponentType>();
const srcComponentType srcMaxValue = getMaxValue<srcComponentType>();
const size_t width = (srcBytesPerRow / sizeof(srcComponentType)) / srcChannelCount;
const size_t minChannelCount = filament::math::min(srcChannelCount, dstChannelCount);
assert(minChannelCount <= 4);
int inds[4] = {0, 1, 2, 3};
if (swizzle03) {
inds[0] = 2;
inds[2] = 0;
const int inds[4] = {swizzle ? 2 : 0, 1, swizzle ? 0 : 2, 3};

int srcStride;
if (flip) {
src += srcBytesPerRow * (height - 1);
srcStride = -srcBytesPerRow;
} else {
srcStride = srcBytesPerRow;
}

for (size_t row = 0; row < height; ++row) {
const componentType* in = (const componentType*) src;
componentType* out = (componentType*) dest;
for (size_t word = 0; word < srcWordCount; ++word) {
const srcComponentType* in = (const srcComponentType*) src;
dstComponentType* out = (dstComponentType*) dest;
for (size_t column = 0; column < width; ++column) {
for (size_t channel = 0; channel < minChannelCount; ++channel) {
out[channel] = in[inds[channel]];
out[channel] = in[inds[channel]] * dstMaxValue / srcMaxValue;
}
for (size_t channel = srcChannelCount; channel < dstChannelCount; ++channel) {
out[channel] = maxValue;
out[channel] = dstMaxValue;
}
in += srcChannelCount;
out += dstChannelCount;
}
src += srcBytesPerRow;
src += srcStride;
dest += dstBytesPerRow;
}
}

// Converts a 4-channel image of UBYTE, INT, UINT, or FLOAT to a different type.
static bool reshapeImage(PixelBufferDescriptor* dst, PixelDataType srcType,
const uint8_t* srcBytes, int srcBytesPerRow, int width, int height, bool swizzle,
bool flip) {
size_t dstChannelCount;
switch (dst->format) {
case PixelDataFormat::RGB: dstChannelCount = 3; break;
case PixelDataFormat::RGBA: dstChannelCount = 4; break;
default: return false;
}
void (*reshaper)(uint8_t*, const uint8_t*, size_t, size_t, size_t, size_t, bool, bool)
= nullptr;
constexpr auto UBYTE = PixelDataType::UBYTE, FLOAT = PixelDataType::FLOAT,
UINT = PixelDataType::UINT, INT = PixelDataType::INT;
switch (dst->type) {
case UBYTE:
switch (srcType) {
case UBYTE: reshaper = reshapeImage<uint8_t, uint8_t>; break;
case FLOAT: reshaper = reshapeImage<uint8_t, float>; break;
case INT: reshaper = reshapeImage<uint8_t, int32_t>; break;
case UINT: reshaper = reshapeImage<uint8_t, uint32_t>; break;
default: return false;
}
break;
case FLOAT:
switch (srcType) {
case UBYTE: reshaper = reshapeImage<float, uint8_t>; break;
case FLOAT: reshaper = reshapeImage<float, float>; break;
case INT: reshaper = reshapeImage<float, int32_t>; break;
case UINT: reshaper = reshapeImage<float, uint32_t>; break;
default: return false;
}
break;
case INT:
switch (srcType) {
case UBYTE: reshaper = reshapeImage<int32_t, uint8_t>; break;
case FLOAT: reshaper = reshapeImage<int32_t, float>; break;
case INT: reshaper = reshapeImage<int32_t, int32_t>; break;
case UINT: reshaper = reshapeImage<int32_t, uint32_t>; break;
default: return false;
}
break;
case UINT:
switch (srcType) {
case UBYTE: reshaper = reshapeImage<uint32_t, uint8_t>; break;
case FLOAT: reshaper = reshapeImage<uint32_t, float>; break;
case INT: reshaper = reshapeImage<uint32_t, int32_t>; break;
case UINT: reshaper = reshapeImage<uint32_t, uint32_t>; break;
default: return false;
}
break;
default:
return false;
}
uint8_t* dstBytes = (uint8_t*) dst->buffer;
const int dstBytesPerRow = PixelBufferDescriptor::computeDataSize(dst->format, dst->type,
dst->stride ? dst->stride : width, 1, dst->alignment);
reshaper(dstBytes, srcBytes, srcBytesPerRow, dstBytesPerRow, dstChannelCount, height,
swizzle, flip);
return true;
}

};

template<> inline float getMaxValue() { return 1.0f; }
template<> inline int32_t getMaxValue() { return 0x7fffffff; }
template<> inline uint32_t getMaxValue() { return 0xffffffff; }
template<> inline uint16_t getMaxValue() { return 0x3c00; } // 0x3c00 is 1.0 in half-float.
template<> inline uint8_t getMaxValue() { return 0xff; }

} // namespace backend
} // namespace filament

Expand Down
2 changes: 1 addition & 1 deletion filament/backend/src/DriverBase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ class DriverBase : public Driver {
explicit DriverBase(Dispatcher* dispatcher) noexcept;
~DriverBase() noexcept override;

void purge() noexcept final;
void purge() noexcept override;

Dispatcher& getDispatcher() noexcept final { return *mDispatcher; }

Expand Down
3 changes: 1 addition & 2 deletions filament/backend/src/TextureReshaper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ TextureReshaper::TextureReshaper(TextureFormat requestedFormat) noexcept {
const size_t reshapedSize = p.size / 6 * 8; // reshaping from 6 to 8 bytes per pixel
void* reshapeBuffer = malloc(reshapedSize);
ASSERT_POSTCONDITION(reshapeBuffer, "Could not allocate memory to reshape pixels.");
// 0x3c00 is 1.0 in 16 bit floating point.
DataReshaper::reshape<uint16_t, 3, 4, 0x3c00>(reshapeBuffer, p.buffer, p.size);
DataReshaper::reshape<uint16_t, 3, 4>(reshapeBuffer, p.buffer, p.size);

PixelBufferDescriptor reshaped(reshapeBuffer, reshapedSize,
PixelBufferDescriptor::PixelDataFormat::RGBA,
Expand Down
1 change: 0 additions & 1 deletion filament/backend/src/vulkan/VulkanContext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,6 @@ void createSwapChain(VulkanContext& context, VulkanSurfaceContext& surfaceContex
for (const VkSurfaceFormatKHR& format : surfaceContext.surfaceFormats) {
if (format.format == VK_FORMAT_R8G8B8A8_UNORM) {
surfaceContext.surfaceFormat = format;
break;
}
}
const auto compositionCaps = caps.supportedCompositeAlpha;
Expand Down
74 changes: 30 additions & 44 deletions filament/backend/src/vulkan/VulkanDriver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1293,25 +1293,22 @@ void VulkanDriver::stopCapture(int) {

}

void VulkanDriver::readPixels(Handle<HwRenderTarget> src,
uint32_t x, uint32_t y, uint32_t width, uint32_t height,
PixelBufferDescriptor&& pbd) {
// TODO: add support for all types listed in the Renderer docstring for readPixels.
assert(pbd.type == PixelBufferDescriptor::PixelDataType::UBYTE);

void VulkanDriver::readPixels(Handle<HwRenderTarget> src, uint32_t x, uint32_t y,
uint32_t width, uint32_t height, PixelBufferDescriptor&& pbd) {
const VkDevice device = mContext.device;
const VulkanRenderTarget* srcTarget = handle_cast<VulkanRenderTarget>(mHandleMap, src);
const VulkanTexture* srcTexture = srcTarget->getColor(0).texture;
const VkFormat swapChainFormat = mContext.currentSurface->surfaceFormat.format;
const VkFormat srcFormat = srcTexture ? srcTexture->vkformat : swapChainFormat;
const bool swizzle = srcFormat == VK_FORMAT_B8G8R8A8_UNORM;

// Create a host visible, linearly tiled image as a staging area.

VkImageCreateInfo imageInfo {
.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO,
.imageType = VK_IMAGE_TYPE_2D,
.format = VK_FORMAT_R8G8B8A8_UNORM,
.extent = {
.width = width,
.height = height,
.depth = 1,
},
.format = srcFormat,
.extent = { width, height, 1 },
.mipLevels = 1,
.arrayLayers = 1,
.samples = VK_SAMPLE_COUNT_1_BIT,
Expand All @@ -1336,20 +1333,21 @@ void VulkanDriver::readPixels(Handle<HwRenderTarget> src,
vkAllocateMemory(device, &allocInfo, nullptr, &stagingMemory);
vkBindImageMemory(device, stagingImage, stagingMemory, 0);

// TODO: Should we allow readPixels within beginFrame / endFrame?

assert(mContext.currentCommands == nullptr);
acquireWorkCommandBuffer(mContext);
// TODO: replace waitForIdle with an image barrier coupled with acquireWorkCommandBuffer.
waitForIdle(mContext);

// Transition the staging image layout.

VulkanTexture::transitionImageLayout(mContext.work.cmdbuffer, stagingImage,
VK_IMAGE_LAYOUT_UNDEFINED, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 0, 1, 1,
VK_IMAGE_ASPECT_COLOR_BIT);

const uint8_t srcMipLevel = srcTarget->getColor(0).level;

VkImageCopy imageCopyRegion = {
.srcSubresource = {
.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT,
.mipLevel = srcMipLevel,
.layerCount = 1,
},
.srcOffset = {
Expand All @@ -1369,11 +1367,10 @@ void VulkanDriver::readPixels(Handle<HwRenderTarget> src,

// Transition the source image layout (which might be the swap chain)

VulkanRenderTarget* srcTarget = handle_cast<VulkanRenderTarget>(mHandleMap, src);
VkImage srcImage = srcTarget->getColor(0).image;
VulkanTexture::transitionImageLayout(mContext.work.cmdbuffer, srcImage,
VK_IMAGE_LAYOUT_UNDEFINED,
VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, 0, 1, 1, VK_IMAGE_ASPECT_COLOR_BIT);
VK_IMAGE_LAYOUT_UNDEFINED, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, srcMipLevel, 1, 1,
VK_IMAGE_ASPECT_COLOR_BIT);

// Perform the blit.

Expand All @@ -1383,16 +1380,15 @@ void VulkanDriver::readPixels(Handle<HwRenderTarget> src,

// Restore the source image layout.

VulkanTexture* srcTexture = srcTarget->getColor(0).texture;
if (srcTexture || mContext.currentSurface->presentQueue) {
const VkImageLayout present = VK_IMAGE_LAYOUT_PRESENT_SRC_KHR;
VulkanTexture::transitionImageLayout(mContext.work.cmdbuffer, srcImage,
VK_IMAGE_LAYOUT_UNDEFINED, srcTexture ? getTextureLayout(srcTexture->usage) : present,
0, 1, 1, VK_IMAGE_ASPECT_COLOR_BIT);
srcMipLevel, 1, 1, VK_IMAGE_ASPECT_COLOR_BIT);
} else {
VulkanTexture::transitionImageLayout(mContext.work.cmdbuffer, srcImage,
VK_IMAGE_LAYOUT_UNDEFINED, VK_IMAGE_LAYOUT_GENERAL,
0, 1, 1, VK_IMAGE_ASPECT_COLOR_BIT);
srcMipLevel, 1, 1, VK_IMAGE_ASPECT_COLOR_BIT);
}

// Transition the staging image layout to GENERAL.
Expand Down Expand Up @@ -1440,28 +1436,18 @@ void VulkanDriver::readPixels(Handle<HwRenderTarget> src,
vkMapMemory(device, stagingMemory, 0, VK_WHOLE_SIZE, 0, (void**) &srcPixels);
srcPixels += subResourceLayout.offset;

uint8_t* dstPixels = (uint8_t*) closure->buffer;
const uint32_t dstStride = closure->stride ? closure->stride : width;
const int dstBytesPerRow = PixelBufferDescriptor::computeDataSize(closure->format,
closure->type, dstStride, 1, closure->alignment);
const int srcBytesPerRow = subResourceLayout.rowPitch;
const VkFormat swapChainFormat = mContext.currentSurface->surfaceFormat.format;
const bool swizzle = !srcTexture && swapChainFormat == VK_FORMAT_B8G8R8A8_UNORM;

switch (closure->format) {
case PixelDataFormat::RGB:
case PixelDataFormat::RGB_INTEGER:
DataReshaper::reshapeImage<uint8_t, 4, 3>(dstPixels, srcPixels, srcBytesPerRow,
dstBytesPerRow, height, swizzle);
break;
case PixelDataFormat::RGBA:
case PixelDataFormat::RGBA_INTEGER:
DataReshaper::reshapeImage<uint8_t, 4, 4>(dstPixels, srcPixels, srcBytesPerRow,
dstBytesPerRow, height, swizzle);
break;
default:
utils::slog.e << "ReadPixels: invalid PixelDataFormat" << utils::io::endl;
break;
// TODO: investigate why this Y-flip conditional exists. At least two SwiftShader-based
// tests (viewer_basic_test.cc and gltf_viewer batch mode) seem to require "false". However
// test_ReadPixels.cpp with MoltenVK requires "true" to be consistent with OpenGL and Metal.
// One hypothesis is that this is due to the layout of the SwiftShader backbuffer.
#ifdef FILAMENT_USE_SWIFTSHADER
constexpr bool flipY = false;
#else
constexpr bool flipY = true;
#endif
if (!DataReshaper::reshapeImage(closure, getComponentType(srcFormat), srcPixels,
subResourceLayout.rowPitch, width, height, swizzle, flipY)) {
utils::slog.e << "Unsupported PixelDataFormat or PixelDataType" << utils::io::endl;
}

vkUnmapMemory(device, stagingMemory);
Expand Down
11 changes: 11 additions & 0 deletions filament/backend/src/vulkan/VulkanDriver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,17 @@ class VulkanDriver final : public DriverBase {
VulkanDriver(VulkanDriver const&) = delete;
VulkanDriver& operator = (VulkanDriver const&) = delete;

void purge() noexcept override {
// First we trigger garbage collection of Vulkan resources. This ensures that transient
// resources (e.g. the ReadPixels staging buffer) are removed if their refcount is 0, which
// allows related BufferDescriptors to move to the purge list.
mDisposer.gc();

// Next, allow the base class to clean up the purge list in order to trigger the
// BufferDescriptor destructors, which in turn triggers the user-provided callbacks.
DriverBase::purge();
}

private:
backend::VulkanPlatform& mContextManager;

Expand Down
Loading

0 comments on commit 548b28c

Please sign in to comment.