Repository navigation
Conversation
Several invalid inputs reached the backends unchecked, where they are undefined behavior (the engine-side checks are compiled out in release builds) or abort under the Metal API validation layer. RenderPass now throws for: - a texture without render target usage used as an attachment; - attachments with different sample counts; - a color format used as the depth-stencil attachment, or a depth-stencil format used as a color attachment; - a non-zero mip level on a backend that cannot render into one; - a render target with no attachments; - a vertex or index BufferView that extends past its buffer; - drawIndexed with more indices than the bound index buffer holds; - setColorBlendEnable/setColorBlendEquation for a color attachment that does not exist; - a scissor rectangle that does not lie within the render target. The existing ColorAttachment and DepthStencilAttachment checks (MSAA resolve texture format, size and sample count, transient load actions) ran only inside an assert; they now run with the others in release builds too.
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.
Several invalid inputs went from
RenderPassstraight to the backends, where the checks are debug-only validation logs or missing, so in a release build they are undefined behavior: an attachment created without render target usage, attachments with different sample counts, a color format as the depth-stencil attachment or a depth-stencil format as a color one, a non-zero mip level on a backend that can't render into one, aBufferViewthat runs past its buffer,drawIndexedwith more indices than the bound index buffer holds, blend state for an attachment that doesn't exist, a scissor outside the render target, and a render target with no attachments at all, which crashes Vulkan. Under the Metal API validation layer the usage and scissor cases abort the process.All of these are checked in
render_pass.dartnow and throw before the call reaches native code. The resolve-texture and transient-load checks that already existed ran only inside anassert;_validateAttachmentscalls them unconditionally, so they hold in release too. An out-of-target scissor throws rather than being clamped, since it is a bug in the caller like every other case here. I looked at the viewport as well: one larger than the target draws fine on all three backends and passes the Metal debug layer, so I left it alone.Code that passes one of these inputs today and happens to render on its backend will now get an exception. I think that's right for undefined behavior, but it is a change in behavior, and the checks are independent of each other, so I can split this up if that's easier to review. It doesn't overlap #192535 and #192536, the C++ Vulkan case of a target with no color attachment: a depth-only target is still allowed here.
Where it was verified. macOS arm64,
host_debug_unopt_arm64, rebased on current main, with onegpu_test.darttest per check (11). Without the change 9 fail on Metal, 10 underflutter_tester_opengles(the mip-level case only applies there), Vulkan on SwiftShader crashes with SIGSEGV on the empty render target, and Metal withMTL_DEBUG_LAYER=1aborts on the usage case. With it all of them pass on the three backends, Metal under the debug layer included. The resolve-texture test passes either way underflutter_tester, which always runs Dart with asserts on, so it can't show the release gap; it pins the messages.Fixes #194155
Pre-launch Checklist
///).