[Mesa-dev] [PATCH 02/23] glsl: Add image type to the GLSL IR.
Francisco Jerez
currojerez at riseup.net
Tue Nov 26 11:26:53 PST 2013
Chris Forbes <chrisf at ijw.co.nz> writes:
> Do you have a suite of piglit tests for this extension?
>
Nope, not yet.
> On Wed, Nov 27, 2013 at 8:01 AM, Chris Forbes <chrisf at ijw.co.nz> wrote:
>> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131126/e0e5d420/attachment.pgp>
More information about the mesa-dev
mailing list