Scarfski/filament
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make sure color attachment textures have copy-able usage set (#8184)

Browse Source 
To ensure the source of readPixels() is properly copy-able, we
want the backing textures to be created with the right BLIT_SRC usage.
However, this was not documented in the API. We workaround the issue
to tag all color attachment textures as BLIT_SRC.

This workaround will be removed in the future. For now, violations of
this condition will elicit a warning being printed out.
This commit is contained in:
Powei Feng
2024-10-10 16:02:11 +00:00
committed by GitHub
parent 40c0d59464
commit 614dbb44d5
7 changed files with 77 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp
+17 -17
View File
@@ -31,22 +31,22 @@ namespace {
std::tuple<VkImage, VkDeviceMemory> createImageAndMemory(VulkanContext const& context,
VkDevice device, VkExtent2D extent, VkFormat format) {
bool const isDepth = isVkDepthFormat(format);
// Filament expects blit() to work with any texture, so we almost always set these usage flags.
// TODO: investigate performance implications of setting these flags.
VkImageUsageFlags const blittable
= VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT;
VkImageCreateInfo imageInfo{
.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO,
.imageType = VK_IMAGE_TYPE_2D,
.format = format,
.extent = {extent.width, extent.height, 1},
.mipLevels = 1,
.arrayLayers = 1,
.samples = VK_SAMPLE_COUNT_1_BIT,
.tiling = VK_IMAGE_TILING_OPTIMAL,
.usage = blittable
| (isDepth ? VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT
: VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT),
// Filament expects blit() to work with any texture, so we almost always set these usage flags
// (see copyFrame() and readPixels()).
VkImageUsageFlags const blittable =
VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT;
VkImageCreateInfo imageInfo {
.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO,
.imageType = VK_IMAGE_TYPE_2D,
.format = format,
.extent = {extent.width, extent.height, 1},
.mipLevels = 1,
.arrayLayers = 1,
.samples = VK_SAMPLE_COUNT_1_BIT,
.tiling = VK_IMAGE_TILING_OPTIMAL,
.usage = blittable | (isDepth ? VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT
: VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT),
};
VkImage image;
VkResult result = vkCreateImage(device, &imageInfo, VKALLOC, &image);
@@ -220,7 +220,7 @@ VkResult VulkanPlatformSurfaceSwapChain::create() {
.imageArrayLayers = 1,
.imageUsage
= VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT
| VK_IMAGE_USAGE_TRANSFER_DST_BIT // Allows use as a blit destination.
| VK_IMAGE_USAGE_TRANSFER_DST_BIT // Allows use as a blit destination (for copyFrame)
| VK_IMAGE_USAGE_TRANSFER_SRC_BIT,// Allows use as a blit source (for readPixels)
// TODO: Setting the preTransform to IDENTITY means we are letting the Android
filament/include/filament/Renderer.h
+4 -1
View File
@@ -508,7 +508,7 @@ public:
*
* Framebuffer as seen on User buffer (PixelBufferDescriptor&)
* screen
*
*
* +--------------------+
* | | .stride .alignment
* | | ----------------------->-->
@@ -538,6 +538,9 @@ public:
* uploaded to it via setImage, the data returned from readPixels will be y-flipped with respect
* to the setImage call.
*
* Note: the texture that backs the COLOR attachment for `renderTarget` must have
* TextureUsage::BLIT_SRC as part of its usage.
*
* @remark
* readPixels() is intended for debugging and testing. It will impact performance significantly.
*
filament/src/details/RenderTarget.cpp
+12 -2
View File
@@ -156,8 +156,8 @@ RenderTarget* RenderTarget::Builder::build(Engine& engine) {
// ------------------------------------------------------------------------------------------------
FRenderTarget::FRenderTarget(FEngine& engine, const RenderTarget::Builder& builder)
: mSupportedColorAttachmentsCount(engine.getDriverApi().getMaxDrawBuffers()) {
: mSupportedColorAttachmentsCount(engine.getDriverApi().getMaxDrawBuffers()),
mSupportsReadPixels(false) {
std::copy(std::begin(builder.mImpl->mAttachments), std::end(builder.mImpl->mAttachments),
std::begin(mAttachments));
@@ -189,6 +189,16 @@ FRenderTarget::FRenderTarget(FEngine& engine, const RenderTarget::Builder& build
(TextureUsage::SAMPLEABLE | Texture::Usage::SUBPASS_INPUT))) {
mSampleableAttachmentsMask |= targetBufferBit;
}
// readPixels() only applies to the color attachment that binds at index 0.
if (i == 0 && any(attachment.texture->getUsage() & TextureUsage::COLOR_ATTACHMENT)) {
// TODO: the following will be changed to
// mSupportsReadPixels =
// any(attachment.texture->getUsage() & TextureUsage::BLIT_SRC);
// in a later filament version when clients have properly added the right usage.
mSupportsReadPixels = attachment.texture->hasBlitSrcUsage();
}
}
}
filament/src/details/RenderTarget.h
+5
View File
@@ -68,6 +68,10 @@ public:
bool hasSampleableDepth() const noexcept;
bool supportsReadPixels() const noexcept {
return mSupportsReadPixels;
}
private:
friend class RenderTarget;
static constexpr size_t ATTACHMENT_COUNT = MAX_SUPPORTED_COLOR_ATTACHMENTS_COUNT + 1u;
@@ -76,6 +80,7 @@ private:
backend::TargetBufferFlags mAttachmentMask = {};
backend::TargetBufferFlags mSampleableAttachmentsMask = {};
const uint8_t mSupportedColorAttachmentsCount;
bool mSupportsReadPixels = false;
};
FILAMENT_DOWNCAST(RenderTarget)
filament/src/details/Renderer.cpp
+16 -4
View File
@@ -455,6 +455,15 @@ void FRenderer::readPixels(uint32_t xoffset, uint32_t yoffset, uint32_t width, u
void FRenderer::readPixels(FRenderTarget* renderTarget,
uint32_t xoffset, uint32_t yoffset, uint32_t width, uint32_t height,
backend::PixelBufferDescriptor&& buffer) {
// TODO: change the following to an assert when client call sites have addressed the issue.
if (!renderTarget->supportsReadPixels()) {
utils::slog.w << "readPixels() must be called with a renderTarget with COLOR0 created with "
"TextureUsage::BLIT_SRC. This precondition will be asserted in a later "
"release of Filament."
<< utils::io::endl;
}
RendererUtils::readPixels(mEngine.getDriverApi(), renderTarget->getHwHandle(),
xoffset, yoffset, width, height, std::move(buffer));
}
@@ -963,15 +972,18 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) {
});
const auto picking = picking_;
if (view.hasPicking()) {
struct PickingResolvePassData {
FrameGraphId<FrameGraphTexture> picking;
};
fg.addPass<PickingResolvePassData>("Picking Resolve Pass",
fg.addPass<PickingResolvePassData>(
"Picking Resolve Pass",
[&](FrameGraph::Builder& builder, auto& data) {
data.picking = builder.read(picking,
FrameGraphTexture::Usage::COLOR_ATTACHMENT);
// Note that BLIT_SRC is needed because this texture will be read later (via
// readPixels()).
data.picking =
builder.read(picking, FrameGraphTexture::Usage::COLOR_ATTACHMENT |
FrameGraphTexture::Usage::BLIT_SRC);
builder.declareRenderPass("Picking Resolve Target", {
.attachments = { .color = { data.picking }}
});
filament/src/details/Texture.cpp
+11
View File
@@ -79,6 +79,7 @@ struct Texture::BuilderDetails {
Sampler mTarget = Sampler::SAMPLER_2D;
InternalFormat mFormat = InternalFormat::RGBA8;
Usage mUsage = Usage::NONE;
bool mHasBlitSrc = false;
bool mTextureIsSwizzled = false;
std::array<Swizzle, 4> mSwizzle = {
Swizzle::CHANNEL_0, Swizzle::CHANNEL_1,
@@ -183,6 +184,15 @@ Texture* Texture::Builder::build(Engine& engine) {
}
}
// TODO: remove in a future filament release.
// Clients might not have known that textures that are read need to have BLIT_SRC as usages. For
// now, we workaround the issue by making sure any color attachment can be the source of a copy
// for readPixels().
mImpl->mHasBlitSrc = any(mImpl->mUsage & TextureUsage::BLIT_SRC);
if (!mImpl->mHasBlitSrc && any(mImpl->mUsage & TextureUsage::COLOR_ATTACHMENT)) {
mImpl->mUsage |= TextureUsage::BLIT_SRC;
}
const bool sampleable = bool(mImpl->mUsage & TextureUsage::SAMPLEABLE);
const bool swizzled = mImpl->mTextureIsSwizzled;
const bool imported = mImpl->mImportedId;
@@ -232,6 +242,7 @@ FTexture::FTexture(FEngine& engine, const Builder& builder) {
mLevelCount = builder->mLevels;
mSwizzle = builder->mSwizzle;
mTextureIsSwizzled = builder->mTextureIsSwizzled;
mHasBlitSrc = builder->mHasBlitSrc;
bool const isImported = builder->mImportedId != 0;
if (mTarget == SamplerType::SAMPLER_EXTERNAL && !isImported) {
filament/src/details/Texture.h
+12 -1
View File
@@ -123,6 +123,11 @@ public:
bool textureHandleCanMutate() const noexcept;
void updateLodRange(uint8_t level) noexcept;
// TODO: remove in a future filament release. See below for description.
inline bool hasBlitSrcUsage() const noexcept {
return mHasBlitSrc;
}
private:
friend class Texture;
struct LodRange {
@@ -163,7 +168,13 @@ private:
bool mTextureIsSwizzled;
Usage mUsage = Usage::DEFAULT;
// there is 7 bytes of padding here
// TODO: remove in a future filament release.
// Indicates whether the user has set the TextureUsage::BLIT_SRC usage. This will be used to
// temporarily validate whether this texture can be used for readPixels.
bool mHasBlitSrc = false;
// there is 5 bytes of padding here
FStream* mStream = nullptr; // only needed for streaming textures
};