flibitijibibo / FNA-MGHistory

FNA - Accuracy-focused XNA4 reimplementation for open platforms
http://fna-xna.github.io/
246 stars 37 forks source link

[Graphics] [OpenGL] Clipping bug: triangle rendered behind an orthographic camera #282

Closed elisee closed 9 years ago

elisee commented 9 years ago

This is a fun one. The following minimal game displays a black screen in XNA but a triangle is (incorrectly) drawn in FNA:

using Microsoft.Xna.Framework;
using Microsoft.Xna.Framework.Graphics;

namespace ConsoleApplication1
{
    class TestGame: Game
    {
        GraphicsDeviceManager graphics;
        BasicEffect effect;

        public TestGame()
        {
            graphics = new GraphicsDeviceManager(this);
        }

        protected override void  Initialize()
        {
            base.Initialize();

            effect = new BasicEffect( GraphicsDevice );
            effect.VertexColorEnabled = true;
        }

        protected override void Draw(GameTime gameTime)
        {
            GraphicsDevice.Clear( Color.Black );

            effect.Projection = Matrix.CreateOrthographic( 10f, 10f, 0.1f, 1000f );
            effect.CurrentTechnique.Passes[0].Apply();

            effect.GraphicsDevice.DrawUserPrimitives<VertexPositionColor>( PrimitiveType.TriangleList, new VertexPositionColor[] {
                new VertexPositionColor( new Vector3( -1f, -1f, 5f ), Color.Red ),
                new VertexPositionColor( new Vector3(  1f,  1f, 5f ), Color.Green ),
                new VertexPositionColor( new Vector3(  1f, -1f, 5f ), Color.Blue ),
            }, 0, 1 );

            base.Draw(gameTime);
        }
    }
}

The triangle's Z position (5f) implies it is behind the orthographic camera and as such should not be rendered.

Comparison, FNA on the left, XNA on the right:

image

elisee commented 9 years ago

I'm pretty sure this issue also exists in MonoGame, it's a long-standing bug but I only just got around to turning it into a minimal test case.

Another thing of note: this is not specific to the BasicEffect, I'm encountering this bug with a simple custom shader too.

flibitijibibo commented 9 years ago

Not sure off the top of my head. This is something that may require staring at a frame in apitrace and looking at the parameters to see what we need to add to get the accurate behavior.

flibitijibibo commented 9 years ago

Ready for some possible disappointment?

https://www.opengl.org/sdk/docs/man/html/glClipControl.xhtml https://www.opengl.org/registry/specs/ARB/clip_control.txt

I'm pretty sure we're looking for glClipControl(GL_LOWER_LEFT, GL_ZERO_TO_ONE) to fix this. Unfortunately it's so new even NVIDIA's driver doesn't have this right now. Looks to be standard in OpenGL 4.5.

This is all a guess though. Even then, we'd probably need to work around this somehow for older drivers/hardware.

flibitijibibo commented 9 years ago

Here's something that might be a nice way to get away from OpenGL for a moment...

http://flibitijibibo.com/MatrixWrong.patch

This is a redo of the CreateOrthographic* calls that seems to make the triangle go away while keeping compatibility, hopefully (seems okay for Apotheon). I have no idea if this is in any way accurate; I'd like to see comparisons to XNA4 with these new calculations (don't have a Windows box open atm).

EDIT: My other games seem just fine... wtf? Were these functions wrong the whole time?

(This is what happens when you ask a music major to debug mathematics code!)

elisee commented 9 years ago

With the patch, the Projection matrix ends up being:

{M11:0,2 M12:0 M13:0 M14:0} {M21:0 M22:0,2 M23:0 M24:0} {M31:0 M32:0 M33:-0,0020002 M34:0} {M41:0 M42:0 M43:-1,0002 M44:1}

While XNA's is:

{M11:0,2 M12:0 M13:0 M14:0} {M21:0 M22:0,2 M23:0 M24:0} {M31:0 M32:0 M33:-0,0010001 M34:0} {M41:0 M42:0 M43:-0,00010001 M44:1}

(Excuse the colons instead of dots everywhere, French number formatting rules)

So M43 is now -1.0002 instead of -0,00010001. Previously they matched.

I just tested with one of the CraftStudio games (running with FNA) that exhibited the bug. The quad that used to appear erronously doesn't anymore but some of the stuff that actually is in front of the camera has disappeared too :)

flibitijibibo commented 9 years ago

Yeah, I'm guessing the original is right... not just because of the test matchup, but because it turns out this code comes directly from Win2D, with an additional commit for (double) casts:

https://github.com/Microsoft/Win2D/blob/master/numerics/DotNet/Matrix4x4.cs#L633

At least this time it's not under GPL!

I would continue looking at ways to get glClipControl without a 4.5 driver... not sure what that may entail.

elisee commented 9 years ago

I think I found an okay workaround for my specific case for now. Sharing in the hope it will be useful:

I store the opposite of the transformed position Z component in the unused Z component of my vertex shader output's tex coords:

VertexShaderOutput VertexShaderFunction(VertexShaderInput input)
{
    VertexShaderOutput output;
    // ...
    output.TexCoord.z = -wvPosition.z;
}

And then I clip any pixel with a Z < 0 in the pixel shader:

float4 PixelShaderFunction(VertexShaderOutput input) : COLOR0
{
    // ...
    clip( input.TexCoord.z );
    // ...
    return color;
}

Seems to work fine.

flibitijibibo commented 9 years ago

Adding this commit to the thread since it's sortakinda related, except not really:

https://github.com/flibitijibibo/FNA/commit/5f586a4114294d6752f77d54f763550c633cc7ae

flibitijibibo commented 9 years ago

I finally updated my driver to NVIDIA 346.47, which has 4.5 support. Sure enough, glClipControl fixes this.

This may be something I put into a ModernGLDevice, but that's still require some kind of bool ClipDepthEXT that notes what the active backend would need to simulate clip control.

flibitijibibo commented 9 years ago

Going to close this, having both added glClipControl to FNA (will be used when available)...

https://github.com/flibitijibibo/FNA/commit/99019befdc4eb6d2de31706d0e89b7532affce3e

... and added some docs to the wiki for fixing clip control when ARB_clip_control is not available (which is most of the time):

https://github.com/flibitijibibo/FNA/wiki/2:-Building-with-FNA#depth-clipping-accuracy

Unfortunately there's not much else we can do at this point, unless we were to make enough IGLDevice backends that properly support this and cover the hardware that does not support the necessary extensions. Maybe if Vulkan's driver support goes back to enough hardware this one won't be so bad.

If the docs need any clarification, or a better example can be made, definitely look into that side of the issue.

elisee commented 9 years ago

Great job! :+1:

flibitijibibo commented 9 years ago

I think we found a fix for this.

I just pushed this change to MojoShader: https://github.com/flibitijibibo/MojoShader/commit/8e5037d1cafab148bac87b1b5d1dbdd4bc32f69e

Apparently this will reliably change depth values from [0, 1] to [-1, 1], without the need for clip control or shader content changes.

This actually works nicely for my current port, but it may need some testing.

flibitijibibo commented 9 years ago

Just pushed a commit removing ARB_clip_control...

https://github.com/flibitijibibo/FNA/commit/a4c92ac64e69cee2327da73f139de7154e6c196c

... because it really does seem to work without any problems. fnalibs.tar.bz2 has been updated accordingly.