Chris Forbes <chr...@ijw.co.nz> writes: > Sounds pretty reasonable. > > Is this a sensible direction for samplers to go in eventually as well? > That's my suspicion... It will definitely make variable sampler array indexing for ARB_gpu_shader5 easier.
> -- Chris
>
> On Wed, Nov 27, 2013 at 7:51 AM, Francisco Jerez <curroje...@riseup.net>
> wrote:
>> Hi Chris,
>>
>> Chris Forbes <chr...@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 <curroje...@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@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
pgpnhjO4DL830.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
