diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index 205d3e0ee7..91e649250d 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -851,17 +851,11 @@ void MetalDriver::beginRenderPass(Handle rth, encoding:NSUTF8StringEncoding]; } - MTLRegion mvp = renderTarget->getRegionFromClientRect(params.viewport); - MTLViewport metalViewport { - .originX = static_cast(mvp.origin.x), - .originY = static_cast(mvp.origin.y), - .width = static_cast(mvp.size.width), - .height = static_cast(mvp.size.height), - .znear = static_cast(params.depthRange.near), - .zfar = static_cast(params.depthRange.far) - }; + MTLViewport metalViewport = + renderTarget->getViewportFromClientViewport(params.viewport, + params.depthRange.near, + params.depthRange.far); [mContext->currentRenderPassEncoder setViewport:metalViewport]; - mContext->currentViewport = metalViewport; // Metal requires a new command encoder for each render pass, and they cannot be reused. @@ -1287,20 +1281,37 @@ void MetalDriver::draw(PipelineState ps, Handle rph, uint32_t } // Set scissor-rectangle. + // In order to do this, we compute the intersection between: + // 1. the scissor rectangle + // 2. the current viewport + // 3. the render target attachment dimensions (important, as the scissor can't be set larger) + // fmax/min are used below to guard against NaN and because the MTLViewport/MTLRegion + // coordinates are doubles. MTLRegion scissor = mContext->currentRenderTarget->getRegionFromClientRect(ps.scissor); - const MTLViewport& viewport = mContext->currentViewport; + const float sleft = scissor.origin.x, sright = scissor.origin.x + scissor.size.width; + const float stop = scissor.origin.y, sbottom = scissor.origin.y + scissor.size.height; - // fmax/min are used here to guard against NaN and because the MTLViewport coordinates are doubles. - const auto left = std::fmax(viewport.originX , scissor.origin.x ); - const auto right = std::fmin(viewport.originX + viewport.width , scissor.origin.x + scissor.size.width ); - const auto top = std::fmax(viewport.originY , scissor.origin.y ); - const auto bottom = std::fmin(viewport.originY + viewport.height, scissor.origin.y + scissor.size.height ); + // Viewport extent + const MTLViewport& viewport = mContext->currentViewport; + const float vleft = viewport.originX, vright = viewport.originX + viewport.width; + const float vtop = viewport.originY, vbottom = viewport.originY + viewport.height; + + // Attachment extent + const auto attachmentSize = mContext->currentRenderTarget->getAttachmentSize(); + const float aleft = 0.0f, atop = 0.0f; + const float aright = static_cast(attachmentSize.x); + const float abottom = static_cast(attachmentSize.y); + + const auto left = std::fmax(std::fmax(sleft, vleft), aleft); + const auto right = std::fmin(std::fmin(sright, vright), aright); + const auto top = std::fmax(std::fmax(stop, vtop), atop); + const auto bottom = std::fmin(std::fmin(sbottom, vbottom), abottom); MTLScissorRect scissorRect = { .x = static_cast(left), - .y = static_cast(top ), + .y = static_cast(top), .width = static_cast(right - left), - .height = static_cast(bottom - top ) + .height = static_cast(bottom - top) }; [mContext->currentRenderPassEncoder setScissorRect:scissorRect]; diff --git a/filament/backend/src/metal/MetalHandles.h b/filament/backend/src/metal/MetalHandles.h index c4863f75be..41bf5af5a2 100644 --- a/filament/backend/src/metal/MetalHandles.h +++ b/filament/backend/src/metal/MetalHandles.h @@ -35,6 +35,8 @@ #include #include +#include + #include #include #include @@ -288,8 +290,21 @@ public: void setUpRenderPassAttachments(MTLRenderPassDescriptor* descriptor, const RenderPassParams& params); + MTLViewport getViewportFromClientViewport( + Viewport rect, float depthRangeNear, float depthRangeFar) { + const int32_t height = int32_t(getAttachmentSize().y); + assert_invariant(height > 0); + + // Metal's viewport coordinates have (0, 0) at the top-left, but Filament's have (0, 0) at + // bottom-left. + return {double(rect.left), + double(height - rect.bottom - int32_t(rect.height)), + double(rect.width), double(rect.height), + double(depthRangeNear), double(depthRangeFar)}; + } + MTLRegion getRegionFromClientRect(Viewport rect) { - const uint32_t height = getAttachmentHeight(); + const uint32_t height = getAttachmentSize().y; assert_invariant(height > 0); // Convert the Filament rect into Metal texture coordinates, taking into account Metal's @@ -301,6 +316,8 @@ public: rect.width, rect.height); } + math::uint2 getAttachmentSize() noexcept; + bool isDefaultRenderTarget() const { return defaultRenderTarget; } uint8_t getSamples() const { return samples; } @@ -316,8 +333,6 @@ private: static id createMultisampledTexture(id device, MTLPixelFormat format, uint32_t width, uint32_t height, uint8_t samples); - uint32_t getAttachmentHeight() noexcept; - MetalContext* context; bool defaultRenderTarget = false; uint8_t samples = 1; @@ -325,7 +340,7 @@ private: Attachment color[MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT] = {}; Attachment depth = {}; Attachment stencil = {}; - uint32_t attachmentHeight = 0; + math::uint2 attachmentSize = {}; }; // MetalFence is used to implement both Fences and Syncs. diff --git a/filament/backend/src/metal/MetalHandles.mm b/filament/backend/src/metal/MetalHandles.mm index 5ae91315a1..2855be15dd 100644 --- a/filament/backend/src/metal/MetalHandles.mm +++ b/filament/backend/src/metal/MetalHandles.mm @@ -28,7 +28,6 @@ #include #include -#include #include #include @@ -790,8 +789,8 @@ MetalRenderTarget::MetalRenderTarget(MetalContext* context, uint32_t width, uint uint8_t samples, Attachment colorAttachments[MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT], Attachment depthAttachment, Attachment stencilAttachment) : HwRenderTarget(width, height), context(context), samples(samples) { - UTILS_UNUSED_IN_RELEASE math::vec2 tmin = {std::numeric_limits::max()}; - UTILS_UNUSED_IN_RELEASE math::vec2 tmax = {0}; + math::uint2 tmin = {std::numeric_limits::max()}; + UTILS_UNUSED_IN_RELEASE math::uint2 tmax = {0}; UTILS_UNUSED_IN_RELEASE size_t attachmentCount = 0; for (size_t i = 0; i < MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT; i++) { @@ -880,9 +879,9 @@ MetalRenderTarget::MetalRenderTarget(MetalContext* context, uint32_t width, uint // The render target dimensions must be less than or equal to the attachment size. assert_invariant(width <= tmin.x && height <= tmin.y); - // Store the height of all attachments. We'll use this when converting from Filament's + // Store the width/height of all attachments. We'll use this when converting from Filament's // coordinate system to Metal's. - attachmentHeight = tmin.y; + attachmentSize = tmin; } void MetalRenderTarget::setUpRenderPassAttachments(MTLRenderPassDescriptor* descriptor, @@ -1065,15 +1064,16 @@ id MetalRenderTarget::createMultisampledTexture(id device return [device newTextureWithDescriptor:descriptor]; } -uint32_t MetalRenderTarget::getAttachmentHeight() noexcept { +math::uint2 MetalRenderTarget::getAttachmentSize() noexcept { if (defaultRenderTarget) { // The default render target always has a color attachment. auto colorAttachment = getDrawColorAttachment(0); assert_invariant(colorAttachment); - return colorAttachment.getTexture().height; + id t = colorAttachment.getTexture(); + return {t.width, t.height}; } - assert_invariant(attachmentHeight > 0); - return attachmentHeight; + assert_invariant(attachmentSize.x > 0 && attachmentSize.y > 0); + return attachmentSize; } MetalFence::MetalFence(MetalContext& context) : context(context), value(context.signalId++) { } diff --git a/filament/backend/test/test_Scissor.cpp b/filament/backend/test/test_Scissor.cpp index b7e4f31c3a..a7fea104bd 100644 --- a/filament/backend/test/test_Scissor.cpp +++ b/filament/backend/test/test_Scissor.cpp @@ -164,4 +164,102 @@ TEST_F(BackendTest, ScissorViewportRegion) { getDriver().purge(); } +// Verify that a negative Viewport origin works with scissor. +TEST_F(BackendTest, ScissorViewportEdgeCases) { + auto& api = getDriverApi(); + + api.startCapture(0); + + // The test is executed within this block scope to force destructors to run before + // executeCommands(). + { + // Create a SwapChain and make it current. We don't really use it so the res doesn't matter. + auto swapChain = api.createSwapChainHeadless(256, 256, 0); + api.makeCurrent(swapChain, swapChain); + + // Create a program. + ShaderGenerator shaderGen(triangleVs, triangleFs, sBackend, sIsMobilePlatform); + Program p = shaderGen.getProgram(api); + ProgramHandle program = api.createProgram(std::move(p)); + + // Create a source color textures. + Handle srcTexture = api.createTexture(SamplerType::SAMPLER_2D, 1, + TextureFormat::RGBA8, 1, 512, 512, 1, + TextureUsage::SAMPLEABLE | TextureUsage::COLOR_ATTACHMENT); + + // Render into the bottom-left quarter of the texture, checking 3 special cases. + // 1. negative viewport left/bottom + Viewport bottomLeftViewport = { + .left = -64, + .bottom = -64, + .width = 256, + .height = 256 + }; + // 2. viewport that extends beyond the top of the render target. + Viewport topLeftViewport = { + .left = -64, + .bottom = 320, + .width = 256, + .height = 256 + }; + // 3. A scissor rect > render target size. + // This is precisely what Filament does to essentially convey "no scissor". + Viewport scissor = {0, 0, (uint32_t)std::numeric_limits::max(), + (uint32_t)std::numeric_limits::max()}; + + Handle renderTarget = api.createRenderTarget( + TargetBufferFlags::COLOR, 512, 512, 1, + {srcTexture, 0, 0}, {}, {}); + + TrianglePrimitive triangle(api); + + // Render a white triangle over blue. + RenderPassParams params = {}; + params.flags.clear = TargetBufferFlags::COLOR0; + params.viewport = bottomLeftViewport; + params.clearColor = math::float4(0.0f, 0.0f, 1.0f, 1.0f); + params.flags.discardStart = TargetBufferFlags::ALL; + params.flags.discardEnd = TargetBufferFlags::NONE; + + PipelineState ps = {}; + ps.program = program; + ps.rasterState.colorWrite = true; + ps.rasterState.depthWrite = false; + ps.scissor = scissor; + + api.makeCurrent(swapChain, swapChain); + api.beginFrame(0, 0); + + api.beginRenderPass(renderTarget, params); + api.draw(ps, triangle.getRenderPrimitive(), 1); + api.endRenderPass(); + + params.viewport = topLeftViewport; + params.flags.clear = TargetBufferFlags::NONE; + params.flags.discardStart = TargetBufferFlags::NONE; + api.beginRenderPass(renderTarget, params); + api.draw(ps, triangle.getRenderPrimitive(), 1); + api.endRenderPass(); + + readPixelsAndAssertHash( + "ScissorViewportEdgeCases", 512, 512, renderTarget, 0x6BF00F31, true); + + api.commit(swapChain); + api.endFrame(0); + + api.stopCapture(0); + + // Cleanup. + api.destroyTexture(srcTexture); + api.destroySwapChain(swapChain); + api.destroyRenderTarget(renderTarget); + } + + // Wait for the ReadPixels result to come back. + api.finish(); + + executeCommands(); + getDriver().purge(); +} + } // namespace test