[Mesa-dev] [PATCH 02/23] glsl: Add image type to the GLSL IR.

Chris Forbes chrisf at ijw.co.nz
Tue Nov 26 11:01:04 PST 2013


Sounds pretty reasonable.

Is this a sensible direction for samplers to go in eventually as well?

-- Chris

On Wed, Nov 27, 2013 at 7:51 AM, Francisco Jerez <currojerez at riseup.net> wrote:
> Hi Chris,
>
> Chris Forbes <chrisf at ijw.co.nz> writes:
>
>> @@ -618,6 +637,9 @@ glsl_type::component_slots() const
>>     case GLSL_TYPE_ARRAY:
>>        return this->length * this->fields.array->component_
>> slots();
>>
>> +   case GLSL_TYPE_IMAGE:
>> +      return 1;
>>
>> Why is an image type one component, whereas the other opaque types are
>> zero at this level?
>>
>
> It seemed to simplify things to treat image variables as any other
> integer scalar rather than pretending that they have no storage here.
>
> Representing them as indices stored in an unsigned integer will make
> things like dynamic array indexing, structure member selection and
> non-inlined function calls [if we ever support that] trivial.  It also
> matches the way the GL API is designed: Images [unlike atomic counters]
> are assigned a one-component location in the default uniform block, and
> they can be set using glUniform1i().
>
> Note that this change is orthogonal to image uniforms being accounted in
> the MAX_*_UNIFORM_COMPONENTS calculation or not.  I haven't found any
> clear reference to that in the spec, other than:
>
> [from the GL 4.4 specification, section 7.6]
>
> | The implementation-dependent amount of storage available for uniform
> | variables, except for subroutine uniforms and atomic counters, in the
> | default uniform block accessed by a shader for a particular shader
> | stage can be queried by calling GetIntegerv with pname as specified in
> | table 7.4 for that stage.
>
> [table 7.4 just lists the MAX_*_UNIFORM_COMPONENTS queries]
>
> | [...] For uniforms with boolean, integer, or floating-point
> | components, [...] a scalar uniform will consume no more than 1
> | component.
>
> [from section 7.11]
>
> | [...] The value of an image uniform is an integer specifying the image
> | unit accessed.
>
> So it seems to me that counting them against MAX_*_UNIFORM_COMPONENTS
> matches the spec wording and it might be necessary for hardware that
> keeps this representation of images as integer scalars internally and
> has a limited amount of uniform storage in the default block.
>
> Thanks.
>
>>
>>
>> On Tue, Nov 26, 2013 at 9:02 PM, Francisco Jerez <currojerez at riseup.net> wrote:
>>> ---
>>>  src/glsl/ast_to_hir.cpp                      |  1 +
>>>  src/glsl/glsl_types.cpp                      | 23 +++++++++++++++++++++++
>>>  src/glsl/glsl_types.h                        | 22 ++++++++++++++++++++++
>>>  src/glsl/ir_clone.cpp                        |  1 +
>>>  src/glsl/link_uniform_initializers.cpp       |  1 +
>>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |  1 +
>>>  src/mesa/program/ir_to_mesa.cpp              |  2 ++
>>>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp   |  1 +
>>>  8 files changed, 52 insertions(+)
>>>
>>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>>> index 43cf497..e7c4ff4 100644
>>> --- a/src/glsl/ast_to_hir.cpp
>>> +++ b/src/glsl/ast_to_hir.cpp
>>> @@ -948,6 +948,7 @@ do_comparison(void *mem_ctx, int operation, ir_rvalue *op0, ir_rvalue *op1)
>>>     case GLSL_TYPE_ERROR:
>>>     case GLSL_TYPE_VOID:
>>>     case GLSL_TYPE_SAMPLER:
>>> +   case GLSL_TYPE_IMAGE:
>>>     case GLSL_TYPE_INTERFACE:
>>>     case GLSL_TYPE_ATOMIC_UINT:
>>>        /* I assume a comparison of a struct containing a sampler just
>>> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
>>> index f740130..e24417d 100644
>>> --- a/src/glsl/glsl_types.cpp
>>> +++ b/src/glsl/glsl_types.cpp
>>> @@ -80,6 +80,24 @@ glsl_type::glsl_type(GLenum gl_type,
>>>     memset(& fields, 0, sizeof(fields));
>>>  }
>>>
>>> +glsl_type::glsl_type(GLenum gl_type,
>>> +                    enum glsl_image_dim dim, bool array,
>>> +                    glsl_base_type type, const char *name) :
>>> +   gl_type(gl_type),
>>> +   base_type(GLSL_TYPE_IMAGE),
>>> +   sampler_dimensionality(0), sampler_shadow(0),
>>> +   sampler_array(0), sampler_type(0), interface_packing(0),
>>> +   vector_elements(1), matrix_columns(1),
>>> +   length(0), fields()
>>> +{
>>> +   init_ralloc_type_ctx();
>>> +   assert(name != NULL);
>>> +   this->name = ralloc_strdup(this->mem_ctx, name);
>>> +   fields.image.type = type;
>>> +   fields.image.dimension = dim;
>>> +   fields.image.array = array;
>>> +}
>>> +
>>>  glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
>>>                      const char *name) :
>>>     gl_type(0),
>>> @@ -172,6 +190,7 @@ bool
>>>  glsl_type::contains_opaque() const {
>>>     switch (base_type) {
>>>     case GLSL_TYPE_SAMPLER:
>>> +   case GLSL_TYPE_IMAGE:
>>>     case GLSL_TYPE_ATOMIC_UINT:
>>>        return true;
>>>     case GLSL_TYPE_ARRAY:
>>> @@ -618,6 +637,9 @@ glsl_type::component_slots() const
>>>     case GLSL_TYPE_ARRAY:
>>>        return this->length * this->fields.array->component_slots();
>>>
>>> +   case GLSL_TYPE_IMAGE:
>>> +      return 1;
>>> +
>>>     case GLSL_TYPE_SAMPLER:
>>>     case GLSL_TYPE_ATOMIC_UINT:
>>>     case GLSL_TYPE_VOID:
>>> @@ -908,6 +930,7 @@ glsl_type::count_attribute_slots() const
>>>        return this->length * this->fields.array->count_attribute_slots();
>>>
>>>     case GLSL_TYPE_SAMPLER:
>>> +   case GLSL_TYPE_IMAGE:
>>>     case GLSL_TYPE_ATOMIC_UINT:
>>>     case GLSL_TYPE_VOID:
>>>     case GLSL_TYPE_ERROR:
>>> diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
>>> index 96eee5e..ea65bcd 100644
>>> --- a/src/glsl/glsl_types.h
>>> +++ b/src/glsl/glsl_types.h
>>> @@ -53,6 +53,7 @@ enum glsl_base_type {
>>>     GLSL_TYPE_FLOAT,
>>>     GLSL_TYPE_BOOL,
>>>     GLSL_TYPE_SAMPLER,
>>> +   GLSL_TYPE_IMAGE,
>>>     GLSL_TYPE_ATOMIC_UINT,
>>>     GLSL_TYPE_STRUCT,
>>>     GLSL_TYPE_INTERFACE,
>>> @@ -72,6 +73,16 @@ enum glsl_sampler_dim {
>>>     GLSL_SAMPLER_DIM_MS
>>>  };
>>>
>>> +enum glsl_image_dim {
>>> +   GLSL_IMAGE_DIM_1D,
>>> +   GLSL_IMAGE_DIM_2D,
>>> +   GLSL_IMAGE_DIM_3D,
>>> +   GLSL_IMAGE_DIM_RECT,
>>> +   GLSL_IMAGE_DIM_CUBE,
>>> +   GLSL_IMAGE_DIM_BUFFER,
>>> +   GLSL_IMAGE_DIM_MS
>>> +};
>>> +
>>>  enum glsl_interface_packing {
>>>     GLSL_INTERFACE_PACKING_STD140,
>>>     GLSL_INTERFACE_PACKING_SHARED,
>>> @@ -152,6 +163,12 @@ struct glsl_type {
>>>        const struct glsl_type *array;            /**< Type of array elements. */
>>>        const struct glsl_type *parameters;       /**< Parameters to function. */
>>>        struct glsl_struct_field *structure;      /**< List of struct fields. */
>>> +
>>> +      struct {
>>> +         glsl_base_type type; /**< Image data type as seen by the shader. */
>>> +         glsl_image_dim dimension; /**< Base dimensionality of this image. */
>>> +         bool array; /**< True if this is an array image type. */
>>> +      } image;
>>>     } fields;
>>>
>>>     /**
>>> @@ -562,6 +579,11 @@ private:
>>>              enum glsl_sampler_dim dim, bool shadow, bool array,
>>>              unsigned type, const char *name);
>>>
>>> +   /** Constructor for image types */
>>> +   glsl_type(GLenum gl_type,
>>> +            enum glsl_image_dim dim, bool array,
>>> +            glsl_base_type type, const char *name);
>>> +
>>>     /** Constructor for record types */
>>>     glsl_type(const glsl_struct_field *fields, unsigned num_fields,
>>>              const char *name);
>>> diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp
>>> index 40ed33a..ed26aae 100644
>>> --- a/src/glsl/ir_clone.cpp
>>> +++ b/src/glsl/ir_clone.cpp
>>> @@ -398,6 +398,7 @@ ir_constant::clone(void *mem_ctx, struct hash_table *ht) const
>>>     }
>>>
>>>     case GLSL_TYPE_SAMPLER:
>>> +   case GLSL_TYPE_IMAGE:
>>>     case GLSL_TYPE_ATOMIC_UINT:
>>>     case GLSL_TYPE_VOID:
>>>     case GLSL_TYPE_ERROR:
>>> diff --git a/src/glsl/link_uniform_initializers.cpp b/src/glsl/link_uniform_initializers.cpp
>>> index 786aaf0..f02978c 100644
>>> --- a/src/glsl/link_uniform_initializers.cpp
>>> +++ b/src/glsl/link_uniform_initializers.cpp
>>> @@ -69,6 +69,7 @@ copy_constant_to_storage(union gl_constant_value *storage,
>>>          break;
>>>        case GLSL_TYPE_ARRAY:
>>>        case GLSL_TYPE_STRUCT:
>>> +      case GLSL_TYPE_IMAGE:
>>>        case GLSL_TYPE_ATOMIC_UINT:
>>>        case GLSL_TYPE_INTERFACE:
>>>        case GLSL_TYPE_VOID:
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>> index 9eb9a9d..3b08f6f 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>> @@ -820,6 +820,7 @@ fs_visitor::emit_assignment_writes(fs_reg &l, fs_reg &r,
>>>        break;
>>>
>>>     case GLSL_TYPE_SAMPLER:
>>> +   case GLSL_TYPE_IMAGE:
>>>     case GLSL_TYPE_ATOMIC_UINT:
>>>        break;
>>>
>>> diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
>>> index c833a12..17490cd 100644
>>> --- a/src/mesa/program/ir_to_mesa.cpp
>>> +++ b/src/mesa/program/ir_to_mesa.cpp
>>> @@ -618,6 +618,7 @@ type_size(const struct glsl_type *type)
>>>        }
>>>        return size;
>>>     case GLSL_TYPE_SAMPLER:
>>> +   case GLSL_TYPE_IMAGE:
>>>        /* Samplers take up one slot in UNIFORMS[], but they're baked in
>>>         * at link time.
>>>         */
>>> @@ -2599,6 +2600,7 @@ _mesa_associate_uniform_storage(struct gl_context *ctx,
>>>             columns = 1;
>>>             break;
>>>          case GLSL_TYPE_SAMPLER:
>>> +        case GLSL_TYPE_IMAGE:
>>>             format = uniform_native;
>>>             columns = 1;
>>>             break;
>>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>> index ac95968..08d5807 100644
>>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>> @@ -983,6 +983,7 @@ type_size(const struct glsl_type *type)
>>>        }
>>>        return size;
>>>     case GLSL_TYPE_SAMPLER:
>>> +   case GLSL_TYPE_IMAGE:
>>>        /* Samplers take up one slot in UNIFORMS[], but they're baked in
>>>         * at link time.
>>>         */
>>> --
>>> 1.8.3.4
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list