[Mesa-dev] [PATCH 1/2] mesa: add Uniform Buffer Object API implementation

Brian Paul brianp at vmware.com
Fri Dec 2 06:33:30 PST 2011


On 12/02/2011 06:22 AM, Vincent Lejeune wrote:
>     v2:Move implementation to ubo.cpp and ubo.h as suggested by
>     Brian Paul
> ---
>   src/mesa/main/api_exec.c                     |    2 +
>   src/mesa/main/bufferobj.c                    |   11 +
>   src/mesa/main/ubo.cpp                        |  263 ++++++++++++++++++++++++++
>   src/mesa/main/ubo.h                          |   33 ++++
>   src/mesa/sources.mak                         |    1 +
>   src/mesa/state_tracker/st_cb_bufferobjects.c |    6 +-
>   6 files changed, 315 insertions(+), 1 deletions(-)
>   create mode 100644 src/mesa/main/ubo.cpp
>   create mode 100644 src/mesa/main/ubo.h
>
> diff --git a/src/mesa/main/api_exec.c b/src/mesa/main/api_exec.c
> index e0bf90d..c908719 100644
> --- a/src/mesa/main/api_exec.c
> +++ b/src/mesa/main/api_exec.c
> @@ -102,6 +102,7 @@
>   #if FEATURE_ARB_shader_objects
>   #include "shaderapi.h"
>   #include "uniforms.h"
> +#include "ubo.h"
>   #endif
>   #include "syncobj.h"
>   #include "main/dispatch.h"
> @@ -307,6 +308,7 @@ _mesa_create_exec_table(void)
>   #if FEATURE_ARB_shader_objects
>      _mesa_init_shader_dispatch(exec);
>      _mesa_init_shader_uniform_dispatch(exec);
> +   _mesa_init_shader_uniform_buffer_objects(exec);
>   #endif
>
>      /* 2. GL_EXT_blend_color */
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index b7e59e8..d74b78c 100644
> --- a/src/mesa/main/bufferobj.c
> +++ b/src/mesa/main/bufferobj.c
> @@ -98,6 +98,8 @@ get_buffer_target(struct gl_context *ctx, GLenum target)
>            return&ctx->Texture.BufferObject;
>         }
>         break;
> +   case GL_UNIFORM_BUFFER:
> +      return&ctx->UniformBufferObject.UniformObj;
>      default:
>         return NULL;
>      }
> @@ -734,6 +736,10 @@ _mesa_BindBufferBase(GLenum target, GLuint index, GLuint buffer)
>
>            break;
>   #endif
> +#if FEATURE_ARB_uniform_buffer_object
> +      case GL_UNIFORM_BUFFER:
> +         break;
> +#endif
>         default:
>            _mesa_error(ctx, GL_INVALID_ENUM, "glBindBufferBase(target)");
>            break;
> @@ -756,6 +762,11 @@ _mesa_BindBufferBase(GLenum target, GLuint index, GLuint buffer)
>            _mesa_bind_buffer_range_for_transform_feedback(ctx, index, bufObj, 0, size);
>            break;
>   #endif
> +#if FEATURE_ARB_uniform_buffer_object
> +      case GL_UNIFORM_BUFFER:
> +         ctx->UniformBufferObject.BindingPoint[index] = bufObj;
> +         break;
> +#endif
>         default: /* should not go here */
>            break;
>      }
> diff --git a/src/mesa/main/ubo.cpp b/src/mesa/main/ubo.cpp
> new file mode 100644
> index 0000000..77f7edd
> --- /dev/null
> +++ b/src/mesa/main/ubo.cpp
> @@ -0,0 +1,263 @@
> +/*
> + * Mesa 3-D graphics library
> + *
> + * Copyright (C) 2004-2011  Brian Paul   All Rights Reserved.
> + * Copyright (C) 2009-2011  VMware, Inc.  All Rights Reserved.
> + * Copyright © 2010-2011 Intel Corporation
> + * Copyright (C) 2011 Vincent Lejeune.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included
> + * in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * BRIAN PAUL BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
> + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/**
> + * API implementation of GL_ARB_uniform_buffer_object.
> + *
> + * Every uniform (in an UBO or not) is given an index that identifies it when
> + * calling one of these functions. Strictly speaking, such an index doesn't
> + * need to match location for classic uniform ; others drivers however use this
> + * approach so we're also matching them for compatibility purpose.
> + */
> +
> +#include<stdint.h>
> +#include "main/glheader.h"
> +#include "main/context.h"
> +#include "main/dispatch.h"
> +#include "main/image.h"
> +#include "main/mfeatures.h"
> +#include "main/mtypes.h"
> +#include "main/shaderapi.h"
> +#include "main/shaderobj.h"
> +#include "main/uniforms.h"
> +#include "program/hash_table.h"
> +#include "program/prog_parameter.h"
> +#include "program/prog_statevars.h"
> +#include "program/prog_instruction.h"
> +#include "main/ubo.h"
> +#include "main/bufferobj.h"
> +
> +static void
> +get_ubo_info(struct gl_context *ctx,
> +             struct gl_shader_program *shProg, GLint ubo_index,
> +             GLenum query, int *data)
> +{
> +   if (ubo_index>  shProg->UBOCount || ubo_index<  0)
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glGetActiveUniformBlock(uniformBlockIndex)");
> +
> +   struct gl_uniform_buffer_object current_ubo =
> +      shProg->UniformBufferObject[ubo_index];

Declarations need to be placed before code to avoid problems compiling 
on MSVC.

Where is the gl_uniform_buffer_object type defined?  I don't see a 
patch to mtypes.h in this patchset.

Also, I think you should use a pointer there:

const struct gl_uniform_buffer_object *current_ubo =
    &shProg->UniformBufferObject[ubo_index];

Then use current_ubo->foo below.


> +   switch (query) {
> +   case GL_UNIFORM_BLOCK_BINDING:
> +      data = 0;                 // TODO : find a nice way to get buffer id
> +      break;
> +   case GL_UNIFORM_BLOCK_DATA_SIZE:
> +      *data = current_ubo.Size;
> +      break;
> +   case GL_UNIFORM_BLOCK_NAME_LENGTH:
> +      *data = strlen(current_ubo.Name) + 1;
> +      break;
> +   case GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS:
> +      *data = current_ubo.NumberOfVariables;
> +      break;
> +   case GL_UNIFORM_BLOCK_REFERENCED_BY_VERTEX_SHADER:
> +      *data = current_ubo.ReferencedByVS;
> +      break;
> +   case GL_UNIFORM_BLOCK_REFERENCED_BY_GEOMETRY_SHADER:
> +      *data = current_ubo.ReferencedByGS;
> +      break;
> +   case GL_UNIFORM_BLOCK_REFERENCED_BY_FRAGMENT_SHADER:
> +      *data = current_ubo.ReferencedByFS;
> +      break;
> +   default:

Should GL_INVALID_ENUM be raised here?


> +      break;
> +   }
> +   return;
> +}
> +
> +static GLuint
> +get_indice(const struct gl_shader_program *prog, const char *name)

get_index?

The function needs a comment explaining what it does too.


> +{
> +   unsigned index;
> +   if (prog->NamedAccessUBOVariables->get(index, name)) {
> +      return index;
> +   }
> +
> +   return GL_INVALID_INDEX;
> +}
> +
> +/**
> + * This function checks if it can copy source into dest.
> + * If it cannot, it returns 1, otherwise it returns 0 and
> + * write size to *length if non NULL
> + */
> +static int
> +copy_char_buffer(const char *source, char *dest, unsigned destsize,
> +                 int *length)
> +{
> +   unsigned sourcesize = strlen(source);
> +   if (sourcesize>  destsize + 1) {
> +      return -1;
> +   }
> +   memcpy(dest, source, sourcesize);
> +   dest[sourcesize] = '0';
> +   if (length)
> +      *length = sourcesize;
> +   return 0;
> +}

I think _mesa_copy_string() can be used instead of that function.


> +
> +static void
> +get_ubo_name(struct gl_context *ctx, struct gl_shader_program *shProg,
> +             GLint index, GLsizei bufsize, GLsizei * length, char *buffer)
> +{
> +   if (index>= shProg->UBOCount || index<  0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "GetActiveUniformBlockName(uniformBlockIndex)");
> +      return;
> +   }
> +
> +   struct gl_uniform_buffer_object current_ubo =
> +      shProg->UniformBufferObject[index];

Again, move declarations before code, use a const pointer, etc.


> +   if (copy_char_buffer(current_ubo.Name, buffer, bufsize, length)) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "GetActiveUniformBlockName(bufSize)");
> +      return;
> +   }
> +   return;
> +}
> +
> +static void
> +get_uniform_variable_name(struct gl_context *ctx,
> +                          struct gl_shader_program *sh, GLuint index,
> +                          GLsizei bufsize, GLsizei * length,
> +                          char *uniformName)
> +{
> +   const char *name = 0;        // sh->Uniforms->Uniforms[index].Name;

Is this unfinished code?


> +   if (copy_char_buffer(name, uniformName, bufsize, length)) {
> +      _mesa_error(ctx, GL_INVALID_VALUE, "GetActiveUniformName(bufSize)");
> +      return;
> +   }
> +   return;
> +}
> +
> +static GLint
> +get_uniform_variable_info(struct gl_shader_program *sh, GLuint index,
> +                          GLenum pname)
> +{
> +   uint starting_index = sh->NumUserUniformStorage;
> +   if (index>= starting_index) {
> +      struct gl_program_ubo_variable *var =
> +         sh->IndexedUBOVariables[index - starting_index];

Do we need a check that index - starting_index isn't too large?


> +      switch (pname) {
> +      case GL_UNIFORM_TYPE:
> +         return var->Type;
> +      case GL_UNIFORM_SIZE:
> +         return 1;
> +      case GL_UNIFORM_NAME_LENGTH:
> +         return strlen(var->Name) + 1;
> +      case GL_UNIFORM_BLOCK_INDEX:
> +         return var->UBO->Index;
> +      case GL_UNIFORM_OFFSET:
> +         return var->Offset;
> +      case GL_UNIFORM_ARRAY_STRIDE:
> +      case GL_UNIFORM_MATRIX_STRIDE:
> +         return var->Stride;
> +      case GL_UNIFORM_IS_ROW_MAJOR:
> +         return sh->UniformBufferObject[var->UBO->Index].MatrixLayout ==
> +            UBO_MATRIX_LAYOUT_ROW_MAJOR;

Do we need a default case that raises GL_INVALID_ENUM?


> +      }
> +   }
> +
> +   return GL_INVALID_VALUE;
> +}
> +
> +void GLAPIENTRY
> +_mesa_GetActiveUniformBlockInfo(GLuint program, GLuint ubo_index,
> +                                GLenum pname, GLint * params)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   struct gl_shader_program *shProg =
> +      _mesa_lookup_shader_program_err(ctx, program,
> +                                      "GetActiveUniformBlockiv");
> +   get_ubo_info(ctx, shProg, ubo_index, pname, params);
> +}
> +
> +void GLAPIENTRY
> +_mesa_GetActiveUniformBlockName(GLuint program, GLuint ubo_index,
> +                                GLsizei bufsize, GLint * length, char *name)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   struct gl_shader_program *shProg =
> +      _mesa_lookup_shader_program_err(ctx, program,
> +                                      "GetActiveUniformBlockName");
> +   get_ubo_name(ctx, shProg, ubo_index, bufsize, length, name);
> +}
> +
> +void GLAPIENTRY
> +_mesa_GetUniformIndices(GLuint program, GLsizei number_of_variables,
> +                        const char **names, GLuint * indices)
> +{
> +   unsigned i;
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   struct gl_shader_program *shProg =
> +      _mesa_lookup_shader_program_err(ctx, program, "GetUniformIndices");
> +   for (i = 0; i<  number_of_variables; i++) {
> +      indices[i] = get_indice(shProg, names[i]);
> +   }
> +}
> +
> +void GLAPIENTRY
> +_mesa_GetActiveUniformName(GLuint program, GLuint index, GLsizei bufsize,
> +                           GLsizei * length, char *uniformName)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   struct gl_shader_program *shProg =
> +      _mesa_lookup_shader_program_err(ctx, program, "GetActiveUniformName");
> +   get_uniform_variable_name(ctx, shProg, index, bufsize, length,
> +                             uniformName);
> +
> +}
> +
> +void GLAPIENTRY
> +_mesa_GetActiveUniformsiv(GLuint program, GLsizei uniformCount,
> +                          const GLuint * uniformIndices, GLenum pname,
> +                          GLint * param)
> +{
> +   unsigned k;
> +   GET_CURRENT_CONTEXT(ctx);
> +   struct gl_shader_program *shProg =
> +      _mesa_lookup_shader_program_err(ctx, program, "GetActiveUniformsiv");
> +
> +
> +   for (k = 0; k<  uniformCount; k++) {
> +      param[k] = get_uniform_variable_info(shProg, uniformIndices[k], pname);
> +   }
> +}
> +
> +void
> +_mesa_init_shader_uniform_buffer_objects(struct _glapi_table *exec)
> +{
> +/* GL_ARB_Uniform_Buffer_Object */
> +   SET_GetActiveUniformBlockiv(exec, _mesa_GetActiveUniformBlockInfo);
> +   SET_GetActiveUniformBlockName(exec, _mesa_GetActiveUniformBlockName);
> +   SET_GetUniformIndices(exec, _mesa_GetUniformIndices);
> +   SET_GetActiveUniformName(exec, _mesa_GetActiveUniformName);
> +   SET_GetActiveUniformsiv(exec, _mesa_GetActiveUniformsiv);
> +}
> diff --git a/src/mesa/main/ubo.h b/src/mesa/main/ubo.h
> new file mode 100644
> index 0000000..32ea8f9
> --- /dev/null
> +++ b/src/mesa/main/ubo.h
> @@ -0,0 +1,33 @@
> +#ifndef UBO_H
> +#define UBO_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +
> +#include "glheader.h"
> +struct _glapi_table;
> +
> +void GLAPIENTRY
> +_mesa_GetActiveUniformBlockInfo (GLuint, GLuint, GLenum, GLint *);
> +
> +extern void GLAPIENTRY
> +_mesa_GetActiveUniformBlockName (GLuint, GLuint, GLsizei, GLint *, char *);
> +
> +extern void GLAPIENTRY
> +_mesa_GetUniformIndices (GLuint, GLsizei, const char **, GLuint *);
> +
> +extern void GLAPIENTRY
> +_mesa_GetActiveUniformName (GLuint, GLuint, GLsizei, GLsizei *, char *);
> +
> +void GLAPIENTRY
> +_mesa_GetActiveUniformsiv (GLuint, GLsizei, const GLuint *, GLenum, GLint *);
> +
> +void _mesa_init_shader_uniform_buffer_objects (struct _glapi_table *exec);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif // UBO_H
> diff --git a/src/mesa/sources.mak b/src/mesa/sources.mak
> index e72a1ce..c87b4be 100644
> --- a/src/mesa/sources.mak
> +++ b/src/mesa/sources.mak
> @@ -106,6 +106,7 @@ MAIN_SOURCES = \
>   MAIN_CXX_SOURCES = \
>   	main/ff_fragment_shader.cpp \
>   	main/shader_query.cpp \
> +	main/ubo.cpp \
>   	main/uniform_query.cpp
>
>   MATH_SOURCES = \
> diff --git a/src/mesa/state_tracker/st_cb_bufferobjects.c b/src/mesa/state_tracker/st_cb_bufferobjects.c
> index adac92f..c8ee6d7 100644
> --- a/src/mesa/state_tracker/st_cb_bufferobjects.c
> +++ b/src/mesa/state_tracker/st_cb_bufferobjects.c
> @@ -128,7 +128,8 @@ st_bufferobj_subdata(struct gl_context *ctx,
>       */
>      pipe_buffer_write(st_context(ctx)->pipe,
>   		     st_obj->buffer,
> -		     offset, size, data);
> +                 offset, size, data);
> +

Let's leave whitespace changes out of this patch.

>   }
>
>
> @@ -195,6 +196,9 @@ st_bufferobj_data(struct gl_context *ctx,
>      case GL_ELEMENT_ARRAY_BUFFER_ARB:
>         bind = PIPE_BIND_INDEX_BUFFER;
>         break;
> +   case GL_UNIFORM_BUFFER:
> +      bind = PIPE_BIND_CONSTANT_BUFFER;
> +      break;
>      default:
>         bind = 0;
>      }



More information about the mesa-dev mailing list