Align rasterizer state, readback paths, cull mode, triangle winding order across backends#1127
Open
EmilioLaiso wants to merge 4 commits intollvm:mainfrom
Open
Align rasterizer state, readback paths, cull mode, triangle winding order across backends#1127EmilioLaiso wants to merge 4 commits intollvm:mainfrom
EmilioLaiso wants to merge 4 commits intollvm:mainfrom
Conversation
Contributor
|
I'm fine with either winding mode, but this feels like something we should add to the documentation. |
manon-traverse
approved these changes
Apr 28, 2026
85e485f to
a63bea2
Compare
a63bea2 to
def5e69
Compare
Contributor
|
The CI is failing on a Mandelbrot test. @EmilioLaiso Could you check if it uses a golden image and if that is still correct? Perhaps there is still a bug in the texture readbacks? |
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.
Summary
Make the three backends use the same image coordinate system, same culling mode and same
SV_IsFrontFacepolarity.The framework had the following setup:
Front-face polarity differed.
DX/MTL Y-flip clip→framebuffer at the viewport; Vulkan with positive-height viewport doesn't.
Same HLSL triangle ended up with opposite framebuffer winding on VK vs DX/MTL, so
SV_IsFrontFacegave opposite answers for the same input. This surfaced while writing a graphics-stage system-values test that neededSV_IsFrontFaceto agree across backends.Visual PNG output was aligned by two compensating flips.
DX and MTL each had a row-reversal on readback documented as "render target origin is top-left, while our image writer expects bottom-left".
The PNG writer reversed rows again on the way out.
VK had neither flip, but VK's no-Y-flip viewport meant its framebuffer was already "upside down" relative to DX/MTL, so its single PNG flip happened to produce a visually identical image. Two-flips-cancel on DX/MTL, no-flip-needed on VK, by accident.
Adding any new behavior on top of this (e.g., the negative-height-viewport change in the earlier stage of this PR that kicked off the investigation) immediately broke the cancellation and produced visually-flipped outputs.
PR
Pick HLSL/DX semantics as canonical:
Untangled the asymmetry by removing the readback flips on DX/MTL and the PNG writer's row reversal together with the VK viewport flip, so all three places change as one atomic step. After that, every backend produces the same Y-down memory layout end-to-end.
Exposed shared
copyFromTexturefunction that all backends can call.If #1113 goes in main, which introduces
Map/Unmapas functions on theBuffertype, the readback path can be generalized further.Golden Images
All graphics golden images still work as expected. The resulting Y-order is maintained.
The
Mandelbrot.testimage required updating as it's a compute path. Here's the PR: llvm/offload-golden-images#6