FNA-XNA / FNA

FNA - Accuracy-focused XNA4 reimplementation for open platforms
https://fna-xna.github.io/
2.66k stars 271 forks source link

Access Violation & FatalExecutionEngineError after using VertexBuffer.GetData #125

Closed jsmars closed 7 years ago

jsmars commented 7 years ago

I'm using a code snippet from Jitter3D Physics Engine to extract vertex and index data from a Model for use as a collision model, but after of the usage of the MeshModelPart.VertexBuffer.GetData, I randomly get 'Access Violation' crash at startup, or FatalExecutionEngineError right after startup.

Managed Debugging Assistant 'FatalExecutionEngineError' : 'The runtime has encountered a fatal error. The address of the error was at 0x713b5af2, on thread 0x3488. The error code is 0xc0000005. This error may be a bug in the CLR or in the unsafe or non-verifiable portions of user code. Common sources of this bug include user marshaling errors for COM-interop or PInvoke, which may corrupt the stack.'

The program '[5684] Apollo.exe' has exited with code -1073741819 (0xc0000005) 'Access violation'.

I've tried using different two different FBX models with the same result. I've also tried the same code in a XNA environment, where I don't get any errors. The XNA test was just an empty project running the code though.

Am I missing something here or is the FNA implementation of GetData doing something shady?

Here is the full code snippet:

       private void ExtractData(List<Vector3> vertices, List<TriangleVertexIndices> indices, Model model)
        {
            Matrix[] bones_ = new Matrix[model.Bones.Count];
            model.CopyAbsoluteBoneTransformsTo(bones_);
            foreach (ModelMesh mm in model.Meshes)
            {
                Matrix xform = bones_[mm.ParentBone.Index];
                foreach (ModelMeshPart mmp in mm.MeshParts)
                {
                    int offset = vertices.Count;
                    Vector3[] a = new Vector3[mmp.NumVertices];
                    mmp.VertexBuffer.GetData<Vector3>(mmp.VertexOffset *
                          mmp.VertexBuffer.VertexDeclaration.VertexStride, 
                          a, 0, mmp.NumVertices, 
                          mmp.VertexBuffer.VertexDeclaration.VertexStride);
                    for (int i = 0; i != a.Length; ++i)
                        Vector3.Transform(ref a[i], ref xform, out a[i]);
                    vertices.AddRange(a);

                    if (mmp.IndexBuffer.IndexElementSize != IndexElementSize.SixteenBits)
                        throw new Exception("Model uses 32-bit indices, which are not supported.");
                    short[] s = new short[mmp.PrimitiveCount * 3];
                    mmp.IndexBuffer.GetData<short>(mmp.StartIndex * 2, s, 0, mmp.PrimitiveCount * 3);
                    TriangleVertexIndices[] tvi = new TriangleVertexIndices[mmp.PrimitiveCount];
                    for (int i = 0; i != tvi.Length; ++i)
                    {
                        tvi[i].I0 = s[i * 3 + 0] + offset;
                        tvi[i].I1 = s[i * 3 + 1] + offset;
                        tvi[i].I2 = s[i * 3 + 2] + offset;
                    }
                    indices.AddRange(tvi);
                }
            }
flibitijibibo commented 7 years ago

I'll definitely need a sample for this one. Could be the struct packing being different, the destination array not being large enough, something not being pinned, threads getting in the way, and so on and so forth.

flibitijibibo commented 7 years ago

Took one more look at the code snippet; what's the VertexDeclaration like? I'm wondering if you've got a buffer with something like this...

struct PartVertex
{
    Vector3 position;
    Vector2 texcoord;
}

... but with a request of Vector3[] just for the positions. The reason I ask is because this would mean the resulting array would actually need to be much larger than what you have to store, so you get to the glGetBufferSubData call...

https://github.com/FNA-XNA/FNA/blob/master/src/FNAPlatform/OpenGLDevice.cs#L2260

... where vertexStride is much larger than elementSizeInBytes (20 for PartVertex vs. 12 for Vector3, for example), resulting in a buffer overflow. I'd be interested in knowing if XNA has the same problem or if it has a staging buffer and clips off the final result with a big :shrug:

In any case this would be... kind of a bug in FNA? Though the real bug would be that you're asking for a subregion of the vertex for each vertex you're asking for, which is possible to implement but would be craaaaaazy slow. I would double check the vertex type and make sure your types match in size, then you can pull what you want from the struct once it's been copied from the buffer.

flibitijibibo commented 7 years ago

It's been a while since we've had a super obscure thing for the OpenGLDevice to fix!

I'm like 99% sure this will fix your problem:

https://github.com/FNA-XNA/FNA/commit/eaac17ff7833024670c89469f62696238eb2bf56

But note all that extra work we're doing behind the scenes... you'll probably get a faster result if you match the vertex size on your end and do the math on the struct elements you care about afterward.

jsmars commented 7 years ago

Thanks for such a quick rescue as always! The VertexDeclaration is a standard Position/Normal/Texture from FBX, making it 32 bytes in size, which is quite a bit bigger than just the 12 for a Vector3. I figured the GetData would account for this since it doesn't throw an exception on the requested type vs the underlying one. For my situation I'm loading a collision model so I don't need any normals or textures.

Since you say this is very inefficient, maybe either throw an exception here or add some type of warning and add your above fix when the sizes differ to avoid this hard to pinpoint error. I guess part of the problem with not allowing such a request is that you will need to specify the VertexDeclaration as the requested type, and the snippet I'm using above from Jitter is a general helper for loading a model into a collision model.

From what I can gather, running the same code in an identical XNA project will not cause any of the errors, yet produce the same result (at least when looking at the returned data). I've attached two identical XNA vs FNA example projects for you to try out. VertexBufferGetData Test FNA XNA.zip

Changing the method to ask for the full vertex data and extract the data I need after, as you suggested, fixes the problem.

var b = new VertexPositionNormalTexture[mmp.NumVertices];
mmp.VertexBuffer.GetData<VertexPositionNormalTexture>(mmp.VertexOffset * mmp.VertexBuffer.VertexDeclaration.VertexStride,
    b, 0, mmp.NumVertices, mmp.VertexBuffer.VertexDeclaration.VertexStride);
for (int i = 0; i != b.Length; ++i)
    Vector3.Transform(ref b[i].Position, ref xform, out a[i]);

Your above fix also fixes the problem, so I guess it is a good idea to keep that in there. If for nothing else, the possibility that other projects are already using the Jitter code to load collision models.

I did a quick performance check, and reading the data from my model of 320 triangles 10.000 times takes 128 milliseconds for your fixed code, and 71 milliseconds for GetData<VertexPositionNormalTexture> variant.. so while the later fix is quite a lot faster, I don't think we are breaking anything as these types of things should be done mostly during load.

flibitijibibo commented 7 years ago

Cool, glad it fixed that. Slow as it is, the XNA spec inexplicably allows it despite a size difference in the exact opposite direction throwing an Exception. Just one of those things we get to live with and hope nobody uses accidentally!