flibitijibibo / FNA-MGHistory

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

[MojoShader] Creating effects causes 'System.AccessViolationException' #302

Closed QuarkRobot closed 9 years ago

QuarkRobot commented 9 years ago

A first chance exception of type 'System.AccessViolationException' occurred in FNA.dll An unhandled exception of type 'System.AccessViolationException' occurred in FNA.dll Additional information: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.

This happens in OpenGLDevice.cs on line 1465 when I try to load a pixel shader compiled with FXC.exe. The shader worked with XNA4.

Example usage:

Dim data() As Byte = IO.File.ReadAllBytes(PATH) effect = New Effect(GraphicsDevice, data)

Apart from this, everything else is working with FNA. Not sure if I'm doing something wrong here. The MojoShader DLL is with the EXE.

flibitijibibo commented 9 years ago

Oh, jeez... I think I have an idea.

I'm throwing something else together right now, but if you want, try this out.

https://github.com/flibitijibibo/FNA/blob/master/src/Graphics/OpenGLDevice.cs#L1455

Change effectCode to instead use an IntPtr obtained via a GCHandle pinning the memory. You'll have to change this call to accept IntPtrs:

https://github.com/flibitijibibo/MojoShader-CS/blob/master/MojoShader.cs#L889

This is a really silly thing about C# that I'll probably have to be more careful about with other C# interop stuff...

flibitijibibo commented 9 years ago

Here's a diff that should pin the byte[] for long enough, can you try this out?

diff --git a/src/Graphics/Effect/Effect.cs b/src/Graphics/Effect/Effect.cs
index 14fe7a7..657077f 100644
--- a/src/Graphics/Effect/Effect.cs
+++ b/src/Graphics/Effect/Effect.cs
@@ -292,7 +292,9 @@ namespace Microsoft.Xna.Framework.Graphics
            GraphicsDevice = graphicsDevice;

            // Send the blob to the GLDevice to be parsed/compiled
+           GCHandle pin = GCHandle.Alloc(effectCode, GCHandleType.Pinned);
            glEffect = graphicsDevice.GLDevice.CreateEffect(effectCode);
+           pin.Free();

            // This is where it gets ugly...
            INTERNAL_parseEffectStruct();
QuarkRobot commented 9 years ago

Thanks flibitijibibo. No luck with the mod below - same result.

// Send the blob to the GLDevice to be parsed/compiled GCHandle pin = GCHandle.Alloc(effectCode, GCHandleType.Pinned); glEffect = graphicsDevice.GLDevice.CreateEffect(effectCode); pin.Free();

Being a VB guy, my C# is a little too rusty to make the changes in your first reply. Thanks for taking a look though.

Silly question; if know body else has raised this issue, could my shader be odd despite working with XNA4?

flibitijibibo commented 9 years ago

You can send me the shader binary and I can run it through MEPTool to see if it works. It should work with anything up to SM3, but maybe there's an effect param type/class I've missed.

QuarkRobot commented 9 years ago

Here is the shader: http://medai.quarkrobot.com/downloads/dark.fx It's extremely basic.

flibitijibibo commented 9 years ago

Wait, so is this precompiled, or are you sending it the source directly? At the moment, compiling at runtime from source isn't supported, so if that's the case, precompile and everything should be okay after that.

QuarkRobot commented 9 years ago

No, I have it precompiled with fxc.exe. This is the compiled version I'm trying load: http://media.quarkrobot.com/downloads/DARKMARK.Fxc which is compiled from http://media.quarkrobot.com/downloads/dark.fx

flibitijibibo commented 9 years ago

Hm, fair enough.

Here's a patch that does the IntPtr work I mentioned before:

http://www.flibitijibibo.com/effectPtr.tar.bz2

Extract in the root FNA folder and it should overwrite a bunch of files. I thought that pinning it alone would work, but maybe it needs the actual IntPtr for this to work?

QuarkRobot commented 9 years ago

A first chance exception of type 'System.NullReferenceException' occurred in FNA.dll Additional information: Object reference not set to an instance of an object.

Line 296 in Effect.cs - looks like effectCode is null. This is quite odd isn't since the effect files are extremely basic.

flibitijibibo commented 9 years ago

It's hard to say, since I'm not entirely sure how things are being loaded (to be honest I didn't think FNA in its current state was compatible with other .NET languages). This explains why there may have been a segfault, if it turns out we were just throwing around null references. Why it's null? No idea, honestly...

Interestingly, according to the MSDN there is no NullReferenceException, so they may very well assume that both graphicsDevice and the byte[] have content:

https://msdn.microsoft.com/en-us/library/ff433788.aspx

Without being in front of the debugger I couldn't really say which.

QuarkRobot commented 9 years ago

I just wiped up a C# test - a blank XNA4 game and swapped it to FNA and put the following in Game1.cs->LoadContent()

Effect fx; fx = new Effect(GraphicsDevice, System.IO.File.ReadAllBytes("DARKMASK.Fxc"));

Same access violation error so I don't think VB would be causing the problem. To be honest, I have zero idea would could be wrong.

flibitijibibo commented 9 years ago

If you can send me a test case I can look at this locally (though it'd be on Linux... shouldn't be too much work after that though).

QuarkRobot commented 9 years ago

http://media.quarkrobot.com/downloads/mojo.zip is the test C# project with the shader. Thanks for taking a look at this flibitijibibo.

flibitijibibo commented 9 years ago

Had a look at the effect binary... the effect is there, but for some reason it's 16 bytes into the blob. How was this effect compiled?

QuarkRobot commented 9 years ago

I just used fxc.exe on a Windows 8.1 x64 machine.

flibitijibibo commented 9 years ago

Do you have the full command line to build? The first u32 is 0xBCF00BCF, while it should be 0xFEFF0901:

https://github.com/flibitijibibo/MojoShader/blob/master/mojoshader_effects.c#L805

The first u32 after the header is the offset to the real effect though. This is similar to how XNA packs the effects, with some preceding info...

https://github.com/flibitijibibo/FNA/blob/master/src/Content/ContentReaders/EffectReader.cs#L72

... but an fxc.exe effect? That'd be new to me...

Compiler version I'm testing with is 9.29.952.3111.

flibitijibibo commented 9 years ago

For what it's worth, it turns out this is the exact constant found within the XNB binaries as well, so I've marked it as such in FNA for now:

https://github.com/flibitijibibo/FNA/commit/4e18b71d6de424bf1228d802506e5b6ffa9cdadc

If I knew what the context was for this extra data, which currently appears to be mostly pointless, it may be the case that my effect parsing in MojoShader should be handling this rather than the content reader. Hard to say without knowing why it's being built that way though.

QuarkRobot commented 9 years ago

Okay, if I do this:

fxc /T fx_2_0 dark.fx /Fo dark.fxc

I now get:

An unhandled exception of type 'System.Collections.Generic.KeyNotFoundException' occurred in mscorlib.dll Additional information: The given key was not present in the dictionary.

on line 1378 of OpenGLDevice.cs.

If I use:

fxc /T ps_2_0 /E PixelShaderFunction dark.fx /Fo dark.fxc

I get the same access violation error. Very odd.

Fxc version is 9.30.9200.16384

QuarkRobot commented 9 years ago

My bad too: those original broken shaders were not compiled used fxc. I used XNA content pipeline to do it: http://stackoverflow.com/a/14673674/1061048

flibitijibibo commented 9 years ago

Since this comes from one of the official compilers I've put the extra header reading into MojoShader's parser:

https://github.com/flibitijibibo/MojoShader/commit/ca260e1ebbdea236e8479a5af8dd8c5d69407b67

This affects how effects are read...

https://github.com/flibitijibibo/EffectRE/commit/935d6c3fea32345f9d615feece8dbab89c8a4f5d https://github.com/flibitijibibo/FNA/commit/66d65613a72d7458ba1c3ce1dc8cb5743c9cde4b

... so the current revision will require new MojoShader binaries (which I'm building now).

flibitijibibo commented 9 years ago

fnalibs has been updated. Try the latest FNA revision with the latest MojoShader.dll.

flibitijibibo commented 9 years ago

Also, just took care of the KeyNotFoundException:

https://github.com/flibitijibibo/FNA/commit/8e7711a1099bcbb95922587151ba6931b9c1f057

Turns out we were missing PointMipLinear this whole time. Guess nobody uses that often.

QuarkRobot commented 9 years ago

Thanks a bunch flibitijibibo! These mods work like charm. I'm not sure why XNA compiles shaders slightly differently to FXC.

Thanks again for fixing this so quick. Cheers.