[Mesa-dev] [PATCH 4/4] glsl: Disallow relinking if a program is used by an active XFB object.

[Marek Olšák]

On Sat, Sep 7, 2013 at 1:40 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> Paused transform feedback objects may refer to a program other than the
> current program.  If any active objects refer to a program, LinkProgram
> must reject the request to relink.
>
> The code to detect this is ugly since _mesa_HashWalk is awkward to use,
> but unfortunately we can't use hash_table_foreach since there's no way
> to get at the underlying struct hash_table (and even then, we'd need to
> handle locking somehow).
>
> Fixes the last subcase of Piglit's new ARB_transform_feedback2
> api-errors test.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/main/shaderapi.c         | 16 +++++++++-------
>  src/mesa/main/transformfeedback.c | 35 +++++++++++++++++++++++++++++++++++
>  src/mesa/main/transformfeedback.h |  4 ++++
>  3 files changed, 48 insertions(+), 7 deletions(-)
>
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 4fe9d9c..30aa83c 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -42,6 +42,7 @@
>  #include "main/dispatch.h"
>  #include "main/enums.h"
>  #include "main/hash.h"
> +#include "main/hash_table.h"
>  #include "main/mtypes.h"
>  #include "main/shaderapi.h"
>  #include "main/shaderobj.h"
> @@ -812,19 +813,20 @@ static void
>  link_program(struct gl_context *ctx, GLuint program)
>  {
>     struct gl_shader_program *shProg;
> -   struct gl_transform_feedback_object *obj =
> -      ctx->TransformFeedback.CurrentObject;
> +   struct gl_transform_feedback_object *obj;

This obj variable seems to be unused.

Other than that, this series is:

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

>
>     shProg = _mesa_lookup_shader_program_err(ctx, program, "glLinkProgram");
>     if (!shProg)
>        return;
>
> -   if (obj->Active
> -       && (shProg == ctx->Shader.CurrentVertexProgram
> -          || shProg == ctx->Shader.CurrentGeometryProgram
> -          || shProg == ctx->Shader.CurrentFragmentProgram)) {
> +   /* From the ARB_transform_feedback2 specification:
> +    * "The error INVALID_OPERATION is generated by LinkProgram if <program> is
> +    *  the name of a program being used by one or more transform feedback
> +    *  objects, even if the objects are not currently bound or are paused."
> +    */
> +   if (_mesa_transform_feedback_is_using_program(ctx, shProg)) {
>        _mesa_error(ctx, GL_INVALID_OPERATION,
> -                  "glLinkProgram(transform feedback active)");
> +                  "glLinkProgram(transform feedback is using the program)");
>        return;
>     }
>
> diff --git a/src/mesa/main/transformfeedback.c b/src/mesa/main/transformfeedback.c
> index 191e88c..bc9b52a 100644
> --- a/src/mesa/main/transformfeedback.c
> +++ b/src/mesa/main/transformfeedback.c
> @@ -44,6 +44,41 @@
>
>  #include "program/prog_parameter.h"
>
> +struct using_program_tuple
> +{
> +   struct gl_shader_program *shProg;
> +   bool found;
> +};
> +
> +static void
> +active_xfb_object_references_program(GLuint key, void *data, void *user_data)
> +{
> +   struct using_program_tuple *callback_data = user_data;
> +   struct gl_transform_feedback_object *obj = data;
> +   if (obj->Active && obj->shader_program == callback_data->shProg)
> +      callback_data->found = true;
> +}
> +
> +/**
> + * Return true if any active transform feedback object is using a program.
> + */
> +bool
> +_mesa_transform_feedback_is_using_program(struct gl_context *ctx,
> +                                          struct gl_shader_program *shProg)
> +{
> +   struct using_program_tuple callback_data;
> +   callback_data.shProg = shProg;
> +   callback_data.found = false;
> +
> +   _mesa_HashWalk(ctx->TransformFeedback.Objects,
> +                  active_xfb_object_references_program, &callback_data);
> +
> +   /* Also check DefaultObject, as it's not in the Objects hash table. */
> +   active_xfb_object_references_program(0, ctx->TransformFeedback.DefaultObject,
> +                                        &callback_data);
> +
> +   return callback_data.found;
> +}
>
>  /**
>   * Do reference counting of transform feedback buffers.
> diff --git a/src/mesa/main/transformfeedback.h b/src/mesa/main/transformfeedback.h
> index f6dc2a7..0ffaab5 100644
> --- a/src/mesa/main/transformfeedback.h
> +++ b/src/mesa/main/transformfeedback.h
> @@ -120,4 +120,8 @@ _mesa_is_xfb_active_and_unpaused(const struct gl_context *ctx)
>        !ctx->TransformFeedback.CurrentObject->Paused;
>  }
>
> +extern bool
> +_mesa_transform_feedback_is_using_program(struct gl_context *ctx,
> +                                          struct gl_shader_program *shProg);
> +
>  #endif /* TRANSFORM_FEEDBACK_H */
> --
> 1.8.3.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev