[Mesa-dev] [PATCH 03/11] mesa: Add state data structures requried for ARB_shader_image_load_store.

Paul Berry stereotype441 at gmail.com
Fri Dec 6 08:14:00 PST 2013


On 24 November 2013 21:00, Francisco Jerez <currojerez at riseup.net> wrote:

> ---
>  src/mesa/main/config.h |   1 +
>  src/mesa/main/dd.h     |   1 +
>  src/mesa/main/mtypes.h | 100
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  src/mesa/main/texobj.c |   1 +
>  4 files changed, 103 insertions(+)
>
> diff --git a/src/mesa/main/config.h b/src/mesa/main/config.h
> index 22bbfa0..8bd9765 100644
> --- a/src/mesa/main/config.h
> +++ b/src/mesa/main/config.h
> @@ -175,6 +175,7 @@
>  #define MAX_COMBINED_ATOMIC_BUFFERS    (MAX_UNIFORM_BUFFERS * 6)
>  /* Size of an atomic counter in bytes according to
> ARB_shader_atomic_counters */
>  #define ATOMIC_COUNTER_SIZE            4
> +#define MAX_IMAGE_UNITS                32
>

It would be nice to have a note in the commit message explaining why you
chose 32 for MAX_IMAGE_UNITS (the spec only requires 8 so it's not obvious).


>  /*@}*/
>
>  /**
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index b5b874f..648062f 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -39,6 +39,7 @@ struct gl_buffer_object;
>  struct gl_context;
>  struct gl_display_list;
>  struct gl_framebuffer;
> +struct gl_image_unit;
>  struct gl_pixelstore_attrib;
>  struct gl_program;
>  struct gl_renderbuffer;
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index e9750f4..7be0664 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1207,6 +1207,9 @@ struct gl_texture_object
>
>     /** GL_OES_EGL_image_external */
>     GLint RequiredTextureImageUnits;
> +
> +   /** GL_ARB_shader_image_load_store */
> +   GLenum ImageFormatCompatibility;
>

This is called IMAGE_FORMAT_COMPATIBILITY_TYPE in the GL spec.  Can we
rename it to ImageFormatCompatibilityType for consistency?


>  };
>
>
> @@ -2373,6 +2376,29 @@ struct gl_shader
>          */
>        GLenum OutputType;
>     } Geom;
> +
> +   /**
> +    * Map from image uniform index to image unit (set by glUniform1i())
> +    *
> +    * An image uniform index is associated with each image uniform by
> +    * the linker.  The image index associated with each uniform is
> +    * stored in the \c gl_uniform_storage::image field.
>

Can we assume image uniform indices are always in the range [0,
NumImages-1]?  If so it would be nice to document that here.


> +    */
> +   GLubyte ImageUnits[MAX_IMAGE_UNITS];
> +
> +   /**
> +    * Access qualifier specified in the shader for each image uniform.
> +    * Either \c GL_READ_ONLY, \c GL_WRITE_ONLY or \c GL_READ_WRITE.
> +    *
> +    * It may be different, though only more strict than the value of
> +    * \c gl_image_unit::Access for the corresponding image unit.
>

Is this array indexed by image uniform index or by image unit?  The comment
should clarify that.


> +    */
> +   GLenum ImageAccess[MAX_IMAGE_UNITS];
>

With those changes, this patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131206/4a34bdf5/attachment-0001.html>


More information about the mesa-dev mailing list