Skip to content

Commit

Permalink
Revert "avoid setting the scissor rect when possible"
Browse files Browse the repository at this point in the history 
This reverts commit fc09541.

clipping is wrong when post-processing is disabled
  • Loading branch information
@pixelflinger
pixelflinger committed Oct 17, 2024
1 parent fc09541 commit ba680cf
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 56 deletions.
13 changes: 1 addition & 12 deletions filament/backend/include/backend/DriverEnums.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,20 +318,9 @@ 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: 21 additions & 29 deletions filament/src/RenderPass.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,12 @@ 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 @@ -877,15 +883,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 const l = scissor.left + scissorViewport.left;
int32_t const b = scissor.bottom + scissorViewport.bottom;
int32_t l = scissor.left + scissorViewport.left;
int32_t 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
assert_invariant(l == std::max(l, scissorViewport.left));
assert_invariant(b == std::max(b, scissorViewport.bottom));
l = std::max(l, scissorViewport.left);
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 @@ -907,14 +913,9 @@ void RenderPass::Executor::execute(FEngine& engine,
if (first != last) {
SYSTRACE_VALUE32("commandCount", last - first);

// 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;
bool const scissorOverride = mScissorOverride;
if (UTILS_UNLIKELY(scissorOverride)) {
// initialize with scissor override
driver.scissor(mScissor);
}

Expand Down Expand Up @@ -998,24 +999,17 @@ void RenderPass::Executor::execute(FEngine& engine,

if (UTILS_UNLIKELY(mi != info.mi)) {
// this is always taken the first time
assert_invariant(info.mi);

mi = info.mi;
assert_invariant(mi);

ma = mi->getMaterial();

// 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
if (UTILS_LIKELY(!scissorOverride)) {
backend::Viewport scissor = mi->getScissor();
if (hasScissorViewport) {
// apply the scissor viewport if any
scissor = applyScissorViewport(mScissor, scissor);
}
if (scissor != currentScissor) {
currentScissor = scissor;
driver.scissor(scissor);
if (UTILS_UNLIKELY(mi->hasScissor())) {
scissor = applyScissorViewport(mScissorViewport, scissor);
}
driver.scissor(scissor);
}

if (UTILS_LIKELY(!polygonOffsetOverride)) {
Expand Down Expand Up @@ -1106,18 +1100,16 @@ RenderPass::Executor::Executor(RenderPass const& pass, Command const* b, Command
mInstancedUboHandle(pass.mInstancedUboHandle),
mInstancedDescriptorSetHandle(pass.mInstancedDescriptorSetHandle),
mColorPassDescriptorSet(pass.mColorPassDescriptorSet),
mScissor(pass.mScissorViewport),
mScissorViewport(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),
mHasScissorViewport(false) {
mScissorOverride(false) {
}

RenderPass::Executor::Executor(Executor&& rhs) noexcept = default;
Expand Down
25 changes: 10 additions & 15 deletions filament/src/RenderPass.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,17 +350,12 @@ class RenderPass {
BufferObjectSharedHandle mInstancedUboHandle;
DescriptorSetSharedHandle mInstancedDescriptorSetHandle;
ColorPassDescriptorSet const* mColorPassDescriptorSet = nullptr;
// 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;
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

Executor(RenderPass const& pass, Command const* b, Command const* e) noexcept;

Expand All @@ -387,6 +382,8 @@ 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 @@ -423,7 +420,7 @@ class RenderPass {
uint8_t channel, Pass pass, CustomCommand custom, uint32_t order,
Executor::CustomCommandFn command);

static Command* resize(Arena& arena, Command* last) noexcept;
static Command* resize(Arena& arena, Command* const last) noexcept;

// sorts commands then trims sentinels
static Command* sortCommands(
Expand Down Expand Up @@ -464,7 +461,7 @@ class RenderPass {

FScene::RenderableSoa const& mRenderableSoa;
ColorPassDescriptorSet const* const mColorPassDescriptorSet;
backend::Viewport const mScissorViewport{ 0, 0, INT32_MAX, INT32_MAX };
backend::Viewport const mScissorViewport;
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 @@ -512,8 +509,6 @@ 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

0 comments on commit ba680cf

Please sign in to comment.