On 12 December 2013 08:29, Francisco Jerez <curroje...@riseup.net> wrote:
> Paul Berry <stereotype...@gmail.com> writes:
>
> > On 26 November 2013 00:02, Francisco Jerez <curroje...@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 <stereotype...@gmail.com>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
