Skip to content

Commit

Permalink
avoid setting the scissor rect when possible
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pixelflinger committed Oct 18, 2024
1 parent a494f9f commit 4425338
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 33 deletions.
13 changes: 12 additions & 1 deletion filament/backend/include/backend/DriverEnums.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
50 changes: 29 additions & 21 deletions filament/src/RenderPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -883,15 +877,15 @@ backend::Viewport RenderPass::Executor::applyScissorViewport(
// clang vectorizes this!
constexpr int32_t maxvali = std::numeric_limits<int32_t>::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);
Expand All @@ -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);
}

Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;
Expand Down
25 changes: 15 additions & 10 deletions filament/src/RenderPass.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion filament/src/details/Renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 4425338

Please sign in to comment.