[Cogl] [PATCH] Unify a lot of gles2 vs gl glsl code

Neil Roberts neil at linux.intel.com
Fri Sep 28 07:56:51 PDT 2012


This looks good except for the following comments inline:

Robert Bragg <robert at sixbynine.org> writes:

> -  /* We can't assume the color will be retained between flushes on
> -     GLES2 because the generic attribute values are not stored as part
> -     of the program object so they could be overridden by any
> -     attribute changes in another program */
> -#ifdef HAVE_COGL_GLES2
> -  if (ctx->driver == COGL_DRIVER_GLES2 && !skip_gl_color)
> +  /* We can't assume the color will be retained between flushes when
> +   * using the glsl progend because the generic attribute values are
> +   * not stored as part of the program object so they could be
> +   * overridden by any attribute changes in another program */
> +  if (pipeline->progend == COGL_PIPELINE_PROGEND_GLSL && !skip_gl_color)

This should probably be wrapped with #ifdef COGL_PIPELINE_PROGEND_GLSL
because that won't be defined when building for GLES1 so it won't
compile.

> +
> +  if ((COGL_CHECK_GL_VERSION (gl_major, gl_minor, 2, 0) ||
> +       _cogl_check_extension ("GL_ARB_point_sprite", gl_extensions)) &&
> +
> +      /* If GLSL is supported then we only enable point sprite support
> +       * too if we have glsl >= 1.2 otherwise we don't have the
> +       * gl_PointCoord builtin which we depend on in the glsl backend.
> +       */
> +      ((flags & COGL_FEATURE_SHADERS_GLSL) == 0 ||
> +       COGL_CHECK_GL_VERSION (ctx->glsl_major, ctx->glsl_minor, 1, 2)))
> +    {
> +      flags |= COGL_FEATURE_POINT_SPRITE;
> +      COGL_FLAGS_SET (ctx->features, COGL_FEATURE_ID_POINT_SPRITE, TRUE);
> +    }
> +
>    /* If all of the old GLSL extensions are available then we can fake
>     * the GL 2.0 GLSL support by diverting to the old function names */
>    else if (ctx->glCreateProgramObject && /* GL_ARB_shader_objects */

This check for point sprites has been added in the middle if an if-else
block that checks for GLSL support. Now the 'else' part of that check is
attached to the check for point sprites which is surely wrong.

For this patch to be rebased on top of master, it will need to query the
feature flag with COGL_FLAGS_GET (ctx->features,
COGL_FEATURE_SHADERS_GLSL) instead of looking at ‘flags’ because that
has now been removed.

Regards,
- Neil


More information about the Cogl mailing list