glVertexAttribPointer doesn't work with offset

redeemarr commented 1 year ago

Spec says:

pointer Specifies a offset of the first component of the first generic vertex attribute in the array in the data store of the buffer currently bound to the GL_ARRAY_BUFFER target.

Example code to reproduce:

struct vert_t
{
    float pos[3];
    float col[3];
};

vert_t verts[3];

GLuint vb, va;
glGenVertexArrays(1, &va);
glGenBuffers(1, &vb);

glBindVertexArray(va);
glBindBuffer(GL_ARRAY_BUFFER, vb);
glBufferData(GL_ARRAY_BUFFER, 3 * sizeof(vert_t), verts, GL_STATIC_DRAW);

size_t offset = 0;
glEnableVertexAttribArray(0);
glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, sizeof(vert_t), (void*)offset); // offset=0 for 'pos'
offset += sizeof(float) * 3;

glEnableVertexAttribArray(1);
glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, sizeof(vert_t), (void*)offset); //  offset=12 for 'col' causes failure in mglVertexAttribPointer 
offset += sizeof(float) * 3;

gabereiser commented 1 year ago

glVertexAttribPointer takes a stride for the interleaved attribute data. You pass in an entire vertex size. Instead of sizeof(vert_t) as the argument, you’ll find sizeof(float_t) * 3 to be correct. The API follows the spec. Your call wasn’t to spec.

redeemarr commented 1 year ago

@gabereiser, look carefully:

stride
Specifies the byte offset between consecutive generic vertex attributes

This means 'distance' between specific attribute data within vertex buffer. So, for interleaved structure like vert_t above it should be sizeof(vert_t). Untitled-1

pointer
Specifies a offset of the first component of the first generic vertex attribute in the array in the data store of the buffer

May be you've confused stride with pointer/offset?

Code like above conforms to spec and works for me as expected for years on different platforms.

gabereiser commented 1 year ago

https://registry.khronos.org/OpenGL-Refpages/gl4/html/glVertexAttribPointer.xhtml

An attribute is a component of your vertex, not your whole vertex

EDIT I just went and looked up my code (deep deep within my engine, haven’t touched in years) and apologies, you are correct, stride is the size of the vertex if your offset into the interleaved remains the same.

darkaegisagain commented 1 year ago

Maybe here in MGLRenderer.m...

(bool) bindBuffersToCurrentRenderEncoder ...

[_currentRenderEncoder setVertexBuffer:buffer offset:0 atIndex:i ];

offset:0 should be the vertex attribute offset?

Mike

"The difference between communism and socialism is that under socialism central planning ends with a gun in your face, whereas under communism central planning begins with a gun in your face." ― Kevin D. Williamson

On Sat, Oct 14, 2023 at 2:34 PM gabereiser @.***> wrote:

https://registry.khronos.org/OpenGL-Refpages/gl4/html/glVertexAttribPointer.xhtml

— Reply to this email directly, view it on GitHub https://github.com/openglonmetal/MGL/issues/69#issuecomment-1763186823, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACOKD33UZDXHVDITH35WJPDX7MAPJANCNFSM6AAAAAAUUT4ABE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

darkaegisagain commented 1 year ago

        vertexDescriptor.attributes[i].offset = ctx->state.vao->attrib

[i].base_plus_relative_offset;

Well the metal vertex descriptor does get set with the value assigned in mglVertexAttribPointer

Do you want to take a look? I can once I get up and running again

Mike

"The difference between communism and socialism is that under socialism central planning ends with a gun in your face, whereas under communism central planning begins with a gun in your face." ― Kevin D. Williamson

On Sat, Oct 14, 2023 at 7:11 PM Michael Larson @.***> wrote:

Maybe here in MGLRenderer.m...
(bool) bindBuffersToCurrentRenderEncoder ...
[_currentRenderEncoder setVertexBuffer:buffer offset:0 atIndex:i ];
offset:0 should be the vertex attribute offset?

Mike

"The difference between communism and socialism is that under socialism central planning ends with a gun in your face, whereas under communism central planning begins with a gun in your face." ― Kevin D. Williamson

On Sat, Oct 14, 2023 at 2:34 PM gabereiser @.***> wrote:

https://registry.khronos.org/OpenGL-Refpages/gl4/html/glVertexAttribPointer.xhtml

— Reply to this email directly, view it on GitHub https://github.com/openglonmetal/MGL/issues/69#issuecomment-1763186823, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACOKD33UZDXHVDITH35WJPDX7MAPJANCNFSM6AAAAAAUUT4ABE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

darkaegisagain commented 1 year ago

So here in processVAO the values from the attributes are assigned.

bool processVAO(GLMContext ctx)

{

VertexArray *vao;

vao = ctx->state.vao;

assert(vao);

*if* (vao->dirty_bits & DIRTY_VAO_BUFFER_BASE)

{

    // map buffer bindings to vertex array

    *for*(*int* i=0;i<ctx->state.max_vertex_attribs; i++)

    {

        *if* (vao->enabled_attribs & (0x1 << i))

        {

            BufferBinding *ptr;

            ptr = &vao->buffer_bindings[vao->attrib[i].

buffer_bindingindex];

            *if* (ptr->buffer)

            {

                // bind buffer at buffer bindings

                vao->attrib[i].buffer = ptr->buffer;

                // weird but this is what the spec says

                vao->attrib[i].base_plus_relative_offset = ptr->offset

vao->attrib[i].relativeoffset;

            assert(vao->attrib[i].base_plus_relative_offset % 4 == 0

);

            vao->attrib[i].stride = ptr->stride;

            vao->attrib[i].divisor = ptr->divisor;

        }

        *else* *if* (vao->attrib[i].buffer == *NULL*)

        {

            // no buffer bound to active attrib...

            *return* *false*;

        }

    }

and then these are used in generateVertexDescriptor for Metal

(MTLVertexDescriptor *)generateVertexDescriptor

{

MTLVertexDescriptor *vertexDescriptor = [[MTLVertexDescriptor alloc]

init];

assert(vertexDescriptor);

[vertexDescriptor reset]; // ??? debug

// we can bind a new vertex descriptor without creating a new

renderbuffer

*for*(*int* i=0;i<ctx->state.max_vertex_attribs; i++)

{

    *if* (VAO_STATE(enabled_attribs) & (0x1 << i))

    {

        MTLVertexFormat format;

        *if* (VAO_ATTRIB_STATE(i).buffer == *NULL*)

        {

            NSLog(@"Error: Invalid VAO defined enabled but no buffer

bound\n");

            *return* *NULL*;

        }

        format = glTypeSizeToMtlType(VAO_ATTRIB_STATE(i).type,

                                     VAO_ATTRIB_STATE(i).size,

                                     VAO_ATTRIB_STATE(i).normalized);

        *if* (format == MTLVertexFormatInvalid)

        {

            NSLog(@"Error: unable to map gl type / size / normalize to

format\n");

            *return* *false*;

        }

        *int* mapped_buffer_index;

        mapped_buffer_index = [*self*

getVertexBufferIndexWithAttributeSet: i];

        vertexDescriptor.attributes[i].bufferIndex =

mapped_buffer_index;

        vertexDescriptor.attributes[i].offset = ctx->state.vao->attrib

[i].base_plus_relative_offset;

        vertexDescriptor.attributes[i].format = format;

        vertexDescriptor.layouts[mapped_buffer_index].stride =

VAO_ATTRIB_STATE(i).stride;

        *if* (ctx->state.vao->attrib[i].divisor)

            vertexDescriptor.layouts[mapped_buffer_index].stepRate = ctx

->state.vao->attrib[i].divisor;

        *else*

            vertexDescriptor.layouts[mapped_buffer_index].stepRate = 1;

        vertexDescriptor.layouts[mapped_buffer_index].stepFunction =

MTLVertexStepFunctionPerVertex;

    }

    // early out

    *if* ((VAO_STATE(enabled_attribs) >> (i+1)) == 0)

        *break*;

It looks correct, I am working fixing the Xcode build.. It's been a while since I worked on this.

I released the source in the hopes that community would work this and do figure out the internals, its quite simple. I would appreciate if you have time to walk these routines with the debugger and figure out if the attribute offset is working as planned

Mike

"The difference between communism and socialism is that under socialism central planning ends with a gun in your face, whereas under communism central planning begins with a gun in your face." ― Kevin D. Williamson

On Sat, Oct 14, 2023 at 7:18 PM Michael Larson @.***> wrote:

        vertexDescriptor.attributes[i].offset = ctx->state.vao->attrib
[i].base_plus_relative_offset;

Well the metal vertex descriptor does get set with the value assigned in mglVertexAttribPointer

Do you want to take a look? I can once I get up and running again

Mike

"The difference between communism and socialism is that under socialism central planning ends with a gun in your face, whereas under communism central planning begins with a gun in your face." ― Kevin D. Williamson

On Sat, Oct 14, 2023 at 7:11 PM Michael Larson @.***> wrote:
Maybe here in MGLRenderer.m...
(bool) bindBuffersToCurrentRenderEncoder ...
[_currentRenderEncoder setVertexBuffer:buffer offset:0 atIndex:i
];
offset:0 should be the vertex attribute offset?

Mike

"The difference between communism and socialism is that under socialism central planning ends with a gun in your face, whereas under communism central planning begins with a gun in your face." ― Kevin D. Williamson

On Sat, Oct 14, 2023 at 2:34 PM gabereiser @.***> wrote:

https://registry.khronos.org/OpenGL-Refpages/gl4/html/glVertexAttribPointer.xhtml

— Reply to this email directly, view it on GitHub https://github.com/openglonmetal/MGL/issues/69#issuecomment-1763186823, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACOKD33UZDXHVDITH35WJPDX7MAPJANCNFSM6AAAAAAUUT4ABE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

darkaegisagain commented 1 year ago

Can you send a full example including the vertex program?

Thanks

Mike

"The difference between communism and socialism is that under socialism central planning ends with a gun in your face, whereas under communism central planning begins with a gun in your face." ― Kevin D. Williamson

On Sun, Oct 15, 2023 at 9:36 AM Michael Larson @.***> wrote:

So here in processVAO the values from the attributes are assigned.

bool processVAO(GLMContext ctx)

{
VertexArray *vao;

vao = ctx->state.vao;

assert(vao);

*if* (vao->dirty_bits & DIRTY_VAO_BUFFER_BASE)

{

    // map buffer bindings to vertex array

    *for*(*int* i=0;i<ctx->state.max_vertex_attribs; i++)

    {

        *if* (vao->enabled_attribs & (0x1 << i))

        {

            BufferBinding *ptr;

            ptr = &vao->buffer_bindings[vao->attrib[i].
buffer_bindingindex];
            *if* (ptr->buffer)

            {

                // bind buffer at buffer bindings

                vao->attrib[i].buffer = ptr->buffer;

                // weird but this is what the spec says

                vao->attrib[i].base_plus_relative_offset = ptr->offset
vao->attrib[i].relativeoffset;
            assert(vao->attrib[i].base_plus_relative_offset % 4
== 0);
            vao->attrib[i].stride = ptr->stride;

            vao->attrib[i].divisor = ptr->divisor;

        }

        *else* *if* (vao->attrib[i].buffer == *NULL*)

        {

            // no buffer bound to active attrib...

            *return* *false*;

        }

    }
and then these are used in generateVertexDescriptor for Metal

(MTLVertexDescriptor *)generateVertexDescriptor

{
MTLVertexDescriptor *vertexDescriptor = [[MTLVertexDescriptor alloc]
init];
assert(vertexDescriptor);

[vertexDescriptor reset]; // ??? debug

// we can bind a new vertex descriptor without creating a new
renderbuffer
*for*(*int* i=0;i<ctx->state.max_vertex_attribs; i++)

{

    *if* (VAO_STATE(enabled_attribs) & (0x1 << i))

    {

        MTLVertexFormat format;

        *if* (VAO_ATTRIB_STATE(i).buffer == *NULL*)

        {

            NSLog(@"Error: Invalid VAO defined enabled but no buffer
bound\n");
            *return* *NULL*;

        }

        format = glTypeSizeToMtlType(VAO_ATTRIB_STATE(i).type,

                                     VAO_ATTRIB_STATE(i).size,

                                     VAO_ATTRIB_STATE(i).normalized);

        *if* (format == MTLVertexFormatInvalid)

        {

            NSLog(@"Error: unable to map gl type / size / normalize
to format\n");
            *return* *false*;

        }

        *int* mapped_buffer_index;

        mapped_buffer_index = [*self*
getVertexBufferIndexWithAttributeSet: i];
        vertexDescriptor.attributes[i].bufferIndex =
mapped_buffer_index;
        vertexDescriptor.attributes[i].offset = ctx->state.vao->attrib
[i].base_plus_relative_offset;
        vertexDescriptor.attributes[i].format = format;

        vertexDescriptor.layouts[mapped_buffer_index].stride =
VAO_ATTRIB_STATE(i).stride;
        *if* (ctx->state.vao->attrib[i].divisor)

            vertexDescriptor.layouts[mapped_buffer_index].stepRate =
ctx->state.vao->attrib[i].divisor;
        *else*

            vertexDescriptor.layouts[mapped_buffer_index].stepRate = 1
;
        vertexDescriptor.layouts[mapped_buffer_index].stepFunction =
MTLVertexStepFunctionPerVertex;
    }

    // early out

    *if* ((VAO_STATE(enabled_attribs) >> (i+1)) == 0)

        *break*;
It looks correct, I am working fixing the Xcode build.. It's been a while since I worked on this.

I released the source in the hopes that community would work this and do figure out the internals, its quite simple. I would appreciate if you have time to walk these routines with the debugger and figure out if the attribute offset is working as planned

Mike

"The difference between communism and socialism is that under socialism central planning ends with a gun in your face, whereas under communism central planning begins with a gun in your face." ― Kevin D. Williamson

On Sat, Oct 14, 2023 at 7:18 PM Michael Larson @.***> wrote:
        vertexDescriptor.attributes[i].offset = ctx->state.vao->
attrib[i].base_plus_relative_offset;

Well the metal vertex descriptor does get set with the value assigned in mglVertexAttribPointer

Do you want to take a look? I can once I get up and running again

Mike

"The difference between communism and socialism is that under socialism central planning ends with a gun in your face, whereas under communism central planning begins with a gun in your face." ― Kevin D. Williamson

On Sat, Oct 14, 2023 at 7:11 PM Michael Larson @.***> wrote:
Maybe here in MGLRenderer.m...
(bool) bindBuffersToCurrentRenderEncoder ...
[_currentRenderEncoder setVertexBuffer:buffer offset:0 atIndex:i
];
offset:0 should be the vertex attribute offset?

Mike

"The difference between communism and socialism is that under socialism central planning ends with a gun in your face, whereas under communism central planning begins with a gun in your face." ― Kevin D. Williamson

On Sat, Oct 14, 2023 at 2:34 PM gabereiser @.***> wrote:

https://registry.khronos.org/OpenGL-Refpages/gl4/html/glVertexAttribPointer.xhtml

— Reply to this email directly, view it on GitHub https://github.com/openglonmetal/MGL/issues/69#issuecomment-1763186823, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACOKD33UZDXHVDITH35WJPDX7MAPJANCNFSM6AAAAAAUUT4ABE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

darkaegisagain commented 1 year ago

Hello?

darkaegisagain commented 1 year ago

Closing

openglonmetal / MGL

glVertexAttribPointer doesn't work with offset #69