PDA

View Full Version : Compute shaders and buffer objects



Mustard
03-25-2013, 07:05 AM
First of all I don't really have any major experience with OpenGl so I might have done some real rookie mistakes.

I'm currently working on a project where I need to send 3 groups of around 100,000 vec3 down to the compute shader. To begin with I've limited myself to passing one of these groups and later adding in the other two when the first is working. However I'm having no luck thus far.

First attempt was to simply state "uniform vec3[100000] positions". While this gave no errors it did result in a linking time of 8 minutes give or take. Not a viable option and I'm not sure why I don't get any errors but that's another matter.

Second attempt was to use Shader Storage Buffer Objects. This is where I am now but I am unable to get this to work.

Setup is as follows:

void Renderer::setBuffer(char* name, std::vector<vec3f>* data){
GLuint buffHandle;
glGenBuffers(1, &buffHandle);
glBindBuffer(GL_SHADER_STORAGE_BUFFER, buffHandle);
glBindBufferRange(GL_SHADER_STORAGE_BUFFER, 0, buffHandle, 0, sizeof(vec3f)*100000);
glBufferData(GL_SHADER_STORAGE_BUFFER, data->size()*sizeof(vec3f), data, GL_STATIC_READ);
glBindBuffer(GL_SHADER_STORAGE_BUFFER, 0);
data->setHandle(buffHandle);
}

Then when running the compute shader I do the following:

void Renderer::compute(ShaderProgram* sp, GLuint buffer){
GL_CHECK_ERRORS();
auto handle = (GLShaderProgramHandle_t *) sp->getHandle();
auto programId = handle->sp_id;
glUseProgram(programId);
glBindBuffer(GL_SHADER_STORAGE_BUFFER, buffer);
glShaderStorageBlockBinding(programId, 0, 0);
glDispatchCompute(512/16, 512/16, 1);
GL_CHECK_ERRORS();
}

And the shader storage block in the shader looks as follows:

layout(std430, binding = 0) buffer list
{
vec3 pos[];
};

I am only calling set buffer once (during the init section of my program).

When I read from pos[] the only information I'm getting is pure 1's (0xFFFFFFFF......) it seems. I'm assuming this is some standard response when the buffer isn't setup correctly.

Alfonse Reinheart
03-25-2013, 08:35 AM
glBindBuffer(GL_SHADER_STORAGE_BUFFER, buffer);
glShaderStorageBlockBinding(programId, 0, 0);

There's your problem.

First, the redundant part: `glShaderStorageBlockBinding` does the same thing as setting the `layout(binding = #)` field in the shader. So it's a superfluous call.

The more egregious issue is the first line. That binding point you mention in the `layout` (and redundantly with a function call) says which `GL_SHADER_STORAGE_BUFFER` index to find the buffer in. But you didn't bind it to any index, you just bound it to the `GL_SHADER_STORAGE_BUFFER` binding. That binding means nothing.

You have to bind a range of the buffer to an index (http://www.opengl.org/wiki/Buffer_Object#Binding_indexed_targets) with `glBindBufferRange` (http://www.opengl.org/wiki/GLAPI/glBindBufferRange). Just as you would for uniform buffers.


When I read from pos[] the only information I'm getting is pure 1's (0xFFFFFFFF......) it seems. I'm assuming this is some standard response when the buffer isn't setup correctly.

The "standard response" is undefined behavior. Also, you should be able to check `pos.length` to see how much data you have available.

Mustard
03-25-2013, 09:09 AM
If I understand what you're saying glShaderStorageBlockBinding does nothing to effect the program, at least not the way things are set up right now, and I need a call to glBindBufferRange to bind the range of the buffer to the index in GL_SHADER_STORAGE_BUFFER. But isn't that what I am doing in setBuffer()?

Alfonse Reinheart
03-25-2013, 10:00 AM
But isn't that what I am doing in setBuffer()?

In the most technical sense, yes. But since your buffer doesn't have storage yet, your `glBindBufferRange` call is failing with GL_INVALID_OPERATION.

Mustard
03-25-2013, 10:14 AM
So my call to glBindBufferRange should be moved to compute() and replace the glBindBuffer call?

Also I'm checking for GL errors at the end of the setBuffer() and I'm getting nothing. Shouldn't I be getting the GL_INVALID_OPERATION there? Or glGetError doesn't pick up on this error?

Alfonse Reinheart
03-25-2013, 11:36 AM
It should register the error. Are you sure you're fetching errors correctly? I generally don't bother with manually fetching errors, relying instead on glIntercept if I think some GL code is buggy.

Mustard
03-25-2013, 12:41 PM
glGetError() returns GL_NO_ERROR (0) if no error has been recorded. And my check is:

GLenum result;
result = glGetError();
if(result) {
....
//Error print
....

return false;
}

return true;

So I assume it should be working as intended. (GL_INVALID_OPERATION is listed as one of the return values for glGetError())

aqnuep
03-25-2013, 02:14 PM
Why can't you just try putting the call to glBindBufferRange *after* you've created the storage of the buffer with glBufferData? First of all, that would be more logical, second, it would less likely to expose a potential driver bug that might actually happened in your case because you were trying to use some weird corner-case scenario.

Btw, glBindBufferRange generates a GL_INVALID_OPERATION error only if the name is not valid, the storage does not have to be ready. Though still, it's silly to bind the buffer before it has a storage.

Alfonse Reinheart
03-25-2013, 09:38 PM
Hmm. I was about to correct you with a spec quote, but I couldn't find one. I could find one for 3.3, 4.0, and 4.1, but the sentence mysteriously vanished in 4.2 and didn't return in 4.3. I have no idea if this is a spec error or the intended behavior, so I filed a bug report on it (http://www.khronos.org/bugzilla/show_bug.cgi?id=841) (fat lot of good that'll do).

Mustard
03-26-2013, 04:00 AM
First of all thanks for the responses thus far.

However I'm still getting a size of 0 on my pos array.

Am I missing any key calls that I should be making?

For the creation of the buffer I should do the following:


glGenBuffers(1, &buffHandle);
glBindBuffer(GL_SHADER_STORAGE_BUFFER, buffHandle);
glBufferData(GL_SHADER_STORAGE_BUFFER, data->getPositions()->size()*sizeof(vec3f), data->getPositions()->data(), GL_STATIC_READ);
glBindBufferRange(GL_SHADER_STORAGE_BUFFER, 0, buffHandle, 0, sizeof(vec3f)*data->getPositions()->size()); //This should bind the buffer to the shader because I specify the binding to 0 in the shader.
glBindBuffer(GL_SHADER_STORAGE_BUFFER, 0);

As for when I want to dispatch my compute shader I should simply specify the program and dispatch:

glUseProgram(programId);
glDispatchCompute(512/16, 512/16, 1);

Also feel I should clarify, I normally don't work with OpenGL on this level. I've done a bit of shading work but I normally don't work with gl-calls so I am at a loss regarding how things are supposed to be done. Reason I'm working on this is that I need support for passing my 300,000 vec3s down to the shader and that puts me beyond what I can do with uniforms (as far as I've understood at least). This means I have to get buffers to work and the framework I'm using doesn't support that. Any and all help is greatly appreciated on this matter.

Alfonse Reinheart
03-26-2013, 04:08 AM
glBindBufferRange should be used when you want to use the buffer. Much like you bind textures to render with them.

Mustard
03-26-2013, 04:19 AM
Ok. So I need to add a call:

glBindBufferRange(GL_SHADER_STORAGE_BUFFER, 0, buffer->getHandle(), 0, sizeof(vec3f)*buffer->getPositions()->size())

before the glDispatchCompute() but after the glUseProgram() I would assume. However I'm still getting nothing in my shader.

Edit: Just noticed that I had declared the buffer as GL_STATIC_READ (which I initially understood as making it a static read only for the GLSL shader. Turns out it is the other way around. I want to fill the buffer once with my vec3s from my program (read from a file) and have the compute shader read that information. Hoping I'm understanding it correctly this time.

Edit nr2: This however didn't solve my issue.

Mustard
03-27-2013, 02:53 AM
I've started simplifying my code in order to try and ensure that nothing else is getting in the way. But I still can't get it to work and I guess it is because I'm failing to understand some part of how things are supposed to be done.

But I'll try and explain what I'm doing (or think I'm doing) and hopefully someone will correct me if I'm horribly wrong.

Creation:

GLuint buffHandle;
glGenBuffers(1, &buffHandle); //(Gives me a free buffer name that is not currently in use)
glBindBuffer(GL_SHADER_STORAGE_BUFFER, buffHandle); //(Specifies that the buffer type of buffHandle)
glBufferData(GL_SHADER_STORAGE_BUFFER, sizeof(vec3f), new vec3f(1.0f), GL_STATIC_DRAW); //(Puts data into the shader storage buffer (I did change it so it will only store a single vec3 right now))
glBindBufferRange(GL_SHADER_STORAGE_BUFFER, 0, buffHandle, 0, sizeof(vec3f)); //(Binds the buffHandle to a specific part of the shader storage buffer and a binding index (same as used by the shader in the layout binding).)
glBindBuffer(GL_SHADER_STORAGE_BUFFER, 0); //(Because unbinding things seems to be the thing to do)


Usage:

glUseProgram(programId);
glBindBufferRange(GL_SHADER_STORAGE_BUFFER, 0, buffHandle, 0, sizeof(vec3f)); //(Binding the buffer to the shader storage index again as well as defining the size and of set in the shader storage array)
glDispatchCompute(512/16, 512/16, 1); //(Dispatch the compute shader to do it's calculations)


As for the shader:

#version 430
uniform image2D destTex;
layout(std430, binding = 0) buffer List
{
vec3[] pos;
};

layout (local_size_x = 16, local_size_y = 16) in;

void main() {
float testing = 0.0;
if(pos.length() != 0){
testing = 1.0;
}
imageStore(destTex, storePos, vec4(0.0, testing, 0.0, 0.0));
}


Something is still wrong and I have no idea how to debug it because I'm getting no errors and I'm not sure how to debug OpenGL. Any advice would be greatly appreciated either helping me understand what I (actually) am doing as opposed to what I should be doing or some advice on how to debug this.

Edit: Removed commented stuff from the code to avoid confusion

Edit 2: Added back "layout (local_size_x = 16, local_size_y = 16) in;" because i removed it by mistake.

Edit 3: Followed Dark Photons advice from below this post and added formating.

Dark Photon
03-27-2013, 05:39 AM
As an aside, consider using
... or ... around your code blocks. Makes a big difference in readability. For instance:

Creation:

GLuint buffHandle;
glGenBuffers(1, &buffHandle); // (Gives me a free buffer name that is not currently in use)
glBindBuffer(GL_SHADER_STORAGE_BUFFER, buffHandle); // (Specifies that the buffer type of buffHandle)
glBufferData(GL_SHADER_STORAGE_BUFFER, sizeof(vec3f), new vec3f(1.0f), GL_STATIC_DRAW); // (Puts data into the shader storage buffer // (I did change it so it will only store a single vec3 right now))
glBindBufferRange(GL_SHADER_STORAGE_BUFFER, 0, buffHandle, 0, sizeof(vec3f)); // (Binds the buffHandle to a specific part of the shader storage buffer and a binding index (same as used by the shader in the layout binding).)
glBindBuffer(GL_SHADER_STORAGE_BUFFER, 0); // (Because unbinding things seems to be the thing to do)


Usage:

glUseProgram(programId);
glBindBufferRange(GL_SHADER_STORAGE_BUFFER, 0, buffHandle, 0, sizeof(vec3f)); // (Binding the buffer to the shader storage index again as well as defining the size and of set in the shader storage array)
glDispatchCompute(512/16, 512/16, 1); // (Dispatch the compute shader to do it's calculations)


As for the shader:

#version 430
uniform image2D destTex;
layout(std430, binding = 0) buffer List
{
vec3[] pos;
};

layout (local_size_x = 16, local_size_y = 16) in;

void main() {
float testing = 0.0;
if(pos.length() != 0){
testing = 1.0;
}
imageStore(destTex, storePos, vec4(0.0, testing, 0.0, 0.0));
}

Mustard
03-27-2013, 07:40 AM
Progress... of a sort...

I edited my shader so pos was actually being read somewhere and suddenly the length is no longer 0. However... weird things are happening...

So my new shader looks as follows:

#version 430

uniform float roll;
uniform image2D destTex;

layout(std430, binding = 0) buffer List
{
vec3 pos[];
};

layout (local_size_x = 16, local_size_y = 16) in;

void main() {
float bla = pos[int(round(roll))].x;
float testing = 0.0;
//if(pos.length() == 1203765248){
//if(pos.length() == 1065353216){
if(pos.length == 1){
testing = 1.0;
}
ivec2 storePos = ivec2(gl_GlobalInvocationID.xy);
float localCoef = length(vec2(ivec2(gl_LocalInvocationID.xy)-8)/8.0);
float globalCoef = sin(float(gl_WorkGroupID.x+gl_WorkGroupID.y)*0.1 + roll)*0.5;
imageStore(destTex, storePos, vec4(globalCoef*localCoef, testing, bla, 0.0));
}


Heres the thing tho. Regardless of what I'm calling glBindBufferRange with pos.length always returns 1.

However pos.length() returns 1065353216 if I use my test data of a single vec3f (as data and sizeof(vec3f) as the size) for the glBufferData and glBindBufferRange calls.

And if pos.length() returns 1203765248 if I use my real data which is 98304 vec3f for the glBufferData and glBindBufferRange calls.

Now for the interesting part I guess... It turns out that 1065353216 in hex is 0x3F800000 and if you assume that is a 32-bit floating point number you get 1. And 1203765248 in hex is 0X47C00000 and if you take that as a 32-bit float it is 98304. So something works... sort of...

So time for my questions:

1) Why is pos.length() returning the integer interpretation of the length which is apparently stored as a float?

2) Is there an easy way to get from the answer I am getting from pos.length() to a normal integer or float?

3) What is the difference between pos.length() and pos.length ?

4) Should I assume OpenGL does optimization and was removing pos from my shader because I wasn't reading from the information contained in it despite reading the length?

Also in other news. My vec3's are getting down to the shader now. So thanks for all the help. =D

Piers Daniell
03-28-2013, 03:14 PM
1) This is a bug we'll fix asap.
2) You could use uint(uintBitsToFloat(pos.length())) as a work around.
3) It should be pos.length(). pos.length probably produces a compile error.
4) I'm not sure it's defined what .length() does on a variable that is unused and eliminated. Returning 0 sounds plausible.