nv2a: Ignore color/depth mask in CLEAR_SURFACE

[abaire]

Fixes #730 and matches observed HW behavior where NV097_SET_COLOR_MASK/NV097_SET_DEPTH_MASK masking is not respected by CLEAR_SURFACE call (where masking is set via a parameter).

Test
Golden results

[antangelo]

Does this also work if you use pg->clearing instead of is_clear?

[abaire]

Ha, I wish I'd noticed that before piping through a bunch of bools :) Thanks for the tip, update to follow shortly.

[Triticum0]

I think this bug also fixes Full spectrum Warrior: 10 Hammers, Midnight Club 3: DUB Edition,Beyond Good & Evil, #140?(issue not documented well there over 10 different assertion with the same name), #387 and #288. All have the same assertion to my knowledge but don't know if the bugs are related. I not going to test so don't ask but games are probably fixed with this pr.

[abaire]

Thanks for the pointers!

I think we'll need to farm this out to the discord to get people to re-test those games post-merge to verify. Unfortunately the glGetError assert doesn't specify what the actual error is, so some of those failures could be due to totally different root causes. Similarly, it's possible to get into a framebuffer != complete status a few different ways so it's possible that even the #387 failures are due to different problems.

[antangelo]

I think this bug also fixes Full spectrum Warrior: 10 Hammers, Midnight Club 3: DUB Edition,Beyond Good & Evil, #140?(issue not documented well there over 10 different assertion with the same name), #387 and #288. All have the same assertion to my knowledge but don't know if the bugs are related. I not going to test so don't ask but games are probably fixed with this pr.

#140 is not related to this. That issue stems from SET_BEGIN_END and isn't related to clearing.

[mborgerson]

I think we'll need to farm this out to the discord to get people to re-test those games post-merge to verify.

Preferably things should be tested before being merged. Feel free to ask whomever filed the last compat report to help test a PR

[abaire]

I think we'll need to farm this out to the discord to get people to re-test those games post-merge to verify.

Preferably things should be tested before being merged. Feel free to ask whomever filed the last compat report to help test a PR

Agreed in general, but in this particular case we have a fix for one particular cause of a generic error message that could be triggered many different ways.

Re-testing every game that has been found to have a generic gl failure (which presumably could even be driver-related) seems like it's going to burn the good will of testers and I'd argue that it's better to get known issues fixed than delay in the hope that some other issues might've been fixed unknowingly.

Alternatively/in-addition, perhaps we can expand on the error messaging in gl error cases so this is a non-issue in the future? Perhaps we could have the debug build leverage glDebugMessageCallback and log the underlying error so we don't have to rely on a developer to try to repro to make the issue de-dupable (this is what I happen to do on my work branch)

[mborgerson]

I think we'll need to farm this out to the discord to get people to re-test those games post-merge to verify.

Preferably things should be tested before being merged. Feel free to ask whomever filed the last compat report to help test a PR

Agreed in general, but in this particular case we have a fix for one particular cause of a generic error message that could be triggered many different ways.

Re-testing every game that has been found to have a generic gl failure (which presumably could even be driver-related) seems like it's going to burn the good will of testers and I'd argue that it's better to get known issues fixed than delay in the hope that some other issues might've been fixed unknowingly.

Alternatively/in-addition, perhaps we can expand on the error messaging in gl error cases so this is a non-issue in the future? Perhaps we could have the debug build leverage glDebugMessageCallback and log the underlying error so we don't have to rely on a developer to try to repro to make the issue de-dupable (this is what I happen to do on my work branch)

If the problem is too generic, we should do the work to isolate what the actual failures are and begin cataloging and fixing them individually. glDebugMessageCallback is an option although sometimes it is not quite as helpful as you'd expect/hope.

[abaire]

If the problem is too generic, we should do the work to isolate what the actual failures are and begin cataloging and fixing them individually. glDebugMessageCallback is an option although sometimes it is not quite as helpful as you'd expect/hope.

Totally agree. I'm happy to file an issue to replace/augment any glGetError assertions with something that either provides sufficient information to determine the underlying issue or at least makes it clear to non-developers that more info is required to be actionable.

If the debug messaging is insufficient I don't personally have enough of a GL background to know what alternative would be viable (other than checking for errors after every GL call and logging the actual error value along with the assert, which presumably would have enough of a perf impact to be non-viable for the release build).

As an aside, I'd suggest a similar thing for uses of assert(false); if we assert(!"Some explanation of the issue") the assertion message becomes better future-proofed and more immediately actionable. At the moment things like pgraph change frequently enough that the line numbers go out of date fast and it'd reduce the need to switch back to old release branches to understand GH issues.

[Triticum0]

#745 also need to be retested with this commit

[Triticum0]

Full-Spectrum-Warrior-Ten-Hammers is fixed with this pull