[Mesa-dev] [PATCH v3 19/23] glsl: Add image built-in function generator.

[Paul Berry]

On 18 December 2013 07:14, Francisco Jerez <currojerez at riseup.net> wrote:

> Paul Berry <stereotype441 at gmail.com> writes:
> >[...]
> > This is definitely an improvement, thanks.  I still think there are some
> > minor tweaks that would reduce duplication of information, though:
> >
> > - Precompute the value (glsl ? IMAGE_FUNCTION_EMIT_STUB : 0) rather than
> > duplicating it at each add_image_function call site.  Alternatively, just
> > make emit_stub a bool parameter to add_image_function rather than
> including
> > it in flags.
> >
>
> Okay.
>
> > - Rather than repeating the pattern add_image_function((glsl ?
> BUILTIN_NAME
> > : INTRINSIC_NAME), INTRINSIC_NAME, ...) everywhere, make BUILTIN_NAME and
> > INTRINSIC_NAME the first two arguments to add_image_function(), and have
> > add_image_function() pick which name to use based on emit_stub.
> >
>
> That would make the interpretation of its arguments somewhat more
> obscure.  Even if the code would be slightly shorter that way, I think
> it's easier to understand what's going on the way it is.  I'd rather
> leave this unchanged.
>

I disagree, but I don't want to waste time arguing about this anymore.
Patch is:

Acked-by: Paul Berry <stereotype441 at gmail.com>


>
> > - Since IMAGE_FUNCTION_HAS_RETURN is by far the more common case, invert
> > the sense of the flag (e.g. change it to IMAGE_FUNCTION_RETURNS_VOID).
> >
>
> See
> http://cgit.freedesktop.org/~currojerez/mesa/commit/?h=image-load-store&id=9026d1d9b645ee7dd1fa48bdcdfff2891a7efbfc
>
> Thanks.
>
> > [...]
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131218/11b3d2cd/attachment-0001.html>