ptitSeb / gl4es

GL4ES is a OpenGL 2.1/1.5 to GL ES 2.0/1.1 translation library, with support for Pandora, ODroid, OrangePI, CHIP, Raspberry PI, Android, Emscripten and AmigaOS4.
http://ptitseb.github.io/gl4es/
MIT License
702 stars 159 forks source link

Some test-case lockup #48

Closed kas1e closed 6 years ago

kas1e commented 6 years ago

There is test case i do check for now on amigaos4: http://kas1e.mikendezign.com/aos4/gl4es/tests/drawbuffer_gl4es.c

Once i run it, after few loops of DrawBox() , it crashes.

It works through when pure opengl used (some old 1.1 version which we have), but can be that bug in the code as well. There is only defined 6 normals for 8 vertices, but probably that should't cause crash ?

@ptitSeb Can you check plz that GL code on your own (with SDL or something) , to see if that code works at all for on gl4es, so we can then know if it amigaos4 only issue or not.

Thanks!

ptitSeb commented 6 years ago

So maybe it's the ternary operator that ogles2 or warp3d doesn't like. Try replacing it by simple if/then/else like: line 354 sprintf(buff, "if(nVP>0.) dd=(nVP * %s%d.diffuse.xyz); else dd=vec3(0.);\n", fm_diffuse, i); line 391 sprintf(buff, "if(nVP>0. && lVP>0.) ss=(%s%d.specular.xyz); else ss=vec3(0.);\n", fm_specular, i);

ptitSeb commented 6 years ago

same idea ;) (I'm off to bed now)

kas1e commented 6 years ago

Damn, same crash :(

Will check more deeply tomorrow.

Thanks for help !

ptitSeb commented 6 years ago

You can also remove the else part: line 354 `sprintf(buff, "dd=vec3(0.0);\nif(nVP>0.) dd=(nVP * %s%d.diffuse.xyz);\n", fm_diffuse, i); line 391 sprintf(buff, "ss=vec3(0.0);\nif(nVP>0. && lVP>0.) ss=(%s%d.specular.xyz);\n", fm_specular, i);

kas1e commented 6 years ago

still same :(

Probably need to throw that shader on the author of shaders.. Damn, strange enough !

ptitSeb commented 6 years ago

A last test you can do is remove completly the if (of course rendering will not be correct).

line 354 sprintf(buff, "dd=vec3(0.0);\ndd=(nVP * %s%d.diffuse.xyz);\n", fm_diffuse, i); line 391 sprintf(buff, "ss=vec3(0.0);\nss=(%s%d.specular.xyz);\n", fm_specular, i);

kas1e commented 6 years ago

With those lines it works ! Renders better, but as you say uncorrectly. I can see rotated cube , can see some green and red normals, but others normal are black. But that expected probably.

I also have an answer yesterday from the developer who works on shaders, that what he say:


Before I start looking for a shader compiler bug, I'd like to know that it actually gets that far. As in, is the shader being compiled successfully all the way through to Warp3D ? Or, is something failing beforehand? For example, it's a larger shader and the lines in question are rather long. Is the buffer that the shader is sprintf()'ing to big enough?

The shader compiler's if/else functionality is fairly well tested, and those if/else sections are pretty basic. Plus, the vertex shader compiles just fine when I do it manually.

Probably he just didn't check our debug logs where all looks fine enough..

kas1e commented 6 years ago

Interesting is that for our last checks without IF , we have generate those lines:

dd = vec3(0.0); dd =(nVP gl_Color.xyz _gl4es_LightSource_0.diffuse.xyz);

and

ss = vec3(0.0); ss = (_gl4es_FrontLightProduct_0.specular.xyz);

Is they correct ? I am mostly about ss = (_gl4es_FrontLightProduct_0.specular.xyz); , as in sprinfs i see %s%d,

ptitSeb commented 6 years ago

the sprintf is sprintf(buff, "ss=vec3(0.0);\nss=(%s%d.specular.xyz);\n", fm_specular, i);, it's coherent with the output you see (fm_specular is "_gl4esFrontLightProduct" in that case, and i is light 0).

About the comment from Warp3D developper: buff is a 1024 characters buffer, so no buffer overflow here. This code has been tested a lot, also using valgrind on linux, I'm pretty sure there is no buffer overflow.

Also, the issue is not at compile time it seems, but really at run time, with some corruption happening at run time. Maybe it's the combination of vec3 and if?

Can you try something slightly different: line 354 sprintf(buff, "dd=(nVP * %s%d.diffuse.xyz);\n", fm_diffuse, i)*((nVP>0.)?1.:0.); line 391 sprintf(buff, "ss=(%s%d.specular.xyz);\n", fm_specular, i)*((nVP>0. && lVP>0.)?1.:0.);

kas1e commented 6 years ago

Fail to compile with:

src/gl/fpe_shader.c:387: error: 'nVP' undeclared (first use in this function) src/gl/fpe_shader.c:387: error: (Each undeclared identifier is reported only once src/gl/fpe_shader.c:387: error: for each function it appears in.) src/gl/fpe_shader.c:413: error: 'lVP' undeclared (first use in this function)

kas1e commented 6 years ago

(don't see that there 387 and 413 lines, just i keep previous ones commented out, so to be able fast check them back)

ptitSeb commented 6 years ago

Ah of course, I messed up things.

line 354 sprintf(buff, "dd=(nVP * %s%d.diffuse.xyz)*((nVP>0.)?1.:0.);\n", fm_diffuse, i); line 391 sprintf(buff, "ss=(%s%d.specular.xyz)*((nVP>0. && lVP>0.)?1.:0.);\n", fm_specular, i);

Should be better that way

kas1e commented 6 years ago

With that one shader compilation fail:

glCompileShader(256) glGetShaderiv(256, 0x8B81, 0x67d71564) glGetShaderInfoLog(256, 1000, 0x0, 0x67d71568) LIBGL: FPE Vertex shader compile failed: ERROR: Code generation failed. Error log: OpSelect instruction isn't implemented yet. INTERNAL ERROR: (Vector/Matrix)TimesScalar instruction's source parameters are missing OpSelect instruction isn't implemented yet. INTERNAL ERROR: (Vector/Matrix)TimesScalar instruction's source parameters are missing INTERNAL ERROR: I/FAdd instruction's source parameters are missing INTERNAL ERROR: I/FAdd instruction's source parameters are missing INTERNAL ERROR: (Vector/Matrix)TimesScalar instruction's source parameters are missing INTERNAL ERROR: I/FAdd instruction's source parameters are missing INTERNAL ERROR: Dot instruction's source parameters are missing INTERNAL ERROR: Store instruction's source parameter is incomplete: OpStore: : tmp122 >> Color

ptitSeb commented 6 years ago

Mmmm, try this then.

line 354 sprintf(buff, "spot=(nVP>0.)?1.0:0.0;\ndd=(nVP * %s%d.diffuse.xyz)*spot;\n", fm_diffuse, i); line 391 sprintf(buff, "spot=(nVP>0. && lVP>0.)?1.0:0.0;\nss=(%s%d.specular.xyz)*spot;\n", fm_specular, i);

kas1e commented 6 years ago

That one seems better, but still fail:

LIBGL: FPE Vertex shader compile failed: ERROR: Code generation failed. Error log: OpSelect instruction isn't implemented yet. INTERNAL ERROR: Store instruction's source parameter is incomplete: OpStore: : tmp76 >> spot OpSelect instruction isn't implemented yet. INTERNAL ERROR: Store instruction's source parameter is incomplete: OpStore: : tmp103 >> spot

ptitSeb commented 6 years ago

mmm, I'm not sure I understand the error. The code seems fine to me (if you try to check the shaders, you have error?) Anyway, let's try with simple if and no ternary operator: line 354 sprintf(buff, "spot=0.0;\nif(nVP>0.) spot=1.0;\ndd=(nVP * %s%d.diffuse.xyz)*spot;\n", fm_diffuse, i); line 391 sprintf(buff, "spot=0.0;\nif(nVP>0. && lVP>0.) spot=1.0;\nss=(%s%d.specular.xyz)*spot;\n", fm_specular, i);

kas1e commented 6 years ago

With last ones shaders compiles fine, but then crashes :)

Seems once "if" in the palce, then we crashes..

kas1e commented 6 years ago

Once i remove from last example that "if" string, i.e. keep just only:

spot=0.0; dd=(nVP _gl4es_Color.xyz _gl4es_LightSource_0.diffuse.xyz)*spot;

and

spot=0.0; ss=(_gl4es_FrontLightProduct_0.specular.xyz)*spot;

Then no crash ! Cube rotates ! Wtf ..

ptitSeb commented 6 years ago

Here, a version with no if or ternary at all! :)

line 354 sprintf(buff, "spot=max(1.0, sign(nVP)+1);\ndd=(nVP * %s%d.diffuse.xyz)*spot;\n", fm_diffuse, i); line 391 sprintf(buff, "ss=(%s%d.specular.xyz)**spot*max(1.0,sign(lVP)+1.0);\n", fm_specular, i);

And this shader should be exact (just slower then original... but not that much).

ptitSeb commented 6 years ago

Or this one should works too:

line 354 sprintf(buff, "spot=step(0.0, nVP);\ndd=(nVP * %s%d.diffuse.xyz)*spot;\n", fm_diffuse, i); line 391 sprintf(buff, "ss=(%s%d.specular.xyz)*spot*step(0.0,lVP);\n", fm_specular, i);

kas1e commented 6 years ago

In previous version shaders fail to compile with words : about wrong operand ypes: no operation "+" exists that takes a left hand operatn on tupe glogal highp float" , etc

But last version works fine :)

kas1e commented 6 years ago

So .. what did it mean at all , that once we have "if" it all crashes in runtime (shader's bug). As well as if we have "?" betwen pairs, crashes too ? (but probably that the same "if" in end, so same code)

kas1e commented 6 years ago

And i notice that something wrong with colors happens as well with last string (on of normales have color of backgorund, while should be purple).

But that not so matter anyway, for test it was enough. If that it shader's issue it should be fixed in our shader compiller.

ptitSeb commented 6 years ago

Yeah, the ternary operator and if probably share the same code in warp3d shaders... Now that you have a version of the shader that work (with step), and a version that doesn't (with if or with ?), you can open a bug in Warp3D side...

Now, I'll see if I can add an optional code path in fpr_shader to limit the ternary operator in the vertex shader, at least for lights. Maybe it can be faster on the Pandora, I have to test.

kas1e commented 6 years ago

Yeah, but only make it optional / possible to remove , as those problems should be fixed in drivers of coruse, or it will lead to massive problems when i will try to port something a little more heavy :)

ptitSeb commented 6 years ago

Yeah. I'll keep the code behind a #define It may be better to use it on certain platform in fact, this has to be checked. I need to test it a bit before pushing...

ptitSeb commented 6 years ago

@kas1e with commit 077d870a7c1ddcc57190f1a4c7a8586ad49e32a2 vertex shaders generated by fpe_shader.c will have no ternary operator.

kas1e commented 6 years ago

And that commit works, yep. Let it be here then until fix for shader's compiler is come.

But now, i found another issues, that can be easy related to Endianes, or, that can be related to the replacement of ternary operator.

There is 2 screenshots, one i catch with our 1.1 version of opengl (called MiniGL). Another one, its from GL4ES i.e. what we test.

MiniGL one: http://kas1e.mikendezign.com/aos4/gl4es/tests/mgl_rotate.jpg GL4ES one: http://kas1e.mikendezign.com/aos4/gl4es/tests/gl4es_rotate.jpg

I catch screenshots on "the same" place, so, we can see that colors of one of the sides of the cube are different and not as they expected to be. In minigl you can see its purple , in the gl4es vesion that side are just of the same as background color.

Can you plz test , if you have on Pandora that "purple" normale with ternary and without ternary operators, or you also have one of the sides of cube with the same as background color.

Thanks !

ptitSeb commented 6 years ago

I have purple with the shader with ternary operator.

I'll rebuild without and retest soon.

ptitSeb commented 6 years ago

I also have purple with the alternate shader. So it's something else.

kas1e commented 6 years ago

Hmm.. I think about Endianes, but usually with endianes all colors wrong, not only one side.. Another idea , is can it be that uniform structures, somehow "swapped" somewhere, so it can cause such effect ?

ptitSeb commented 6 years ago

Maybe yes. But colors will be an attribute, not a uniform here. can you create a full log?

Mmm, colors are floats, so I doubt endianess has anything to do with the issue here. Still, there maybe something wrong with thoses attribute values...

kas1e commented 6 years ago

Here is: http://kas1e.mikendezign.com/aos4/gl4es/tests/wrong_color.txt

Through its 5mb of size, its very fast fills by data , and i do not edit it just in case something intersting will be at end on somewhere.

ptitSeb commented 6 years ago

The log looks fine to me. Maybe one of the driver (OGLES or Warp3D) doesn't like the fact that the Color attribute (and the Normal too, but it's not really visible) is a "constant" attribute. It would be interresting to make it a non-constant one. In your sample, change:

glColor3fv(&colors[i][0]);

by

    GLfloat col[8][3];
    for (int j=0; j<8; j++) memcpy(col[j], colors[i], 3*4);
    glEnableClientState(GL_COLOR_ARRAY);
    glColorPointer(3, GL_FLOAT, 0, col);

(yeah ugly code, it's just for testing :p )

kas1e commented 6 years ago

And yes, everything good with that changed code.. DAMN ! Sorry that i throw it all at you, like you need to fix all our bugs :) Can we reduce the test somehow so we can be sure what wrong ?

kas1e commented 6 years ago

Btw, also it fix "geometry" : if you check previous 2 screenshots, you can see that right/down area in minigl version are "blue" , while, when we run it with glColor3fv(&colors[i][0]); on gl4es, then there is no that blue area. What mean its not only color, but normales too ?

ptitSeb commented 6 years ago

The Normal are set the same way the colors were. So I guess there are no setup properly. But with this zoomed cube, I'm not sure you see the effect of a bad normal correctly (the reflection should be slightly different then with minigl, the moment where the face is "white" because it completly reflect the light).

ptitSeb commented 6 years ago

So I think you can open a new ticket for the Attribute issue: Vertex Attribute set with glVertexAttrib4fv() does not seems to be correctly updated.

ptitSeb commented 6 years ago

I prefer to not try to compensate for this bug for now. It is possible, but that mean creating new array, so it's really a waste and prefer to avoid that (unless there is no chance to get a fix).

kas1e commented 6 years ago

Sure, fix in driver are must. Will write to author now.

Thanks!

kas1e commented 6 years ago

@ptitSeb By the way, what you use on Pandora when build pure GLES2 examples ? SDL or EGL ?

ptitSeb commented 6 years ago

I use SDL, it's much easier to setup then EGL directly.

kas1e commented 6 years ago

That cool ! I do not know if it will be too much to ask , but as our shader's compiler author "want test case", is it possible for you to create a GLES2 example from that test-case debug output ? I mean i can see in debug log what functions calls, etc, but i do not know how to make it all works on GLES2 code. All i know that i need to take those 2 shaders, load them, and then mimic logic of what i do in test case . But while on GL it looks easy enough, on GLES2 it can be some mess ?:)

ptitSeb commented 6 years ago

I'll see what I can do.

ptitSeb commented 6 years ago

But I'm not sure why he cannot used the current version that use gl4es. He wants to change the shaders?

kas1e commented 6 years ago

He want to shorten all code and layers in between to minimum, so can find where problem come. For first probably it will be pure gles2, then probable he will eleminate gles2 and made direct warp3d sample.

ptitSeb commented 6 years ago

Well, I'll see what I can do, it's a bit of work an hour or two to get something working I guess.

kas1e commented 6 years ago

@ptitSeb Can you also give me your paypal address, want to motivate you a bit by some donation :) If you doesnt mind of course

ptitSeb commented 6 years ago

I assume SDL2 is ok, right?

I do have a paypal yes: https://www.paypal.com/donate/?token=RUuvxCN8rmpvaup-t30Qd-uuGmLvTDcEc2jR52jgOnE5Qctz5qC8CcZs0n9NeLYGozQPYG

kas1e commented 6 years ago

Yep, sdl2 good