From 04d943579b739ea844e6d67231df52157f120139 Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Fri, 16 Aug 2013 21:23:37 +0100 Subject: [PATCH] gl: bind position attribute to location 0 Full GL treats the position attribute specially and requires that it must be bound to generic attribute location 0 unlike GLES 2.0 or GL 3.2 core. We now make sure to unconditionally bind the cogl_position_in attribute to location 0 before linking any glsl program in cogl. For reference the relevant part of the GL 3.0 spec that covers these semantics is Section 2.7 "Vertex Specification" pg 27 After this change there was one remaining problem in test-custom-attributes where the test_short_verts() test was using its own "pos" attribute instead of cogl_position_in and so cogl wasn't able to ensure it would be bound to location 0. This updates the test to use cogl_position_in but to work around the fact that glVertexPointer doesn't support UNSIGNED_SHORT components we force the test to use the glsl backend by setting a shader snippet on the pipeline. https://bugs.freedesktop.org/show_bug.cgi?id=67548 Reviewed-by: Neil Roberts (cherry picked from commit 992ef7b3b49ebb56adde2133bb36330c04133a3f) --- cogl/driver/gl/cogl-pipeline-progend-glsl.c | 7 ++++ tests/conform/test-custom-attributes.c | 45 ++++++++++++--------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/cogl/driver/gl/cogl-pipeline-progend-glsl.c b/cogl/driver/gl/cogl-pipeline-progend-glsl.c index ee8de2ada..d59e74859 100644 --- a/cogl/driver/gl/cogl-pipeline-progend-glsl.c +++ b/cogl/driver/gl/cogl-pipeline-progend-glsl.c @@ -742,6 +742,13 @@ _cogl_pipeline_progend_glsl_end (CoglPipeline *pipeline, if ((backend_shader = _cogl_pipeline_vertend_glsl_get_shader (pipeline))) GE( ctx, glAttachShader (program_state->program, backend_shader) ); + /* XXX: OpenGL as a special case requires the vertex position to + * be bound to generic attribute 0 so for simplicity we + * unconditionally bind the cogl_position_in attribute here... + */ + GE( ctx, glBindAttribLocation (program_state->program, + 0, "cogl_position_in")); + link_program (program_state->program); program_changed = TRUE; diff --git a/tests/conform/test-custom-attributes.c b/tests/conform/test-custom-attributes.c index c27dfa19b..633dc2ad8 100644 --- a/tests/conform/test-custom-attributes.c +++ b/tests/conform/test-custom-attributes.c @@ -24,6 +24,7 @@ typedef struct typedef struct { int16_t x, y; + int16_t r, g, b, a; } ShortVert; static void @@ -167,7 +168,7 @@ test_byte_verts (TestState *state, int offset_x, int offset_y) static void test_short_verts (TestState *state, int offset_x, int offset_y) { - CoglAttribute *attributes[1]; + CoglAttribute *attributes[2]; CoglAttributeBuffer *buffer; CoglPipeline *pipeline, *pipeline2; CoglSnippet *snippet; @@ -175,32 +176,31 @@ test_short_verts (TestState *state, int offset_x, int offset_y) static const ShortVert short_verts[] = { - { -10, -10 }, - { -1, -10 }, - { -5, -1 } + { -10, -10, /**/ 0xffff, 0, 0, 0xffff }, + { -1, -10, /**/ 0xffff, 0, 0, 0xffff }, + { -5, -1, /**/ 0xffff, 0, 0, 0xffff } }; - pipeline = cogl_pipeline_new (test_ctx); - snippet = cogl_snippet_new (COGL_SNIPPET_HOOK_VERTEX_TRANSFORM, - "attribute vec2 pos;", - NULL); - cogl_snippet_set_replace (snippet, - "cogl_position_out = " - "cogl_modelview_projection_matrix * " - "vec4 (pos, 0.0, 1.0);"); - cogl_pipeline_add_snippet (pipeline, snippet); - cogl_object_unref (snippet); + + pipeline = cogl_pipeline_copy (state->pipeline); cogl_pipeline_set_color4ub (pipeline, 255, 0, 0, 255); buffer = cogl_attribute_buffer_new (test_ctx, sizeof (short_verts), short_verts); attributes[0] = cogl_attribute_new (buffer, - "pos", + "cogl_position_in", sizeof (ShortVert), G_STRUCT_OFFSET (ShortVert, x), 2, /* n_components */ COGL_ATTRIBUTE_TYPE_SHORT); + attributes[1] = cogl_attribute_new (buffer, + "color", + sizeof (ShortVert), + G_STRUCT_OFFSET (ShortVert, r), + 4, /* n_components */ + COGL_ATTRIBUTE_TYPE_UNSIGNED_SHORT); + cogl_attribute_set_normalized (attributes[1], TRUE); cogl_framebuffer_push_matrix (test_fb); cogl_framebuffer_translate (test_fb, @@ -211,7 +211,7 @@ test_short_verts (TestState *state, int offset_x, int offset_y) primitive = cogl_primitive_new_with_attributes (COGL_VERTICES_MODE_TRIANGLES, 3, /* n_vertices */ attributes, - 1); /* n_attributes */ + 2); /* n_attributes */ cogl_primitive_draw (primitive, test_fb, pipeline); cogl_object_unref (primitive); @@ -221,14 +221,21 @@ test_short_verts (TestState *state, int offset_x, int offset_y) /* Test again treating the attribute as unsigned */ attributes[0] = cogl_attribute_new (buffer, - "pos", + "cogl_position_in", sizeof (ShortVert), G_STRUCT_OFFSET (ShortVert, x), 2, /* n_components */ COGL_ATTRIBUTE_TYPE_UNSIGNED_SHORT); - pipeline2 = cogl_pipeline_copy (pipeline); - cogl_pipeline_set_color4ub (pipeline2, 0, 255, 0, 255); + /* XXX: this is a hack to force the pipeline to use the glsl backend + * because we know it's not possible to test short vertex position + * components with the legacy GL backend since which might otherwise + * be used internally... */ + pipeline2 = cogl_pipeline_new (test_ctx); + snippet = cogl_snippet_new (COGL_SNIPPET_HOOK_VERTEX, + "attribute vec4 color;", + "cogl_color_out = vec4 (0.0, 1.0, 0.0, 1.0);"); + cogl_pipeline_add_snippet (pipeline2, snippet); cogl_framebuffer_push_matrix (test_fb); cogl_framebuffer_translate (test_fb,