ACR GitHub integration test 967 isolated#2
Draft
amarpMSFT wants to merge 1 commit into
Draft
Conversation
… filters The CalculateMeanVariance and DisocclusionBlur compute shaders exchanged each row's values across wave lanes assuming WaveGetLaneIndex() == SV_GroupIndex (row-major thread->lane packing) and a wave width >= 16. HLSL guarantees neither: for 2D thread groups (these use [numthreads(8,8,1)]) the SM 6.6 derivatives spec explicitly states no relation between SV_GroupIndex and WaveGetLaneIndex() should be assumed. On WARP (wave width 4) the reads address lanes that do not exist and the denoiser output is wrong on every interior pixel. Changes: - Make a portable groupshared-memory row exchange the default permutation (correct on any device); keep the original wave-intrinsic exchange as an opt-in fast path behind the RTAO_WAVE_READ_LANE_PATH define (thin wrapper shaders CalculateMeanVarianceCS_WaveReadLaneAt / DisocclusionBlur3x3CS_ WaveReadLaneAt). - Select the fast path at runtime, per kernel, only when the device reports wave ops and WaveLaneCountMin >= 16 (SupportsWaveIntrinsicDenoiserFilterPath). - Expose it as a "Filter row exchange" EnumVar; the option collapses to groupshared-only (SetListLength(1)) on devices that cannot run the wave path. - Apply the same fix to the currently-unused DepthAwareSeparableGaussianFilter 3x3CS_AnyToAnyWaveReadLaneAt variant for consistency. - Remove now-stale measured performance numbers from the readme and shader comments; note the preview video predates the fix. Validated on WARP against a CPU reference: the groupshared path matches to half-precision, while the wave path is wrong on 576/576 mean/variance and 35/36 disocclusion interior pixels.
amarpMSFT
commented
Jul 16, 2026
amarpMSFT
left a comment
amarpMSFT
left a comment
Owner
Author
There was a problem hiding this comment.
[ACR-TEST-TEMP] live smoke review
amarpMSFT
commented
Jul 16, 2026
amarpMSFT
left a comment
amarpMSFT
left a comment
Owner
Author
There was a problem hiding this comment.
[ACR-TEST-TEMP] live smoke review
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
1 participant
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.
Private-fork integration fixture for AgentCodeReview GitHub PR support. Base and head are copied refs pinned to the same commits as upstream PR 967. Safe for comments, reviews, resolution, and added test iterations. Do not merge.