Repository navigation
MUFU.SIN and MUFU.COS of a large number of turns keep their fraction before Metal's fast sin / cos - #1234
Closed
lunks wants to merge 1 commit into
Closed
MUFU.SIN and MUFU.COS of a large number of turns keep their fraction before Metal's fast sin / cos#1234lunks wants to merge 1 commit into
lunks wants to merge 1 commit into
Conversation
…before Metal's fast sin / cos: Bananza's SSAO at full size had no occlusion past x*y about 659,000 Ampere's MUFU.SIN / COS take the angle in turns and use only its fraction. The translator multiplied the turns by 2*pi in float and called Metal's sin / cos: the product rounds the fraction away (a step of 0.4 rad at 2^20 turns), and fast sin / cos return 0 from about 2^20 turns on. Donkey Kong Bananza's SSAO (a6f04605183d9915) rotates its kernel per pixel by an angle that grows with x*y; past x*y of about 659,000 the rotation became (0, 0) and the lower right of the picture had no ambient occlusion. The SSAO runs at half the scene's size: at the docked top of dynamic resolution (1152x648) that is a wedge in the corner (5,425 pixels), with the effects at full size (1676x943) a fifth of the picture; at 838x471 there is no hole, but angles off by up to 0.23 rad. Any Ampere shader passing sin / cos an angle that large had the same fault; only Bananza was checked. AMufu now lowers both as sin / cos((t - rint(t)) * 2*pi): t - rint(t) is exact and in [-0.5, 0.5], where the fast functions are accurate. Maxwell code is unchanged. Checked: t_mufu_turns compares the lowering on the GPU with a double reference over the SSAO's angles at 838x471, 1152x648, 1676x943 and 2560x1440 (largest error 2e-7; the old lowering collapsed 0, 5,425, 345,255 and 1,893,254 rotations). regress.py check on Bananza: 23,638 of 23,638 shaders build on both sides, 0 newly failing, 8,258 with changed MSL, 0 interface changes.
seedds
added a commit
that referenced
this pull request
Nov 7, 2026
…before Metal's fast sin / cos: Donkey Kong Bananza's SSAO no longer loses its ambient occlusion in the lower right (chain 05b67a35081d, PR #1234) Ampere's MUFU.SIN / COS take turns and use only the fraction, exact for any float. The translator multiplied the turns by 2pi in float and called Metal's sin / cos: the product rounds the fraction away (0.4 rad steps at 2^20 turns) and Metal's fast sin / cos return 0 from about 2^20 turns on. Bananza's SSAO (a6f04605183d9915) rotates its kernel by an angle growing with x*y, so past x*y = 659,000 the rotation was (0, 0) and the lower right had no occlusion. AMufu now lowers both as sin / cos((t - rint(t)) * 2pi) (ATurnsToRadians): t - rint(t) is exact and in [-0.5, 0.5], where the fast functions are accurate, and a small t is unchanged. Maxwell's MUFU, which follows RRO in radians, is untouched. Checked here: t_mufu_turns on this M4 Pro's GPU over the SSAO's angles at 838x471, 1152x648, 1676x943 and 2560x1440: largest error 2e-7 and no collapsed rotation (the old lowering collapses 0, 5,425, 345,255 and 1,893,254); test_dmsh_ampere_mem MufuTurns fails on the translator before and passes; the 21 translator test files (test_dmsh_pairs' two NameErrors as on the old translator), test_nsp_chain, test_xlate_anycpu, test_docs. Not run here: Bananza (no library or seen list on this Mac); the author's before/after shots are in the PR. Regression: one Ocarina of Time run on the chain that ends this batch. Co-authored-by: lunks <91118+lunks@users.noreply.github.com>
Owner
|
Landed as b594834 with you as co-author (translator rebuilt here; t_mufu_turns and MufuTurns pass, MufuTurns fails before). Thank you. |
Aznracer
pushed a commit
to Aznracer/Mighty-Green-Kokiri-Kid
that referenced
this pull request
Nov 8, 2026
…before Metal's fast sin / cos: Donkey Kong Bananza's SSAO no longer loses its ambient occlusion in the lower right (chain 05b67a35081d, PR seedds#1234)
2 tasks done
Know-It-All-Brother
pushed a commit
to Know-It-All-Brother/nxnative
that referenced
this pull request
Nov 9, 2026
…before Metal's fast sin / cos: Donkey Kong Bananza's SSAO no longer loses its ambient occlusion in the lower right (chain 05b67a35081d, PR seedds#1234) Ampere's MUFU.SIN / COS take turns and use only the fraction, exact for any float. The translator multiplied the turns by 2pi in float and called Metal's sin / cos: the product rounds the fraction away (0.4 rad steps at 2^20 turns) and Metal's fast sin / cos return 0 from about 2^20 turns on. Bananza's SSAO (a6f04605183d9915) rotates its kernel by an angle growing with x*y, so past x*y = 659,000 the rotation was (0, 0) and the lower right had no occlusion. AMufu now lowers both as sin / cos((t - rint(t)) * 2pi) (ATurnsToRadians): t - rint(t) is exact and in [-0.5, 0.5], where the fast functions are accurate, and a small t is unchanged. Maxwell's MUFU, which follows RRO in radians, is untouched. Checked here: t_mufu_turns on this M4 Pro's GPU over the SSAO's angles at 838x471, 1152x648, 1676x943 and 2560x1440: largest error 2e-7 and no collapsed rotation (the old lowering collapses 0, 5,425, 345,255 and 1,893,254); test_dmsh_ampere_mem MufuTurns fails on the translator before and passes; the 21 translator test files (test_dmsh_pairs' two NameErrors as on the old translator), test_nsp_chain, test_xlate_anycpu, test_docs. Not run here: Bananza (no library or seen list on this Mac); the author's before/after shots are in the PR. Regression: one Ocarina of Time run on the chain that ends this batch. Co-authored-by: lunks <91118+lunks@users.noreply.github.com>
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.
What and why
Ampere's
MUFU.SIN/COStake the angle in turns and use only its fraction, which is exact for any float. The translator multiplied the turns by 2π in float and called Metal'ssin/cos. The product rounds the fraction away (a step of 0.4 rad at 2^20 turns), and Metal's fastsin/cosreturn 0 from about 2^20 turns on.Found on Donkey Kong Bananza's SSAO (
a6f04605183d9915). It rotates its kernel per pixel by an angle that grows with x·y. Past x·y ≈ 659,000 the rotation is (0, 0), every sample lands on the pixel itself, and the lower right of the picture has no ambient occlusion, bounded by an arc centred off screen. The SSAO runs at half the scene's size, which dynamic resolution moves:Any Ampere shader that passes
sin/cosan angle that large, such as a per-pixel rotation built from pixel coordinates, has the same fault.Fix:
AMufulowers both assin/cos((t − rint(t)) · 2π)(ATurnsToRadiansinInstEmitAmpereAlu.cs).t − rint(t)is exact and in [−0.5, 0.5], where the fast functions are accurate. Maxwell code is unchanged; itsMUFUfollowsRROand takes radians.Reference entry: MUFU.SIN and MUFU.COS of a large number of turns.
Checked
t_mufu_turns(new, GPU) compares the lowering with a double-precision reference over the SSAO's angles at 838x471, 1152x648, 1676x943 and 2560x1440. Largest error 2e-7, no collapsed rotations. The old lowering collapses 0, 5,425, 345,255 and 1,893,254.test_dmsh_ampere_mem.pyMufuTurnschecks the lowered MSL.make -C dm/gfx/tests GAME=040074A01BF12000 runpasses.scripts/regress.py check --games 040074A01BF12000, on main at 4447be1 (the branch was then rebased over Mario Kart World 1.8.0 (v917504) on main: plays well, one sky artifact #1186's c0 offsets, which changed only the chain header here): CHANGED.regress.py impactlists Breath of the Wild, Pokémon Scarlet and Tears of the Kingdom only through the chain header. They are Maxwell games, whose code this does not change.Docs
docs/reference/shaders.mdand the game note, andpython3 dm/tools/docs_symptoms.pywas rundmsh_chain.py --headerwas run anddm/gfx/nvn/dmsh_chain.his part of the change