[Mesa-dev] [PATCH 3/6] glsl: move half<->float convertion to util

[Rob Clark]

On Mon, Oct 12, 2015 at 2:22 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Mon, Oct 12, 2015 at 11:12 AM, Rob Clark <robdclark at gmail.com> wrote:
>> On Mon, Oct 12, 2015 at 12:47 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>>>> > +/**
>>>> > + * Convert a 2-byte half float to a 4-byte float.
>>>> > + * Based on code from:
>>>> > + * http://www.opengl.org/discussion_boards/ubb/Forum3/HTML/008786.html
>>>> > + */
>>>> > +static inline float
>>>> > +_mesa_half_to_float(GLhalfARB val)
>>>> > +{
>>>> > +   return half_to_float(val);
>>>> > +}
>>>
>>> This is kind of ugly. How hard would it really be to just replace the uses
>>> with the new name?  I don't think its used *that* often.
>>
>> hmm, ~20-30 call sites.. not impossible to update them all, but I
>> think it should be a separate patch and then drop the compat shims
>> rather than one big patch that changes everything..
>>
>> We could also keep the _mesa_half_to_float() name.. but I really
>> wanted to drop the GLhalfARB and not drag along GL typedefs.  If no
>> one objects to just replacing GLhalfARB with uint16_t.
>>
>> I could go either way.
>
> Replacing GLhalfARB with uint16_t seems like a good plan.

ok, then the most straightforward (least churn) approach seems to be
to keep the _mesa_ prefix but use uint16_t..

>>>> > +
>>>> > +#include <stdint.h>
>>>> > +
>>>> > +#ifdef __cplusplus
>>>> > +extern "C" {
>>>> > +#endif
>>>> > +
>>>> > +uint16_t float_to_half(float val);
>>>> > +float half_to_float(uint16_t val);
>>>>
>>>> I think these functions need to be prefixed with something -- util_*
>>>> or something or just leave them as _mesa_*.
>>>
>>> Unfortunately, until is kind of a grab-bag right now.  Some stuff has a
>>> util_ prefix, some has kept its _mesa_ or u_ prefix depending on whether it
>>> was copied from Mesa or gallium, and some (hash_table for example) isn't
>>> prefixed at all.  Personally, I'd go with either util_ (it is a utility
>>> library) or just keep _mesa_ (this is still the mesa project after all). I'm
>>> not going to be too insistent though
>>
>> the util_ prefix conflicts w/ u_half.h (which appears to implement
>> basically the same thing in a simpler way, but maybe not compatible
>> enough to just switch to the gallium implementation?  Otherwise the
>> easier approach would be to just move the gallium implementation to
>> global util directory and use that instead)..  I was going to use
>> convert_ prefix but that also conflicts..
>
> Seems valuable to ultimately remove the Gallium implementation as
> well. Chad did some really tedious work to improve the functions
> you're moving to round properly.

hmm, looks like his change was mostly important for compiler (to match
what intel hw does).. otoh I guess it shouldn't hurt for gallum (which
looks to be used mostly for texture/format conversion) and would be
nice to de-duplicate..