Skip to content

Commit

Permalink
make scissor() work the same way on all backends
Browse files Browse the repository at this point in the history
scissor() works like on metal now, that is, it is disabled when a
render pass starts.

The GL backend already assumed this in debug mode. We cannot really run
into issues at the moment because every time we get a new 
MaterialInstance we set the scissor -- we do this both for the
color pass and the post-process passes. With this change we will be
able to skip setting the scissor altogether in a lot of cases.
  • Loading branch information
pixelflinger committed Sep 24, 2024
1 parent 67f37d4 commit fe15a30
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 8 deletions.
6 changes: 3 additions & 3 deletions filament/backend/src/opengl/OpenGLDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2917,6 +2917,9 @@ void OpenGLDriver::beginRenderPass(Handle<HwRenderTarget> rth,
GLuint const fbo = gl.bindFramebuffer(GL_FRAMEBUFFER, rt->gl.fbo);
CHECK_GL_FRAMEBUFFER_STATUS(utils::slog.e, GL_FRAMEBUFFER)

// each render-pass starts with a disabled scissor
gl.disable(GL_SCISSOR_TEST);

if (gl.ext.EXT_discard_framebuffer
&& !gl.bugs.disable_invalidate_framebuffer) {
AttachmentArray attachments; // NOLINT
Expand All @@ -2929,7 +2932,6 @@ void OpenGLDriver::beginRenderPass(Handle<HwRenderTarget> rth,
// It's important to clear the framebuffer before drawing, as it resets
// the fb to a known state (resets fb compression and possibly other things).
// So we use glClear instead of glInvalidateFramebuffer
gl.disable(GL_SCISSOR_TEST);
clearWithRasterPipe(discardFlags & ~clearFlags, { 0.0f }, 0.0f, 0);
}

Expand All @@ -2945,7 +2947,6 @@ void OpenGLDriver::beginRenderPass(Handle<HwRenderTarget> rth,
}

if (any(clearFlags)) {
gl.disable(GL_SCISSOR_TEST);
clearWithRasterPipe(clearFlags,
params.clearColor, (GLfloat)params.clearDepth, (GLint)params.clearStencil);
}
Expand All @@ -2964,7 +2965,6 @@ void OpenGLDriver::beginRenderPass(Handle<HwRenderTarget> rth,

#ifndef NDEBUG
// clear the discarded (but not the cleared ones) buffers in debug builds
gl.disable(GL_SCISSOR_TEST);
clearWithRasterPipe(discardFlags & ~clearFlags,
{ 1, 0, 0, 1 }, 1.0, 0);
#endif
Expand Down
13 changes: 8 additions & 5 deletions filament/backend/src/vulkan/VulkanDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,11 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
VulkanCommandBuffer& commands = mCommands.get();
VkCommandBuffer const cmdbuffer = commands.buffer();

// Scissor is reset with each render pass
// This also takes care of VUID-vkCmdDrawIndexed-None-07832.
VkRect2D const scissor{ .offset = { 0, 0 }, .extent = extent };
vkCmdSetScissor(cmdbuffer, 0, 1, &scissor);

UTILS_NOUNROLL
for (uint8_t samplerGroupIdx = 0; samplerGroupIdx < Program::SAMPLER_BINDING_COUNT;
samplerGroupIdx++) {
Expand Down Expand Up @@ -1901,11 +1906,6 @@ void VulkanDriver::bindPipeline(PipelineState const& pipelineState) {
mPipelineCache.bindLayout(pipelineLayout);
mPipelineCache.bindPipeline(commands);

// Since we don't statically define scissor as part of the pipeline, we need to call scissor at
// least once. Context: VUID-vkCmdDrawIndexed-None-07832.
auto const& extent = rt->getExtent();
scissor({0, 0, extent.width, extent.height});

FVK_SYSTRACE_END();
}

Expand Down Expand Up @@ -1976,6 +1976,9 @@ void VulkanDriver::scissor(Viewport scissorBox) {
VulkanCommandBuffer& commands = mCommands.get();
VkCommandBuffer cmdbuffer = commands.buffer();

// TODO: it's a common case that scissor() is called with (0, 0, maxint, maxint)
// we should maybe have a fast path for this and avoid vkCmdSetScissor() if possible

// Set scissoring.
// clamp left-bottom to 0,0 and avoid overflows
constexpr int32_t maxvali = std::numeric_limits<int32_t>::max();
Expand Down

0 comments on commit fe15a30

Please sign in to comment.