Halo 2 specific fixes

[polymetal0]

Some Halo 2 specific fixes and an additional general dynamic shadow fix

After some testing with nv2a_pgraph_tests (I could NOT do the z24s8 tests), thanks to @GXTX advice, I closed the previous pull request and opened this one in order to tidy things up.

I'll be posting some results comparison for each of the commits, and also split the changes in different commits for anyone to be able to keep track of them, since they are quite subtle and documentation is quite poor.

[polymetal0]

Commit 2cc9f7a : full dynamic shadows + skybox

• No noticeable changes in nv2a_pgraph_tests
• Fully restores skybox
• Fully restores dynamic shadows (mainly characters)

Examples:

[polymetal0]

Commit e2b98d5 : Cutscene lighting fix

• No noticeable changes in nv2a_pgraph_tests
• Apparently, there was some sort of overlapping issue. Leaving out the case for oPos.w = oPos.z in perspective fixes this

Examples:

[polymetal0]

Commit 637b5ef : See through fix:

• Small noticeable changes in nv2a_pgraph_tests. Only one that might be considered "for worse" or "too different"
• Produces flickering in some textures (paintings/stickers on walls, reflections, lights in enemies armors). We also lose Johnson and Chief in one of the first cutscenes of the game
• There's also this sort of hard fog/light cutoff in open air levels
• Fixes see through issue

Change in tests:

Examples:

[polymetal0]

Commit 89c5f92 : General fix for other games: dynamic shadows

• No noticeable changes in tests
• Restores dynamic shadows (mainly characters) in other games (tested in Conker and Halo CE)

Sorry, I did not know how to branch this out of the other changes, since it is important for the new code to be in the else statement for z_perspective.

Examples:

[0Alastair1]

Does cause some fog regressions - but does bring fog back to areas where there is no fog.

Xbox:

Xemu v0.7.122 (and v0.7.121)

Xemu v0.7.122 with pr

[mborgerson]

Thanks for the contribution!

Here are some thoughts to improve this submission:

• Pull Request Process: There's no need to close and open a new PR if the intent of the submission is the same. Feel free to clean up and/or rebase development branches then force push updates to your existing branch; your PR will be updated on GitHub accordingly.
• Robustness: We need to make sure stability and robustness are increasing. Although regressions sometimes happen, changes that fix some areas and knowingly causes other issues suggest we need a deeper understanding of the system. This means we need to do more research to understand how the hardware works in these cases. The nxdk_pgraph_tests repository is a great resource for developing specific hardware tests. Will you consider taking the task of developing specific hardware tests for these cases to provide strong support for the proposed change?
• Technical Notes: Clamping depth to 1.0 seems reasonable for integer depth format as higher values cannot be represented but we need hardware tests to confirm, and the other changes around z_perspective that correct some depth issues may not be correct so we need more tests for this also. This has been on my radar for a while; and if memory serves a former development branch with a similar approach caused issues in other games.

[polymetal0]

@0Alastair1 Awesome! it also looks like weapon/hand lighting got improved as well. Could you please try each commit individually and see which one causes the regressions? it'll most likely be 637b5ef. I'm working on it, just wanted to upload that change for you all to do this kind of testing. I feel like it could be a problem related to fog or lighting rather than depth.

@mborgerson Thank you for the remarks, the tests have definitely been a useful tool. It would be nice to expand them, but I wouldn't know where to begin. What do you suggest? and how feasible would it be to add some nv2a documentation? Also notice that clamping takes place in the else statement, only when z_perspective=false. This fixed the shadows without altering the tests nor any other thing I could notice (it even might improve performance in Halo CE when zoom-aiming). This may not be an absolute fix, but we're getting close. If you remembered the old dev branch or the other games where this caused issues, it would be really helpful 👍

[mborgerson]

@polymetal0

I wouldn't know where to begin. What do you suggest?

Take a look at the existing test cases. Typically it involves building a test matrix that exercise combinations of configurations and values through the system and capturing the result for analysis and comparison against xemu. This can be a bit tedious, but fortunately you already have xemu as a guide for what configuration options and values you will expect to see. You can use a graphics debugger like renderdoc to inspect issues and refine your test cases.

how feasible would it be to add some nv2a documentation

The test cases, test results, nxdk, and xemu code are the best documentation we have. If you are interested in writing additional documentation, you are welcome to do so.

If you remembered the old dev branch or the other games where this caused issues, it would be really helpful

Unfortunately, I do not.

[polymetal0]

I had to revert the shadows because it caused problems when working on the see through issue, and it didn't do anything else that I knew of. Skybox is reimplemented in next commit.

Commit 4e72fc5: Improvements on see through fix:

• Flickering persists as per 637b5ef
• The fog/lighting issue also persists
• Cutscenes seem perfect so far (outside of forementioned issues)

Persisting issues examples :

Release

PR

[PacketTrace]

It's great to see this is being worked on. This issue makes the game unplayable for me.

[polymetal0]

I have pretty much come to a dead end. I'm leaving it here for now since these changes could make it to the main branch without any regression I could find.

Shadows are also fixed as in previous commits, but the see through issue remains. It's trickier than I had anticipated...I don't really know if there's something else involved I'm missing, but I have fixed a few things along the way that improve the experience, so enjoy :)

[mborgerson]

Per my previous comment, I'm not planning to merge these changes without adequate testing that addresses the root of the problem. We need to understand more about how the hardware actually works in these cases. That said I do appreciate the effort you put in to this

[polymetal0]

@mborgerson

I don't understand what you mean. As I commented, I rolled back the changes regarding the see through fix precisely because I could not actually identify the problem. Putting this one aside, the remaining changes still make the emulator behave closer to the original hardware with no regressions.

It was clear to me that the root problem was in the vertex shader. Bringing gl_ClipDistance[0] closer fixes shadows, fog...and forcing gl_ClipDistance[1] to never be less than oPos.w*clipRange.w brings back Skybox, which makes perfect sense. The changes in the code can be reduced to basically this (in vsh.c):

gl_ClipDistance[0] = clipRange.x; de-normalized clipRange.z also works in case you don't want to have this be 0.0 (clipRange.x equals 0) gl_ClipDistance[1] = oPos.w*clipRange.w - min(0.0, oPos.z); ensuring there won't be a subtraction of a positive z value, always resulting in a sum.

There was also the general dynamic shadow fix. Clamping the z coordinate to 1 worked with no apparent new issues, but that one can be left out if you find it too "patchy". While working on this I also found out that the z_perspective variable is never (or almost never) set to true in many games (like Halo CE), that's also why I coded two differentiated behaviors for this (which until now are virtually the same, but could vary in the future).

The code might be a bit messy, since I ended up working on many issues at the same time, but you could have at least put the branch as a draft again, to undergo further testing after my last commit...I'll send another PR with the changes I described hoping they will be tested in other cases. Please, at least find out what's wrong before closing it, because I'd also like to know. And if everything is fine and the PR makes it to the next release, all the better.

[ryzendew]

I agree don’t toss this aside instead learn from it and find proper fixes i have tested it in halo 2 and other titles and saw zero regressions only improvement.

[mborgerson]

@polymetal0

Please, at least find out what's wrong before closing it, because I'd also like to know.

I do not plan to debug your code. In my previous comment I asked for hardware test cases to help support your implementation. Screenshots from a game do not fulfill this request.

@ryzendew

If you're interested in helping the project, you are welcome to help with building test cases. Unsolicited PM input is not something the project requires.

[polymetal0]

@mborgerson

What? you don't need to debug anything. The test results were unaltered when compared to those of the latest release, and I have made no new implementations that require new test cases, since I have rolled back everything related to the see through fix as per my latest comment and commit, Again, I have just made a subtle little change in two parameters in the vertex shader, which knowingly fixes stuff without any known regression.

I'll send the new PR when I can, just please don't close it right away and let other contributors try it out and maybe come up with new test cases.

[antangelo]

The pgraph tests are not exhaustive and don't exercise every GPU configuration. Not seeing regressions in the test cases just means that your code hasn't broken those paths, but doesn't necessarily mean that your code is generally correct.

You'll want to create your own tests that cover the sections of code you're modifying so that you can validate your changes against what real hardware produces over the relevant inputs.