diff --git a/filament/backend/include/backend/DriverEnums.h b/filament/backend/include/backend/DriverEnums.h index e8bfb6956e..b194242a08 100644 --- a/filament/backend/include/backend/DriverEnums.h +++ b/filament/backend/include/backend/DriverEnums.h @@ -98,20 +98,14 @@ struct Viewport { }; struct RenderPassFlags { - uint8_t clear; + TargetBufferFlags clear; TargetBufferFlags discardStart; TargetBufferFlags discardEnd; - uint8_t dependencies; - - static constexpr uint8_t DEPENDENCY_BY_REGION = 1; // see "framebuffer-local" in Vulkan spec. - - // Extra RenderPass-only flags stashed in the "clear" field. - static const uint8_t IGNORE_SCISSOR = 0x10; + bool ignoreScissor; }; struct RenderPassParams { - // RenderPass flags are 4 bytes. The first three are buffer selections composed from - // TargetBufferFlags. The last byte is an optional dependency hint (used only for Vulkan). + // RenderPass flags (4 bytes) RenderPassFlags flags{}; // Viewport (16 bytes) diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index a6b8a6c053..ebe1c962f8 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -1794,8 +1794,8 @@ void OpenGLDriver::beginRenderPass(Handle rth, mRenderPassTarget = rth; mRenderPassParams = params; - const auto clearFlags = TargetBufferFlags(params.flags.clear & ~RenderPassFlags::IGNORE_SCISSOR); - const uint8_t ignoreScissor = params.flags.clear & RenderPassFlags::IGNORE_SCISSOR; + const TargetBufferFlags clearFlags = params.flags.clear; + const bool ignoreScissor = params.flags.ignoreScissor; TargetBufferFlags discardFlags = params.flags.discardStart; GLRenderTarget* rt = handle_cast(rth); @@ -1823,7 +1823,7 @@ void OpenGLDriver::beginRenderPass(Handle rth, // everything must appear as though the multi-sample buffer was lost. if (ALLOW_REVERSE_MULTISAMPLE_RESOLVE) { // We only copy the non msaa buffers that were not discarded or cleared. - const TargetBufferFlags discarded = discardFlags | (clearFlags & TargetBufferFlags::ALL); + const TargetBufferFlags discarded = discardFlags | clearFlags; resolvePass(ResolveAction::LOAD, rt, discarded); } else { // However, for now filament specifies that a non multi-sample attachment to a @@ -1837,7 +1837,7 @@ void OpenGLDriver::beginRenderPass(Handle rth, params.viewport.width, params.viewport.height); // Use scissor test if not told to ignore, and if the viewport doesn't cover the whole target. - const bool respectScissor = !(ignoreScissor & RenderPassFlags::IGNORE_SCISSOR) && + const bool respectScissor = !ignoreScissor && (params.viewport.left != 0 || params.viewport.bottom != 0 || params.viewport.width != rt->width || diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 42ae4e6cce..3db0d860af 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -673,9 +673,7 @@ void VulkanDriver::updateSamplerGroup(Handle sbh, *sb->sb = samplerGroup; } -void VulkanDriver::beginRenderPass(Handle rth, - const RenderPassParams& params) { - +void VulkanDriver::beginRenderPass(Handle rth, const RenderPassParams& params) { assert(mContext.currentCommands); assert(mContext.currentSurface); VulkanSurfaceContext& surface = *mContext.currentSurface; @@ -694,13 +692,19 @@ void VulkanDriver::beginRenderPass(Handle rth, mDisposer.acquire(color.offscreen, mContext.currentCommands->resources); mDisposer.acquire(depth.offscreen, mContext.currentCommands->resources); - // TODO: do not make assumptions about the future use of this attachment, instead get a flag - // via RenderPassParams and use that to determine which layout to transition to at the end. VkImageLayout finalColorLayout; VkImageLayout finalDepthLayout; + if (rt->isOffscreen()) { - finalColorLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; + + // If we're discarding the contents of the color buffer after the render pass, it's safe to + // assume that we will not be sampling from it. + finalColorLayout = any(params.flags.discardEnd & TargetBufferFlags::COLOR) ? + VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL : + VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; + finalDepthLayout = VK_IMAGE_LAYOUT_GENERAL; + } else { finalColorLayout = VK_IMAGE_LAYOUT_PRESENT_SRC_KHR; finalDepthLayout = VK_IMAGE_LAYOUT_PRESENT_SRC_KHR; @@ -711,10 +715,9 @@ void VulkanDriver::beginRenderPass(Handle rth, .finalDepthLayout = finalDepthLayout, .colorFormat = color.format, .depthFormat = depth.format, - .flags.clear = params.flags.clear, - .flags.discardStart = (uint8_t)params.flags.discardStart, - .flags.discardEnd = (uint8_t)params.flags.discardEnd, - .flags.dependencies = params.flags.dependencies + .flags.clear = params.flags.clear, + .flags.discardStart = params.flags.discardStart, + .flags.discardEnd = params.flags.discardEnd }); mBinder.bindRenderPass(renderPass); diff --git a/filament/backend/src/vulkan/VulkanFboCache.cpp b/filament/backend/src/vulkan/VulkanFboCache.cpp index 506b16c3da..38d7b1f293 100644 --- a/filament/backend/src/vulkan/VulkanFboCache.cpp +++ b/filament/backend/src/vulkan/VulkanFboCache.cpp @@ -108,7 +108,7 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { VkAttachmentDescription colorAttachment { .format = config.colorFormat, .samples = VK_SAMPLE_COUNT_1_BIT, - .loadOp = (config.flags.clear & (uint8_t)TargetBufferFlags::COLOR) ? + .loadOp = any(config.flags.clear & TargetBufferFlags::COLOR) ? VK_ATTACHMENT_LOAD_OP_CLEAR : VK_ATTACHMENT_LOAD_OP_DONT_CARE, .storeOp = VK_ATTACHMENT_STORE_OP_STORE, .stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE, @@ -118,7 +118,7 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { VkAttachmentDescription depthAttachment { .format = config.depthFormat, .samples = VK_SAMPLE_COUNT_1_BIT, - .loadOp = (config.flags.clear & (uint8_t)TargetBufferFlags::DEPTH) ? + .loadOp = any(config.flags.clear & TargetBufferFlags::DEPTH) ? VK_ATTACHMENT_LOAD_OP_CLEAR : VK_ATTACHMENT_LOAD_OP_DONT_CARE, .storeOp = VK_ATTACHMENT_STORE_OP_STORE, .stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE, @@ -126,36 +126,13 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { .finalLayout = config.finalDepthLayout }; - // We define dependencies only when the framebuffer local hint is applied. - // NOTE: It's likely that VK_DEPENDENCY_BY_REGION_BIT and VK_ACCESS_COLOR_ATTACHMENT_READ do - // not actually achieve anything since are neither defining multiple subpasses, nor reading back - // from the framebuffer in the shader using subpassLoad(). - VkSubpassDependency dependencies[] = {{ - .srcSubpass = VK_SUBPASS_EXTERNAL, - .dstSubpass = 0, - .srcStageMask = VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, - .dstStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, - .srcAccessMask = VK_ACCESS_MEMORY_READ_BIT, - .dstAccessMask = VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, - .dependencyFlags = config.flags.dependencies - }, { - .srcSubpass = 0, - .dstSubpass = VK_SUBPASS_EXTERNAL, - .srcStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, - .dstStageMask = VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, - .srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, - .dstAccessMask = VK_ACCESS_MEMORY_READ_BIT, - .dependencyFlags = config.flags.dependencies - }}; - // Finally, create the VkRenderPass. VkAttachmentDescription attachments[2]; VkRenderPassCreateInfo renderPassInfo { .sType = VK_STRUCTURE_TYPE_RENDER_PASS_CREATE_INFO, .attachmentCount = 0u, .pAttachments = attachments, - .dependencyCount = config.flags.dependencies ? 2u : 0u, - .pDependencies = config.flags.dependencies ? dependencies : nullptr, + .dependencyCount = 0u, .subpassCount = 1, .pSubpasses = &subpass }; diff --git a/filament/backend/src/vulkan/VulkanFboCache.h b/filament/backend/src/vulkan/VulkanFboCache.h index ca54e1be22..bbce4c234f 100644 --- a/filament/backend/src/vulkan/VulkanFboCache.h +++ b/filament/backend/src/vulkan/VulkanFboCache.h @@ -44,14 +44,14 @@ public: VkFormat depthFormat; // 4 bytes union { struct { - uint8_t clear; - uint8_t discardStart; - uint8_t discardEnd; - uint8_t dependencies; + TargetBufferFlags clear; + TargetBufferFlags discardStart; + TargetBufferFlags discardEnd; + uint8_t padding0; }; uint32_t value; // 4 bytes } flags; - uint32_t padding; // 4 bytes + uint32_t padding1; // 4 bytes }; struct RenderPassVal { VkRenderPass handle; diff --git a/filament/src/Renderer.cpp b/filament/src/Renderer.cpp index b65479e2ec..4d9ebfda45 100644 --- a/filament/src/Renderer.cpp +++ b/filament/src/Renderer.cpp @@ -446,8 +446,8 @@ void FRenderer::copyFrame(FSwapChain* dstSwapChain, filament::Viewport const& ds // Clear color to black if the CLEAR flag is set. if (flags & CLEAR) { params.clearColor = {0.f, 0.f, 0.f, 1.f}; - params.flags.clear = (uint8_t)TargetBufferFlags::COLOR; - params.flags.clear |= RenderPassFlags::IGNORE_SCISSOR; + params.flags.clear = TargetBufferFlags::COLOR; + params.flags.ignoreScissor = true; params.flags.discardStart = TargetBufferFlags::ALL; params.flags.discardEnd = TargetBufferFlags::NONE; params.viewport.left = 0; diff --git a/filament/src/ShadowMap.cpp b/filament/src/ShadowMap.cpp index b79cc6bfef..acff9db2d8 100644 --- a/filament/src/ShadowMap.cpp +++ b/filament/src/ShadowMap.cpp @@ -159,14 +159,14 @@ void ShadowMap::render(DriverApi& driver, RenderPass& pass, FView& view) noexcep // FIXME: in the future this will come from the framegraph RenderPassParams params = {}; - params.flags.clear = (uint8_t)TargetBufferFlags::DEPTH; + params.flags.clear = TargetBufferFlags::DEPTH; params.flags.discardStart = TargetBufferFlags::DEPTH; params.flags.discardEnd = TargetBufferFlags::COLOR_AND_STENCIL; params.clearDepth = 1.0; params.viewport = viewport; // disable scissor for clearing so the whole surface, but set the viewport to the // the inset-by-1 rectangle. - params.flags.clear |= RenderPassFlags::IGNORE_SCISSOR; + params.flags.ignoreScissor = true; FCamera const& camera = getCamera(); details::CameraInfo cameraInfo = { diff --git a/filament/src/fg/FrameGraph.cpp b/filament/src/fg/FrameGraph.cpp index d9e79c9d24..8b68a804a5 100644 --- a/filament/src/fg/FrameGraph.cpp +++ b/filament/src/fg/FrameGraph.cpp @@ -154,7 +154,6 @@ FrameGraphPassResources::getRenderTarget(FrameGraphRenderTargetHandle handle, ui // overwrite discard flags with the per-rendertarget (per-pass) computed value info.params.flags.discardStart = renderTarget.targetFlags.discardStart; info.params.flags.discardEnd = renderTarget.targetFlags.discardEnd; - info.params.flags.dependencies = renderTarget.targetFlags.dependencies; // check that this FrameGraphRenderTarget is indeed declared by this pass ASSERT_POSTCONDITION_NON_FATAL(info.target, @@ -588,8 +587,7 @@ FrameGraph& FrameGraph::compile() noexcept { pRenderTarget->targetFlags = { .clear = {}, // this is eventually set by the user .discardStart = discardStart, - .discardEnd = discardEnd, - .dependencies = {} + .discardEnd = discardEnd }; } } diff --git a/filament/src/fg/fg/RenderTarget.cpp b/filament/src/fg/fg/RenderTarget.cpp index 1c6a592bd3..d4d483cfa8 100644 --- a/filament/src/fg/fg/RenderTarget.cpp +++ b/filament/src/fg/fg/RenderTarget.cpp @@ -35,7 +35,7 @@ void RenderTarget::resolve(FrameGraph& fg) noexcept { if (pos != renderTargetCache.end()) { cache = pos->get(); - cache->targetInfo.params.flags.clear |= (uint8_t)userClearFlags; + cache->targetInfo.params.flags.clear |= userClearFlags; } else { TargetBufferFlags attachments{}; uint32_t width = 0; @@ -100,7 +100,7 @@ void RenderTarget::resolve(FrameGraph& fg) noexcept { backend::TargetBufferFlags(attachments), width, height, colorFormat); renderTargetCache.emplace_back(pRenderTargetResource, fg); cache = pRenderTargetResource; - cache->targetInfo.params.flags.clear |= (uint8_t)userClearFlags; + cache->targetInfo.params.flags.clear |= userClearFlags; } } } diff --git a/filament/test/filament_framegraph_test.cpp b/filament/test/filament_framegraph_test.cpp index ef40ab45be..cdc21c8640 100644 --- a/filament/test/filament_framegraph_test.cpp +++ b/filament/test/filament_framegraph_test.cpp @@ -352,7 +352,7 @@ TEST(FrameGraphTest, RenderTargetLifetime) { renderPassExecuted2 = true; auto const& rt = resources.getRenderTarget(data.rt); EXPECT_TRUE(rt.target); - EXPECT_EQ(0x40u|0x80u, rt.params.flags.clear); + EXPECT_EQ(TargetBufferFlags(0x40u|0x80u), rt.params.flags.clear); EXPECT_EQ(rt1.getId(), rt.target.getId()); // FIXME: this test is always true the NoopDriver EXPECT_EQ(TargetBufferFlags::DEPTH_AND_STENCIL, rt.params.flags.discardStart); EXPECT_EQ(TargetBufferFlags::DEPTH_AND_STENCIL, rt.params.flags.discardEnd);