floooh / sokol

minimal cross-platform standalone C headers
https://floooh.github.io/sokol-html5
zlib License
7.11k stars 501 forks source link

Empty storage buffers in fragment stage (opengl 4.3) #1119

Closed Marsimplodation closed 1 month ago

Marsimplodation commented 1 month ago

Platform: Arch linux using opengl 4.3

Storage buffers created only for the fragment shader will contain invalid data. If for the vertex shader a different buffer with the same binding number exists, the data will be equivalent to that. If no Storage Buffer exists in the vertex stage, with that binding number, the data will be filled with 0.0

Steps to replicate: Take the base example of no storage buffers and add an array with data just to the fragment stage. display if any data is not 0


struct data_t {
   int val;
};

layout(std430, binding=0) readonly buffer ssbo1 {
    data_t buffer[]; 
};

//...
int has_Data = 0;
for(int i = 0; i < length_of_data; ++i) if(buffer[i].val != 0) has_Data = 1;

Now pass the same data to the vertex shader in the same binding to the vertex stage and this will now be true.

Also: Sokol-shdc does not recognize storage buffers for only the frag shader in opengl, only if other targets are specified. Thus a manual binding was done here

floooh commented 1 month ago

Hmm, the closest sample is probably the sbuftex-sapp sample which uses a storage buffer on the fragment stage (but also a storage buffer for vertex pulling):

sokol-shdc glsl: https://github.com/floooh/sokol-samples/blob/master/sapp/sbuftex-sapp.glsl

C code: https://github.com/floooh/sokol-samples/blob/master/sapp/sbuftex-sapp.c

I'll see if I can reproduce the issue when replacing the vertex stage storage buffer with a fixed function vertex input in that sample...

One thing that's important though (at least until the 'bindings cleanup' which is currently wip in a branch):

If you define the binding manually, the fragment shader storage buffer bindings must start at binding 8 (e.g. see: https://github.com/floooh/sokol/blob/38e4c9a516f8808d706343a5c525acfe7007fe67/sokol_gfx.h#L1068-L1071):

layout(std430, binding=8) readonly buffer ssbo1 {
    data_t buffer[]; 
};

PS: also you can't use the same bindings for the vertex- and fragment-shader stage, for instance if you have one storage buffer on the vertex stage, and one on the fragment stage, you need to use binding=0 for the vertex shader storage buffer and binding=8 on the fragment shader storage buffer.

Those details will change with the 'bindings cleanup' stuff I'm currently working on, so don't get too used to it ;)

Marsimplodation commented 1 month ago

starting at 8 in the fragment shader, indeed fixes the issue. In the c code I have now bound different data to the same binding number, while int the shader +8 for frag. This gives each shader the correct data

floooh commented 1 month ago

Can you give more information about the sokol-shdc problem you're seeing? (e.g. not recognizing a storage buffer in the fragment stage?). I did a quick adhoc test with a modified cube-sapp.glsl shader (see below), and at least the sokol-shdc output looks correct.

@ctype mat4 hmm_mat4

@vs vs
uniform vs_params {
    mat4 mvp;
};

in vec4 position;

void main() {
    gl_Position = mvp * position;
}
@end

@fs fs
out vec4 frag_color;

struct sb_color {
    vec4 color;
};

readonly buffer colors {
    sb_color clr[];
};

void main() {
    frag_color = clr[0].color;
}
@end

@program cube vs fs

...but I didn't test if it also works at runtime.

Marsimplodation commented 1 month ago

this example correctly creates the SLOT_colors, when I tried it, it did not, I don't have the shader code anymore tho

For the shader code I had when I tried it, the generated the slot information with all backend but not with just opengl. Maybe an issue with my shader code

Marsimplodation commented 1 month ago

as this seems to be just me using the wrong numbers and you reworking the binding anyway, I will close this issue. The empty data for no equivalent binding will be due to the wrong number as well

floooh commented 1 month ago

The empty data for no equivalent binding will be due to the wrong number as well

Yes, what basically happened is that no buffer was bound to binding 0 and the fragment shader is trying to read data from a non-existing buffer binding. That's either 'undefined behaviour' and/or will return zero values.