Skip to content

Fix GX_NRM_NBT and GX_NRM_NBT3 vertex attribute handling#181

Merged
encounter merged 3 commits into
encounter:mainfrom
Ahmad8864:gx-nbt-crash-fix
Jun 4, 2026
Merged

Fix GX_NRM_NBT and GX_NRM_NBT3 vertex attribute handling#181
encounter merged 3 commits into
encounter:mainfrom
Ahmad8864:gx-nbt-crash-fix

Conversation

@Ahmad8864

@Ahmad8864 Ahmad8864 commented May 13, 2026

Copy link
Copy Markdown
Contributor

Fixes the crash path for #93. Aurora was crashing on GX_NRM_NBT / GX_NRM_NBT3 vertex attributes in a few places: comp_cnt_count only handled GX_NRM_XYZ, the VAT-A decoder ignored the bit-31 escape and collapsed NBT3 into NBT, and the shader generator tried to emit a nonexistent fetch_f32_9. All three are fixed here.

The follow-up commit addresses @magcius's review: NBT3 indexed mode now reads three separate indices per vertex from the DL buffer (one each for N, B, T) instead of treating it as a single-index NBT, and GX_TG_BINRM / GX_TG_TANGENT are supported as texgen sources, the binormal and tangent slices of GX_VA_NRM are loaded on-demand when a sampled texgen consumes them. The latest commit implements emboss bump mapping (GX_TG_BUMP0–7), offsetting the source texcoord by the light direction projected onto the binormal and tangent.

All 180 existing tests still pass, though I don't have an NBT test ROM to visually verify the output.

@dbalatoni13

Copy link
Copy Markdown
Contributor

Thanks a lot! I will test it.

@magcius

magcius commented May 13, 2026

Copy link
Copy Markdown
Contributor

Even without implementing bump, you should still be able to add support for GX_TG_BINRM / GX_TG_TANGENT as texgen sources. I don't think this implements NBT3 indexing correctly either (you need to read three indices from the DL buffer).

@Ahmad8864

Copy link
Copy Markdown
Contributor Author

Even without implementing bump, you should still be able to add support for GX_TG_BINRM / GX_TG_TANGENT as texgen sources. I don't think this implements NBT3 indexing correctly either (you need to read three indices from the DL buffer).

I'll take a look at adding texgen support and revisit the NBT3 indexing, thanks

@Ahmad8864

Copy link
Copy Markdown
Contributor Author

@magcius @dbalatoni13 Sorry for the delay, addressed the NBT3 review feedback and added binormal/tangent texgen support. Would you have a chance to test?

@dbalatoni13

Copy link
Copy Markdown
Contributor

Looks correct to me graphically

@Ahmad8864

Copy link
Copy Markdown
Contributor Author

Hi @encounter does this PR look good to you?

@encounter

Copy link
Copy Markdown
Owner

It does, thanks for the implementation!

@encounter encounter merged commit 2c1e8ed into encounter:main Jun 4, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants