Fix wrong face culling once and for all

[gdkchan]

NVN exposes a function that allows the application to set the origin of the window. It can be set to upper left (following Direct3D conventions), or lower left (following OpenGL). This should be directly equivalent to glClipControl on OpenGL.

The way how this is actually implemented on the GPU, however, is not directly equivalent. The GPU has a bit that changes something on the rasterization stage to take the different origin into account (nouveau calls this bit TriangleRastFlip). It does not flip the viewport, however. Flipping the viewport is done by using a negative value for the viewport scale.

So when the UpperLeft mode is used, NVN does 2 things:

• Negates the viewport Y scale value. This effectively flips the viewport, so all the geometry is flipped on the Y direction. This also affects face culling as the orientation of the triangles will change if you flip them.
• It sets the TriangleRastFlip bit to 0 (to indicate the UpperLeft origin is being used, LowerLeft is 1). This negates the effects that the above viewport flipping would have on the face culling, as it changes something during rasterization.

So in most cases, directly setting origin to UpperLeft or LowerLeft using glClipControl should be safe and equivalent to NVN behavior, as long the above conditions holds (that is, either TriangleRastFlip is 0 and the viewport scale Y is negative, or TriangleRastFlip is 1 and viewport scale Y is positive). However, some games cheats and this may not be the case.

One example of such a game is Xenoblade. It manages to bypass the step where NVN sets the TriangleRastFlip bit to 0 by only setting the origin to upper left after queue initialization. This effectively causes NVN to still use negative viewport scale Y values, while not setting triangle rast flip to 0, because during queue initialization, origin was still lower left.

Approach used to fix the issue

We can still use ARB_clip_control to emulate TriangleRastFlip behavior, however the viewport flipping might be undesirable. We can negate that part by flipping the viewport again. To my knowledge, per-viewport flipping is possible, but only using NVIDIA NV_viewport_swizzle extension, only available on Maxwell and newer. For GPUs without the support, we can still do the flipping on the vertex shader, but it affects all the viewports.

The fix here uses NV_viewport_swizzle (or shader emulation) to flip the viewport as needed. glClipControl is still used, but origin is now set to LowerLeft or UpperLeft based on the value of TriangleRastFlip bit. The various combinations of bits and values that may flip it in the Y direction (negative Y scale, Y negate etc) are propagated to the viewport swizzle Y component.

Note: The shader implementation currently has another issue, on top of not supporting per-viewport flipping. Since we don't recompile shaders when the state changes, the shader may be assuming a stale/incorrect viewport swizzle state. However this is a existing issue.

---

I recommend testing this on a few games, including the ones that had culling issues, and games that used to work fine, to ensure it does not regress anything. If you find issues, please also note the GPU it was tested on (basically, we need to know if its using NV_viewport_swizzle or the shader implementation).

Should fix culling issues on Xenoblade 2, and maybe other games.

This supersedes #912

[marysaka]

nice catch 👍

[riperiperi]

Looking good apart from 2 typos. I'm glad this was finally found out.

I think we'll need some more tests on the games mentioned in the last PR, just to make sure all bases are covered.

[EmulationFanatic]

Tested this PR via Appveyor build 1.0.4624 on Disgaea 4 Complete Plus vs 1.0.4640 master.
Screenshots Below:

Master :

PR 1277 build :

---

Master :

PR 1277 Build :

---

Master :

PR 1277 Build :

---

Master :

PR 1277 Build :

[EmulationFanatic]

Tested again with the latest rebased version (1.0.4641) and everything is still looking good!

[riperiperi]

LGTM!

[Cristallix]

onlyy => typo

[gdkchan]

I will fix the typo when I touch this file again, since the PR was already merged.

[Cristallix]

Why not using the default of the switch ?

[gdkchan]

It prints some information on the log before returning. What would be the advantage of using the default case here?

[Cristallix]

Suggested change

	Logger.PrintDebug(LogClass.Gpu, $"Invalid {nameof(ViewportSwizzle)} enum value: {swizzle}.");
	return NvViewportSwizzle.ViewportSwizzlePositiveXNv;
	default:
	Logger.PrintDebug(LogClass.Gpu, $"Invalid {nameof(ViewportSwizzle)} enum value: {swizzle}.");
	return NvViewportSwizzle.ViewportSwizzlePositiveXNv;

You can do your log and then return in the default. To me it's more consistent.