Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VideoCommon: Treat invalid normal count as NormalTangentBinormal #11207

Merged
merged 1 commit into from Oct 25, 2022

Conversation

Pokechu22
Copy link
Contributor

See https://bugs.dolphin-emu.org/issues/13070.

I hardware tested this using the codcamp.dff fifolog from that issue, along with a version I hex-edited, using the hardware fifoplayer: codcamp.7z.zip. Note that the last object in this fifolog needs to be disabled to get rid of a white screen (and the one before it can be removed to disable bloom). Specifically, I edited object 476 on frame 0, at offsets 00175d49 and 00175d54. The default version uses XFMEM_VTXSPECS with Num normals: Invalid (3) and CP_VAT_REG_A with Normal elements: 3 (normal, tangent, binormal) (1), while my hex-edited version uses Num normals: Normal only (1) and Normal elements: 1 (normal) (0). (I had to hex-edit it so that I had working base versions, as changing two fields at once could result in hangs if a frame gets drawn when only one of them is changed).

Here are my results (rows XFMEM_VTXSPECS, columns CP_VAT_REG_A):

Normal Normal, Tangent, Binormal
None (10) Hang Hang
Normal only (14) Works Hang
NTB (18) Hang Works
Invalid (1C) Hang Works

The hardware fifoplayer didn't make it easy to test no normals enabled at all (since that would involve editing primitive data). I assume that it hangs if XFMEM_VTXSPECS isn't set to None, though.

I also confirmed that Dolphin is rendering the scene (particularly the reflections on the bag) correctly.

@JMC47
Copy link
Contributor

JMC47 commented Oct 25, 2022

Change is very simple and I checked the game. There's a popup + messages in older builds. No popup in this build.

image

@JMC47 JMC47 merged commit 9ef7a3b into dolphin-emu:master Oct 25, 2022
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants