Repository navigation
lower_to_maxwell: a REGMAP marker is never a number another field of the instruction names (overexposed foliage, #696, #667) - #905
Closed
raxis666-cmyk wants to merge 1 commit into
Conversation
…the instruction names (overexposed foliage, NotRexed#696, NotRexed#667) A high GPR in the scratch range is renamed to a marker of the uniform bank, chosen among the numbers no field seen so far named. A field after it could name the same number: OoT's foliage mesh d38b0d52 has FFMA R247, R180, R216, R247, R247 took the marker 216, and the prefix then mapped 216 for both fields (the run for a native R216 comes last and takes it back). The light probes' weighted average divided by data instead of weights: stalks, grass blades and flowers came out about 8x too bright. The marker now skips every number any register field of the instruction names. tests/test_lower_regmap_marker.py lowers four such FFMAs, applies the prefix as ApplyRegisterMap does and checks each field names its native register (fails on dev without the change, passes with it). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
|
Thank you for tracking this down! The same marker collision (a REGMAP marker equal to a number another field of the instruction names: the glowing foliage stems) is fixed on main by #922, and #759's version of the fix has been taken into the maintainer's tree for the next release; this branch now conflicts with both. Closing as superseded. If you have a case that still collides with main plus #759, please open an issue or a small follow-up with it. |
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 was wrong
When a native instruction names a high GPR in the scratch range (R246 up), the lowering renames that field to a
marker (a uniform-bank number, R216-R245) and the REGMAP_nx2 prefix maps the marker to the extended register.
The marker was picked among the numbers no field seen so far had named, so a field processed later could name the
same number:
d38b0d52a94ab5a8(program89ca750f9852b7ca: stalks, grass blades, flower stems) uses GPRs upto R252. Its
FFMA R247, R180, R216, R247gave R247 the marker 216.If R216 is extended itself, its run comes after the renamed ones and
ApplyRegisterMap(last run wins) maps themarker back to R216, so the FFMA writes over R216.
the probe average ended up summing data instead of weights, and the ambient (SH) irradiance of the stalks came out
about 8x too high.
That's the "overexposed / glowing stalks, grass and flowers" in #696 (RX 9070 XT) and #667 (also RTX 3060 Ti): it is
not vendor-specific.
The fix
The marker search also skips every number any register field of the instruction names (with the same span rules the
renaming uses), so a marker can't alias another field. Instructions with no such collision lower to the same words as
before.
How it was found and checked
0.18 0.41 0.19to0.024 0.018 0.010; the leaves next to it are0.023 0.021 0.009. The glowing stalks and bladesare gone; ferns, plants and flowers are all still drawn.
(5-28), while the shader's average came out at 264, which is how the broken divisor showed up.
--prepare-shaders: 31985 programs, 0 failed. A walking run stayed stable.tests/test_lower_regmap_marker.py(registered intests/run_all.py): it lowers four FFMAswith high GPRs, applies the prefix the way
ApplyRegisterMapdoes and checks that every Maxwell field names itsnative register. It fails on
devwithout the fix (e.g.Rb 503for R216) and passes with it.test_lower_atomg_cas.pyandtest_lower_depbar.pystill pass.The in-game measurements were made on a tree based on
3f5b0aa(fast-updates). This branch applies the same changeto current
dev(232eecb), keeping dev'sregister_fieldlist (TXD 0xb6c and ATOMG 0x9a8 included).Fixes #696, fixes #667.
🤖 Generated with Claude Code