From 4425338559f1f70ef4a063ede30169e5993ac641 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 16 Oct 2024 22:03:53 -0700 Subject: [PATCH] avoid setting the scissor rect when possible RenderPass is now tracking the scissor state locally so it can avoid re-setting the scissor when it doesn't change. We also consolidate the scissor override and the scissor-viewport in RenderPass::Execute. --- .../backend/include/backend/DriverEnums.h | 13 ++++- filament/src/RenderPass.cpp | 50 +++++++++++-------- filament/src/RenderPass.h | 25 ++++++---- filament/src/details/Renderer.cpp | 4 +- 4 files changed, 59 insertions(+), 33 deletions(-) diff --git a/filament/backend/include/backend/DriverEnums.h b/filament/backend/include/backend/DriverEnums.h index 191e7ac2262..f99160b348a 100644 --- a/filament/backend/include/backend/DriverEnums.h +++ b/filament/backend/include/backend/DriverEnums.h @@ -318,8 +318,19 @@ struct Viewport { int32_t right() const noexcept { return left + int32_t(width); } //! get the top coordinate in window space of the viewport int32_t top() const noexcept { return bottom + int32_t(height); } -}; + friend bool operator==(Viewport const& lhs, Viewport const& rhs) noexcept { + // clang can do this branchless with xor/or + return lhs.left == rhs.left && lhs.bottom == rhs.bottom && + lhs.width == rhs.width && lhs.height == rhs.height; + } + + friend bool operator!=(Viewport const& lhs, Viewport const& rhs) noexcept { + // clang is being dumb and uses branches + return bool(((lhs.left ^ rhs.left) | (lhs.bottom ^ rhs.bottom)) | + ((lhs.width ^ rhs.width) | (lhs.height ^ rhs.height))); + } +}; /** * Specifies the mapping of the near and far clipping plane to window coordinates. diff --git a/filament/src/RenderPass.cpp b/filament/src/RenderPass.cpp index 2df6a88c67a..9498184c867 100644 --- a/filament/src/RenderPass.cpp +++ b/filament/src/RenderPass.cpp @@ -861,12 +861,6 @@ void RenderPass::Executor::overridePolygonOffset(backend::PolygonOffset const* p } } -void RenderPass::Executor::overrideScissor(backend::Viewport const* scissor) noexcept { - if ((mScissorOverride = (scissor != nullptr))) { // NOLINT(*-assignment-in-if-condition) - mScissor = *scissor; - } -} - void RenderPass::Executor::overrideScissor(backend::Viewport const& scissor) noexcept { mScissorOverride = true; mScissor = scissor; @@ -883,15 +877,15 @@ backend::Viewport RenderPass::Executor::applyScissorViewport( // clang vectorizes this! constexpr int32_t maxvali = std::numeric_limits::max(); // compute new left/bottom, assume no overflow - int32_t l = scissor.left + scissorViewport.left; - int32_t b = scissor.bottom + scissorViewport.bottom; + int32_t const l = scissor.left + scissorViewport.left; + int32_t const b = scissor.bottom + scissorViewport.bottom; // compute right/top without overflowing, scissor.width/height guaranteed // to convert to int32 int32_t r = (l > maxvali - int32_t(scissor.width)) ? maxvali : l + int32_t(scissor.width); int32_t t = (b > maxvali - int32_t(scissor.height)) ? maxvali : b + int32_t(scissor.height); // clip to the viewport - l = std::max(l, scissorViewport.left); - b = std::max(b, scissorViewport.bottom); + assert_invariant(l == std::max(l, scissorViewport.left)); + assert_invariant(b == std::max(b, scissorViewport.bottom)); r = std::min(r, scissorViewport.left + int32_t(scissorViewport.width)); t = std::min(t, scissorViewport.bottom + int32_t(scissorViewport.height)); assert_invariant(r >= l && t >= b); @@ -913,9 +907,14 @@ void RenderPass::Executor::execute(FEngine& engine, if (first != last) { SYSTRACE_VALUE32("commandCount", last - first); - bool const scissorOverride = mScissorOverride; - if (UTILS_UNLIKELY(scissorOverride)) { - // initialize with scissor override + // The scissor rectangle is associated to a render pass, so the tracking can be local. + backend::Viewport currentScissor{ 0, 0, INT32_MAX, INT32_MAX }; + bool const hasScissorOverride = mScissorOverride; + bool const hasScissorViewport = mHasScissorViewport; + if (UTILS_UNLIKELY(hasScissorViewport || hasScissorOverride)) { + // we should never have both an override and scissor-viewport + assert_invariant(!hasScissorViewport || !hasScissorOverride); + currentScissor = mScissor; driver.scissor(mScissor); } @@ -999,17 +998,24 @@ void RenderPass::Executor::execute(FEngine& engine, if (UTILS_UNLIKELY(mi != info.mi)) { // this is always taken the first time - mi = info.mi; - assert_invariant(mi); + assert_invariant(info.mi); + mi = info.mi; ma = mi->getMaterial(); - if (UTILS_LIKELY(!scissorOverride)) { + // if we have the scissor override, the material instance and scissor-viewport + // are ignored (typically used for shadow maps). + if (!hasScissorOverride) { + // apply the MaterialInstance scissor backend::Viewport scissor = mi->getScissor(); - if (UTILS_UNLIKELY(mi->hasScissor())) { - scissor = applyScissorViewport(mScissorViewport, scissor); + if (hasScissorViewport) { + // apply the scissor viewport if any + scissor = applyScissorViewport(mScissor, scissor); + } + if (scissor != currentScissor) { + currentScissor = scissor; + driver.scissor(scissor); } - driver.scissor(scissor); } if (UTILS_LIKELY(!polygonOffsetOverride)) { @@ -1100,16 +1106,18 @@ RenderPass::Executor::Executor(RenderPass const& pass, Command const* b, Command mInstancedUboHandle(pass.mInstancedUboHandle), mInstancedDescriptorSetHandle(pass.mInstancedDescriptorSetHandle), mColorPassDescriptorSet(pass.mColorPassDescriptorSet), - mScissorViewport(pass.mScissorViewport), + mScissor(pass.mScissorViewport), mPolygonOffsetOverride(false), mScissorOverride(false) { + mHasScissorViewport = mScissor != backend::Viewport{ 0, 0, INT32_MAX, INT32_MAX }; assert_invariant(b >= pass.begin()); assert_invariant(e <= pass.end()); } RenderPass::Executor::Executor() noexcept : mPolygonOffsetOverride(false), - mScissorOverride(false) { + mScissorOverride(false), + mHasScissorViewport(false) { } RenderPass::Executor::Executor(Executor&& rhs) noexcept = default; diff --git a/filament/src/RenderPass.h b/filament/src/RenderPass.h index fc4370aa8b9..a11d89e9c2f 100644 --- a/filament/src/RenderPass.h +++ b/filament/src/RenderPass.h @@ -350,12 +350,17 @@ class RenderPass { BufferObjectSharedHandle mInstancedUboHandle; DescriptorSetSharedHandle mInstancedDescriptorSetHandle; ColorPassDescriptorSet const* mColorPassDescriptorSet = nullptr; - backend::Viewport mScissorViewport; - - backend::Viewport mScissor{}; // value of scissor override - backend::PolygonOffset mPolygonOffset{}; // value of the override - bool mPolygonOffsetOverride : 1; // whether to override the polygon offset setting - bool mScissorOverride : 1; // whether to override the polygon offset setting + // this stores either the scissor-viewport or the scissor override + backend::Viewport mScissor{ 0, 0, INT32_MAX, INT32_MAX }; + + // value of the polygon offset override + backend::PolygonOffset mPolygonOffset{}; + // whether to override the polygon offset from the MaterialInstance + bool mPolygonOffsetOverride : 1; + // whether to override the scissor rectangle from the MaterialInstance + bool mScissorOverride : 1; + // whether the scissor-viewport is set + bool mHasScissorViewport : 1; Executor(RenderPass const& pass, Command const* b, Command const* e) noexcept; @@ -382,8 +387,6 @@ class RenderPass { // if non-null, overrides the material's polygon offset void overridePolygonOffset(backend::PolygonOffset const* polygonOffset) noexcept; - // if non-null, overrides the material's scissor - void overrideScissor(backend::Viewport const* scissor) noexcept; void overrideScissor(backend::Viewport const& scissor) noexcept; void execute(FEngine& engine, const char* name) const noexcept; @@ -420,7 +423,7 @@ class RenderPass { uint8_t channel, Pass pass, CustomCommand custom, uint32_t order, Executor::CustomCommandFn command); - static Command* resize(Arena& arena, Command* const last) noexcept; + static Command* resize(Arena& arena, Command* last) noexcept; // sorts commands then trims sentinels static Command* sortCommands( @@ -461,7 +464,7 @@ class RenderPass { FScene::RenderableSoa const& mRenderableSoa; ColorPassDescriptorSet const* const mColorPassDescriptorSet; - backend::Viewport const mScissorViewport; + backend::Viewport const mScissorViewport{ 0, 0, INT32_MAX, INT32_MAX }; Command const* /* const */ mCommandBegin = nullptr; // Pointer to the first command Command const* /* const */ mCommandEnd = nullptr; // Pointer to one past the last command mutable BufferObjectSharedHandle mInstancedUboHandle; // ubo for instanced primitives @@ -509,6 +512,8 @@ class RenderPassBuilder { return *this; } + // Specifies the viewport for the scissor rectangle, that is, the final scissor rect is + // offset by the viewport's left-top and clipped to the viewport's width/height. RenderPassBuilder& scissorViewport(backend::Viewport viewport) noexcept { mScissorViewport = viewport; return *this; diff --git a/filament/src/details/Renderer.cpp b/filament/src/details/Renderer.cpp index 9ebad01c7b4..acb869a8bfd 100644 --- a/filament/src/details/Renderer.cpp +++ b/filament/src/details/Renderer.cpp @@ -1108,7 +1108,9 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { // FIXME: we should use 'vp' when rendering directly into the swapchain, but that's hard to // know at this point. This will usually be the case when post-process is disabled. // FIXME: we probably should take the dynamic scaling into account too - passBuilder.scissorViewport(hasPostProcess ? xvp : vp); + // if MSAA is enabled, we end-up rendering in an intermediate buffer. This is the only case where + // "!hasPostProcess" doesn't guarantee rendering into the swapchain. + passBuilder.scissorViewport(hasPostProcess || msaaOptions.enabled ? xvp : vp); // This one doesn't need to be a FrameGraph pass because it always happens by construction // (i.e. it won't be culled, unless everything is culled), so no need to complexify things.