Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mojoshader missing variable, and assertion in preShader #24

Closed
jeffphilp opened this issue May 6, 2016 · 6 comments
Closed

Mojoshader missing variable, and assertion in preShader #24

jeffphilp opened this issue May 6, 2016 · 6 comments

Comments

@jeffphilp
Copy link

Hi,

When initially loading the following file:
shader.txt

it fails to compile the generated glsl because of a missing variable ps_v1, I believe this is caused by the NORMAL0 parameter passed into pixel shader which I think can be fixed by adding the following code:

 else if (usage == MOJOSHADER_USAGE_NORMAL)
 {
     push_output(ctx, &ctx->globals);
     output_line(ctx, "vec4 %s;", var);
     pop_output(ctx);
 } // else if        

at https://github.com/flibitijibibo/MojoShader/blob/master/mojoshader.c#L2731

This change allowed the shader to compile, but when I actually make use of the effect I run into another issue which relates to an assertion failure in the MOJOSHADER_runPreshader
https://github.com/flibitijibibo/MojoShader/blob/master/mojoshader_effects.c#L193
where the final operand is actually of type MOJOSHADER_PRESHADEROPERAND_INPUT.
The last commit to that line is putting in the assertion for OUTPUT only though the previous changeset had a commented out assert which tolerated INPUT or OUTPUT. Should we be allowing INPUT types as the destination?

@flibitijibibo
Copy link
Member

flibitijibibo commented May 6, 2016

I think I fixed the first one (it was the same realm as the COLOR2 issue), but the preshader may need some different attention.

As far as I know, the VM does not write to input registers, and the preshader instructions indicate this:

                * 0: "AmbientLight"
                  register set float4
                  register index 0
                  register count 1
                  symbol class vector
                  symbol type float
                  rows 1
                  columns 3
                  elements 1
                * 1: "DiffuseTextureEnabled"
                  register set float4
                  register index 1
                  register count 1
                  symbol class scalar
                  symbol type float
                  rows 1
                  columns 1
                  elements 1

            neg r0.x, c1.x
            cmp c0.x, r0.x, (0), (1)
            neg r0.xyz, c0.xyz
            add c1.xyz, (1, 1, 1), r0.xyz

r0 is a temp register and c0/c1 are output registers (for the dest paths, c0/c1 are otherwise constants that you sent in). See if you can figure out why it wants to write to input... (or optimize this out of the preshader, if you want).

@jeffphilp
Copy link
Author

Forgive my ignorance, I am fairly new to the graphics area, but by VM do you mean a layer provided by mojoshader or by the effects framework? Also how were you able to generate those instructions?

@flibitijibibo
Copy link
Member

The preshader VM is this guy:

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

Basically the effects compiler will generate preshaders when you've got stuff that you should be doing on the CPU, but for some reason gave to the GPU instead. The compiler hoists it out and attempts to calculate it once when you hit CommitChanges (or Apply in XNA4), but the VM can be very slow, and in our case, very unstable.

I parsed the preshader out with this:

https://hg.icculus.org/icculus/mojoshader/file/6e760a19f456/utils/testparse.c

@flibitijibibo
Copy link
Member

I just stepped through the preshader parse, and guess what? You found a new operand token!

https://github.com/flibitijibibo/MojoShader/blob/master/mojoshader.c#L8570

For the failed assertion we get token '5' for the cmp instruction's dest token. I have no idea why it's not 4 in this case! I'll take a closer look myself, but if you're trying to find a starting point, that'll be it.

@flibitijibibo
Copy link
Member

flibitijibibo commented May 6, 2016

Dug into the source shader and figured it out; your preshader is writing to the bool registers, something that's never happened before.

How it's working is that it's doing this to calculate the "real" bool value...

neg r0.x, c1.x
cmp c0.x, r0.x, (0), (1)

... basically turning your DiffuseTextureEnabled > 0 into, and I swear this is true, !(-DiffuseTextureEnabled >= 0), and then doing your branch as-is.

Getting the VM to map this operand type to bool registers is possible, but will be EXTREMELY painful to do. My recommendation is to just turn that DiffuseTextureEnabled into a bool like you're writing it as anyway, as it skips two instructions in the VM and works exactly the same way.

@jeffphilp
Copy link
Author

That is crazy, and when I wrote that shader for some reason I didn't think I could use booleans. However I think your advice is sound and I have changed the shader and it works perfectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants