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

[Paul Berry]

On 12 December 2013 08:29, Francisco Jerez <currojerez at riseup.net> wrote:

> Paul Berry <stereotype441 at gmail.com> writes:
>
> > On 26 November 2013 00:02, Francisco Jerez <currojerez at riseup.net>
> wrote:
> >
> >>
> >> +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
> >> +};
> >> +
> >>
> >
> > glsl_image_dim seems redundant  with glsl_sampler_dim (they are identical
> > except that the order is different, and glsl_sampler_dim includes the
> extra
> > value GLSL_SAMPLER_DIM_EXTERNAL).  I'd prefer to just use one enum for
> both
> > purposes, and don't create any image types using
> GLSL_SAMPLER_DIM_EXTERNAL.
> >
>
> This was a deliberate change.  My intention was to be able to refer to a
> surface dimensionality and whether it's an image or sampler type with a
> single identifier.  If we use the same type for both we will have to
> find another way for overload selection and that will probably make the
> notation in the builtin_type_macros.h code in PATCH 16 inconsistent with
> the macros for sampler types, unless we change the sampler glsl_type
> constructor too.
>

It looks to me like the only function where you're taking advantage of the
different enum types is to distinguish between the glsl_type constructor
for samplers and images.  But the only reason that's necessary is because
of a lot of other code duplication.  It seems to me that we ought to be
able to coalesce:

glsl_type::sampler_dimensionality and glsl_type::fields.image.dimension
glsl_type::sampler_array and glsl_type::fields.image.array
glsl_type::sampler_type and glsl_type::fields.image.type

and then all we need to do is add a base_type argument to the existing
sampler type constructor, and we can reuse it for image types.


>
> > With that changed, 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/20131216/2497f2c6/attachment.html>