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 <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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev