[Cogl] [PATCH 3/4] Always add the #version pragma to shaders
Robert Bragg
robert at sixbynine.org
Mon Sep 2 06:33:48 PDT 2013
This looks good to land to me:
Reviewed-by: Robert Bragg <robert at linux.intel.com>
thanks,
Robert
On Sun, Aug 25, 2013 at 2:42 AM, Neil Roberts <neil at linux.intel.com> wrote:
> Previously we would only add the #version pragma to shaders when
> point sprite texture coordinates are enabled for a layer so that we
> can access the gl_PointCoord builtin. However I don't think there's
> any good reason not to just always request GLSL version 1.2 if it's
> available. That way applications can always use gl_PointCoord without
> having to enable point sprite texture coordinates.
>
> This adds a glsl_version_to_use member to CoglContext which is used to
> generate the #version pragma as part of the shader boilerplate. On
> desktop GL this is set to 120 if version 1.2 is available, otherwise
> it is left at 110. On GLES it is always left as 100.
> ---
> cogl/cogl-context-private.h | 8 ++++++++
> cogl/cogl-glsl-shader-private.h | 1 -
> cogl/cogl-glsl-shader.c | 13 +++++++------
> cogl/driver/gl/cogl-pipeline-fragend-glsl.c | 18 ++----------------
> cogl/driver/gl/cogl-pipeline-vertend-glsl.c | 1 -
> cogl/driver/gl/gl/cogl-driver-gl.c | 7 +++++++
> cogl/driver/gl/gles/cogl-driver-gles.c | 1 +
> 7 files changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/cogl/cogl-context-private.h b/cogl/cogl-context-private.h
> index c5f215d..502740d 100644
> --- a/cogl/cogl-context-private.h
> +++ b/cogl/cogl-context-private.h
> @@ -79,6 +79,14 @@ struct _CoglContext
> int glsl_major;
> int glsl_minor;
>
> + /* This is the GLSL version that we will claim that snippets are
> + * written against using the #version pragma. This will be the
> + * largest version that is less than or equal to the version
> + * provided by the driver without massively altering the syntax. Eg,
> + * we wouldn't use version 1.3 even if it is available because that
> + * removes the ‘attribute’ and ‘varying’ keywords. */
> + int glsl_version_to_use;
> +
> /* Features cache */
> unsigned long features[COGL_FLAGS_N_LONGS_FOR_SIZE (_COGL_N_FEATURE_IDS)];
> CoglPrivateFeatureFlags private_feature_flags;
> diff --git a/cogl/cogl-glsl-shader-private.h b/cogl/cogl-glsl-shader-private.h
> index 1ec9762..f0c0aa5 100644
> --- a/cogl/cogl-glsl-shader-private.h
> +++ b/cogl/cogl-glsl-shader-private.h
> @@ -25,7 +25,6 @@
>
> void
> _cogl_glsl_shader_set_source_with_boilerplate (CoglContext *ctx,
> - const char *version_string,
> GLuint shader_gl_handle,
> GLenum shader_gl_type,
> GLsizei count_in,
> diff --git a/cogl/cogl-glsl-shader.c b/cogl/cogl-glsl-shader.c
> index b469ede..6bdd266 100644
> --- a/cogl/cogl-glsl-shader.c
> +++ b/cogl/cogl-glsl-shader.c
> @@ -41,7 +41,6 @@
>
> void
> _cogl_glsl_shader_set_source_with_boilerplate (CoglContext *ctx,
> - const char *version_string,
> GLuint shader_gl_handle,
> GLenum shader_gl_type,
> GLsizei count_in,
> @@ -53,16 +52,16 @@ _cogl_glsl_shader_set_source_with_boilerplate (CoglContext *ctx,
>
> const char **strings = g_alloca (sizeof (char *) * (count_in + 4));
> GLint *lengths = g_alloca (sizeof (GLint) * (count_in + 4));
> + char *version_string;
> int count = 0;
>
> vertex_boilerplate = _COGL_VERTEX_SHADER_BOILERPLATE;
> fragment_boilerplate = _COGL_FRAGMENT_SHADER_BOILERPLATE;
>
> - if (version_string)
> - {
> - strings[count] = version_string;
> - lengths[count++] = -1;
> - }
> + version_string = g_strdup_printf ("#version %i\n\n",
> + ctx->glsl_version_to_use);
> + strings[count] = version_string;
> + lengths[count++] = -1;
>
> if (ctx->private_feature_flags & COGL_PRIVATE_FEATURE_GL_EMBEDDED &&
> cogl_has_feature (ctx, COGL_FEATURE_ID_TEXTURE_3D))
> @@ -118,4 +117,6 @@ _cogl_glsl_shader_set_source_with_boilerplate (CoglContext *ctx,
>
> GE( ctx, glShaderSource (shader_gl_handle, count,
> (const char **) strings, lengths) );
> +
> + g_free (version_string);
> }
> diff --git a/cogl/driver/gl/cogl-pipeline-fragend-glsl.c b/cogl/driver/gl/cogl-pipeline-fragend-glsl.c
> index 6274212..46264a9 100644
> --- a/cogl/driver/gl/cogl-pipeline-fragend-glsl.c
> +++ b/cogl/driver/gl/cogl-pipeline-fragend-glsl.c
> @@ -88,8 +88,6 @@ typedef struct
> GString *header, *source;
> UnitState *unit_state;
>
> - CoglBool ref_point_coord;
> -
> /* List of layers that we haven't generated code for yet. These are
> in reverse order. As soon as we're about to generate code for
> layer we'll remove it from the list so we don't generate it
> @@ -111,7 +109,6 @@ shader_state_new (int n_layers)
> shader_state = g_slice_new0 (CoglPipelineShaderState);
> shader_state->ref_count = 1;
> shader_state->unit_state = g_new0 (UnitState, n_layers);
> - shader_state->ref_point_coord = FALSE;
>
> return shader_state;
> }
> @@ -410,11 +407,8 @@ ensure_texture_lookup_generated (CoglPipelineShaderState *shader_state,
>
> if (cogl_pipeline_get_layer_point_sprite_coords_enabled (pipeline,
> layer->index))
> - {
> - shader_state->ref_point_coord = TRUE;
> - g_string_append_printf (shader_state->source,
> - "vec4 (gl_PointCoord, 0.0, 1.0)");
> - }
> + g_string_append_printf (shader_state->source,
> + "vec4 (gl_PointCoord, 0.0, 1.0)");
> else
> g_string_append_printf (shader_state->source,
> "cogl_tex_coord%i_in",
> @@ -977,7 +971,6 @@ _cogl_pipeline_fragend_glsl_end (CoglPipeline *pipeline,
> GLint compile_status;
> GLuint shader;
> CoglPipelineSnippetData snippet_data;
> - const char *version_string;
>
> COGL_STATIC_COUNTER (fragend_glsl_compile_counter,
> "glsl fragment compile counter",
> @@ -1040,14 +1033,7 @@ _cogl_pipeline_fragend_glsl_end (CoglPipeline *pipeline,
> lengths[1] = shader_state->source->len;
> source_strings[1] = shader_state->source->str;
>
> - if (shader_state->ref_point_coord &&
> - !(ctx->private_feature_flags & COGL_PRIVATE_FEATURE_GL_EMBEDDED))
> - version_string = "#version 120\n";
> - else
> - version_string = NULL;
> -
> _cogl_glsl_shader_set_source_with_boilerplate (ctx,
> - version_string,
> shader, GL_FRAGMENT_SHADER,
> 2, /* count */
> source_strings, lengths);
> diff --git a/cogl/driver/gl/cogl-pipeline-vertend-glsl.c b/cogl/driver/gl/cogl-pipeline-vertend-glsl.c
> index 5dd7aee..33d67de 100644
> --- a/cogl/driver/gl/cogl-pipeline-vertend-glsl.c
> +++ b/cogl/driver/gl/cogl-pipeline-vertend-glsl.c
> @@ -485,7 +485,6 @@ _cogl_pipeline_vertend_glsl_end (CoglPipeline *pipeline,
> source_strings[1] = shader_state->source->str;
>
> _cogl_glsl_shader_set_source_with_boilerplate (ctx,
> - NULL,
> shader, GL_VERTEX_SHADER,
> 2, /* count */
> source_strings, lengths);
> diff --git a/cogl/driver/gl/gl/cogl-driver-gl.c b/cogl/driver/gl/gl/cogl-driver-gl.c
> index 7c1b856..40d76cb 100644
> --- a/cogl/driver/gl/gl/cogl-driver-gl.c
> +++ b/cogl/driver/gl/gl/cogl-driver-gl.c
> @@ -401,6 +401,13 @@ _cogl_driver_update_features (CoglContext *ctx,
> parse_gl_version (glsl_version, &ctx->glsl_major, &ctx->glsl_minor);
> }
>
> + if (COGL_CHECK_GL_VERSION (ctx->glsl_major, ctx->glsl_minor, 1, 2))
> + /* We want to use version 120 if it is available so that the
> + * gl_PointCoord can be used. */
> + ctx->glsl_version_to_use = 120;
> + else
> + ctx->glsl_version_to_use = 110;
> +
> COGL_FLAGS_SET (ctx->features,
> COGL_FEATURE_ID_UNSIGNED_INT_INDICES, TRUE);
> COGL_FLAGS_SET (ctx->features, COGL_FEATURE_ID_DEPTH_RANGE, TRUE);
> diff --git a/cogl/driver/gl/gles/cogl-driver-gles.c b/cogl/driver/gl/gles/cogl-driver-gles.c
> index 6826aac..d33a80e 100644
> --- a/cogl/driver/gl/gles/cogl-driver-gles.c
> +++ b/cogl/driver/gl/gles/cogl-driver-gles.c
> @@ -217,6 +217,7 @@ _cogl_driver_update_features (CoglContext *context,
>
> context->glsl_major = 1;
> context->glsl_minor = 0;
> + context->glsl_version_to_use = 100;
>
> _cogl_gpu_info_init (context, &context->gpu);
>
> --
> 1.8.3.1
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl
More information about the Cogl
mailing list