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

[Francisco Jerez]

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.

> - 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 --------------
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/20131218/5450da0b/attachment.pgp>