[Mesa-dev] [PATCH 4/9] mesa: Uniform Buffer Objects data structures

[Eric Anholt]

On Thu,  1 Dec 2011 16:35:33 +0100, Vincent Lejeune <vljn at ovi.com> wrote:
>    v2:Big cleanup of data structures used
>    v3:Data structures moved to mtypes.h

A 100-line commit really needs more than a 1-line commit message.

> ---
>  src/mesa/main/config.h            |    4 ++
>  src/mesa/main/mtypes.h            |  109 +++++++++++++++++++++++++++++++++++++
>  src/mesa/main/shaderobj.c         |   25 ++++++++-
>  src/mesa/program/prog_parameter.h |    1 +
>  4 files changed, 138 insertions(+), 1 deletions(-)
> 
> diff --git a/src/mesa/main/config.h b/src/mesa/main/config.h
> index 7b7740e..1c4c810 100644
> --- a/src/mesa/main/config.h
> +++ b/src/mesa/main/config.h
> @@ -357,5 +357,9 @@
>   */
>  #define MAX_CLIPPED_VERTICES ((2 * (6 + MAX_CLIP_PLANES))+1)
>  
> +/**
> + * UBO Variables
> + */
> +#define MAX_UBO_COUNT 8

What does this comment tell me?  If there's a comment, it should
probably explain why 8, or what this 8 is used for.

In particular, looking at ARB_uniform_buffer_object.txt, I see minimum
maximums of things I think this MAX_UBO_COUNT variable might be
affecting actually being a minimum maximum of 12 or 36.

>  #endif /* MESA_CONFIG_H_INCLUDED */
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 08cd80a..654ae98 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1653,6 +1653,14 @@ struct gl_array_attrib
>     struct gl_buffer_object *ArrayBufferObj;
>  };
>  
> +/**
> + * UBO state
> + */
> +struct gl_ubo_attribs
> +{
> +    struct gl_buffer_object *UniformObj;
> +    struct gl_buffer_object *BindingPoint[MAX_UBO_COUNT];
> +};
>  
>  /**
>   * Feedback buffer state
> @@ -2127,6 +2135,82 @@ struct gl_sl_pragmas
>     GLboolean Debug;     /**< defaults off */
>  };
>  
> +/**
> + * Uniform Buffer Object variable informations.
> + * This come in 2 flavors :
> + * As layout informations (Size,Offset,...) are defined per program and not per shader,
> + * shaders_ubo_variable only store the relevant information to preserve memory.
> + * program_ubo_variable stores everything that can be retrieved by GetActiveUniformsiv
> + * (which makes it bigger than the data it refers to...).
> + */
> +
> +struct gl_shader_ubo_variable
> +{
> +   char* Name;
> +   const struct glsl_type* Type;
> +};
> +
> +struct gl_program_ubo_variable
> +{
> +   char* Name;
> +   GLenum Type; /** TYPE_ARRAY,TYPE_RECORD,TYPE_FLOAT,TYPE_VEC */
> +   GLuint Size; /** In bytes */
> +   GLuint Offset; /** In bytes, from start of UBO */
> +   GLuint Stride; /** In TYPE_ARRAY case, stride between 2 consecutive elements, undef otherwise */
> +   GLuint IndexInUBO; /** Position inside UBO declaration */
> +   GLuint ContainedVariablesCount; /** For TYPE_ARRAY and TYPE_RECORD, number of element in ContainedVariables */
> +   struct gl_program_ubo_variable* ContainedVariables; /** For TYPE_ARRAY and TYPE_RECORD, array holding child data */
> +   struct gl_uniform_buffer_object* UBO; /** Pointer to containing UBO structure inside program */
> +};
> +
> +
> +/**
> + * glsl_base_type doesn't have a VEC type,
> + * we have to define our own enum here
> + * FIXME:investigate if glsl_base_type can accept another item ?
> + */
> +enum {
> +   TYPE_ARRAY,
> +   TYPE_RECORD,
> +   TYPE_SCALAR,
> +   TYPE_VEC,
> +};
> +
> +enum UBOLayout {
> +   UBO_LAYOUT_PACKED,
> +   UBO_LAYOUT_SHARED,
> +   UBO_LAYOUT_SDT140,
> +};
> +
> +enum UBOMatrixLayout {
> +   UBO_MATRIX_LAYOUT_ROW_MAJOR,
> +   UBO_MATRIX_LAYOUT_COLUMN_MAJOR,
> +};
> +
> +/**
> + * Uniform Buffer Object Information.
> + * This struct is used by shader and program struct ;
> + * For StorageLayout, Compact form is used in shader,
> + * Full form is used in program.
> + */
> +
> +struct gl_uniform_buffer_object
> +{
> +   char* Name;
> +   GLuint Index;
> +   union {
> +      struct gl_program_ubo_variable*  Full;  /** Malloced, sefined in program */
> +      struct gl_shader_ubo_variable* Compact; /** Ralloced, defined in shader */
> +   } StorageLayout; /** array of variables that are part of the UBO  */
> +   GLuint Layout; /** packed, shared or std140 */
> +   GLuint MatrixLayout; /** rowmajor or columnmajor */
> +   GLuint NumberOfVariables; /**< number of UBOVariableInfo in StorageLayout */
> +   unsigned ReferencedByVS:1; /** Is it present in VS ? Set at link time */
> +   unsigned ReferencedByGS:1; /** Is it present in GS ? Set at link time */
> +   unsigned ReferencedByFS:1; /** Is it present in FS ? Set at link time */
> +   unsigned Size; /** In bytes, with padding included */
> +};

I think these structure definitions would do better in the commits where
they start actually getting used.  As is, I can't review in this commit
whether this is reasonable stuff to be here or not, because I can't see
how it's used.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111202/f0695f38/attachment.pgp>