[Cogl] [PATCH] attribute: Adds support for constant CoglAttributes

Neil Roberts neil at linux.intel.com
Fri Sep 28 09:08:09 PDT 2012


Here are some comments inline:

Robert Bragg <robert at sixbynine.org> writes:

> +static CoglAttribute *
> +_cogl_attribute_new_const (CoglContext *context,
> +                           const char *name,
> +                           int n_components,
> +                           int n_columns,
> +                           CoglBool transpose,
> +                           const float *value)

It seems like this should use the default value for whether the
attribute is normalized and also verify that the number of components
makes sense in the same way that cogl_attribute_new does.

> +                                    2, /* 1 column vector */

This comment and the other similar ones look like a cut-and-paste error.

>  static void
>  _cogl_attribute_free (CoglAttribute *attribute)
>  {
> -  cogl_object_unref (attribute->attribute_buffer);
> +  cogl_object_unref (attribute->d.buffered.attribute_buffer);

Shouldn't this only free the buffer if the attribute isn't constant? If
the attribute is constant presumably it needs to destroy the boxed value
too, although maybe that doesn't really matter because all of the
possible boxed values don't have any extra allocation anyway.

> + * The constant @value is a single precision floating point scalar
> + * which should have a corresponding declaration in GLSL code like:
> + *
> + * [|
> + * varying float name;
> + * |]

The corresponding variable in GLSL would be an attribute not a varying.

I wonder if we need some way to be able to modify the constant value. It
would be kind of a shame if an application was repeatedly drawing with
different values and they had to recreate the attribute every time
because it would repeatedly have to revalidate the attribute name. I
guess we could always add some extra API for that later though so maybe
it's not worth worrying about for now.

Regards,
- Neil


More information about the Cogl mailing list