diff --git a/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp b/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp index 6c643f754d8..cfec76e3bc9 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp +++ b/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp @@ -31,22 +31,22 @@ namespace { std::tuple 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 diff --git a/filament/include/filament/Renderer.h b/filament/include/filament/Renderer.h index a4e919610db..7e66c898c45 100644 --- a/filament/include/filament/Renderer.h +++ b/filament/include/filament/Renderer.h @@ -508,7 +508,7 @@ class UTILS_PUBLIC Renderer : public FilamentAPI { * * Framebuffer as seen on User buffer (PixelBufferDescriptor&) * screen - * + * * +--------------------+ * | | .stride .alignment * | | ----------------------->--> @@ -538,6 +538,9 @@ class UTILS_PUBLIC Renderer : public FilamentAPI { * 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. * diff --git a/filament/src/details/RenderTarget.cpp b/filament/src/details/RenderTarget.cpp index 6c232e43e5c..53697057b7d 100644 --- a/filament/src/details/RenderTarget.cpp +++ b/filament/src/details/RenderTarget.cpp @@ -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(); + } } } diff --git a/filament/src/details/RenderTarget.h b/filament/src/details/RenderTarget.h index 1b57b5acc9b..9fba1f60ff1 100644 --- a/filament/src/details/RenderTarget.h +++ b/filament/src/details/RenderTarget.h @@ -68,6 +68,10 @@ class FRenderTarget : public RenderTarget { 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 @@ class FRenderTarget : public RenderTarget { backend::TargetBufferFlags mAttachmentMask = {}; backend::TargetBufferFlags mSampleableAttachmentsMask = {}; const uint8_t mSupportedColorAttachmentsCount; + bool mSupportsReadPixels = false; }; FILAMENT_DOWNCAST(RenderTarget) diff --git a/filament/src/details/Renderer.cpp b/filament/src/details/Renderer.cpp index e201c8dd63e..494486e2a14 100644 --- a/filament/src/details/Renderer.cpp +++ b/filament/src/details/Renderer.cpp @@ -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 picking; }; - fg.addPass("Picking Resolve Pass", + fg.addPass( + "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 }} }); diff --git a/filament/src/details/Texture.cpp b/filament/src/details/Texture.cpp index 2f405fd2ab3..176907345b7 100644 --- a/filament/src/details/Texture.cpp +++ b/filament/src/details/Texture.cpp @@ -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 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) { diff --git a/filament/src/details/Texture.h b/filament/src/details/Texture.h index 7c0234bb535..6bf04dae4ac 100644 --- a/filament/src/details/Texture.h +++ b/filament/src/details/Texture.h @@ -123,6 +123,11 @@ class FTexture : public Texture { 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 @@ class FTexture : public Texture { 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 };