metal: flip + clamp scissor against the active render pass extent#643
Open
benface wants to merge 1 commit into
Open
metal: flip + clamp scissor against the active render pass extent#643benface wants to merge 1 commit into
benface wants to merge 1 commit into
Conversation
5c75f29 to
4ade8f2
Compare
`apply_scissor_rect` was using `crate::window::screen_size()` to
derive the Y-flip pivot, which only matches the active render
pass's height for the default drawable. Two separate problems:
1. **Off-screen targets at high DPI.** When the active pass is an
off-screen render target with different pixel dimensions —
typical at high DPI, where logical render-target sizes are
smaller than the physical drawable — the pivot is wrong and
the resulting `MTLScissorRect` lands entirely outside the
target. Every fragment then gets scissor-clipped, silently
dropping all draws to the off-screen target. Surfaced by
trying to render a high-DPI MSAA off-screen target on macOS;
uniformly nothing reached the resolve texture.
2. **Mid-rotation race on iOS.** During an iPad / iPhone rotation
animation, the caller's view of `screen_size` (cached via
`Resize` events) can lead MTKView's actual `currentDrawable`
by a frame, and on the in-flight frame the caller asks for a
scissor sized against the new dimensions while the encoder is
still bound to the previous-size drawable. Metal validation
then fires:
-[MTLDebugRenderCommandEncoder setScissorRect:]: failed
assertion `Set Scissor Rect Validation
(rect.y(137) + rect.height(2491))(2628) must be <=
render pass height(2622)'
Reproduced on iPhone 16 Pro by rotating mid-frame; only the
debug-validated build asserts, release silently clips.
Track the active pass's pixel width + height in `begin_pass`,
read straight from the descriptor's color attachment texture
(not `screen_size()` — which is the cache that's wrong in case
2). Then in `apply_scissor_rect`, clamp the resulting rect into
`[0, pass_size]` on both axes. The default-drawable case for a
correctly-cached size is unchanged; the off-screen high-DPI case
gets the right pivot; the mid-rotation case clips an at-most one
frame of over-large scissor to the actual pass extent instead
of asserting.
4ade8f2 to
30d4724
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
apply_scissor_rectwas usingcrate::window::screen_size()to derive the Y-flip pivot, which only matches the active render pass's height for the default drawable. Two separate problems fall out of that:1. Off-screen targets at high DPI
When the active pass is an off-screen render target with different pixel dimensions — typical at high DPI, where logical render-target sizes are smaller than the physical drawable — the pivot is wrong and the resulting
MTLScissorRectlands entirely outside the target. Every fragment then gets scissor-clipped, silently dropping all draws to the off-screen target.Surfaced by trying to render a high-DPI MSAA off-screen target on macOS; uniformly nothing reached the resolve texture.
2. Mid-rotation race on iOS
During an iPad / iPhone rotation animation, the caller's cached view of
screen_size(updated viaResizeevents) can lead MTKView's actualcurrentDrawableby a frame. On the in-flight frame the caller asks for a scissor sized against the new dimensions while the encoder is still bound to the previous-size drawable. Metal validation fires:Reproduced on iPhone 16 Pro by rotating mid-frame on a debug-validated build; release silently clips and you don't notice unless you launch from Xcode.
Fix
begin_passnow reads the active pass's width + height straight from the descriptor's color-attachment texture (notscreen_size()— the cache that's wrong during the rotation race). Stored incurrent_pass_width/current_pass_height.apply_scissor_rectflips Y againstcurrent_pass_height(this is the off-screen fix), then clamps the resulting rect into[0, pass_size]on both axes (the rotation-race defensive band-aid).The default-drawable case with a correctly-cached
screen_sizeis functionally unchanged. The off-screen high-DPI case gets the right pivot and draws land in the target. The mid-rotation case clips at most one frame of over-large scissor to the actual pass extent instead of asserting.The mid-rotation race itself is upstream of the renderer (
MTKView.drawableSizeupdates beforecurrentDrawable.textureis recreated, and apps caching screen size fromResizenotifications observe the new size one frame ahead). Fixing the race at the source would require either deferring theResizenotification until the drawable actually resizes, or having apps read the drawable size at draw time. Both are larger surface-area changes than the clamp here, which gets the validation green at the cost of one frame of scissor truncation during the transition.