[Mesa-dev] [PATCH] glsl: always do sqrt(abs()) and inversesqrt(abs())

[Samuel Pitoiset]

On 01/11/2017 05:32 PM, Marek Olšák wrote:
> On Wed, Jan 11, 2017 at 4:33 PM, Erik Faye-Lund <kusmabite at gmail.com> wrote:
>> On Wed, Jan 11, 2017 at 4:14 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>>> On 11.01.2017 13:17, Marek Olšák wrote:
>>>>
>>>> On Tue, Jan 10, 2017 at 6:48 PM, Jason Ekstrand <jason at jlekstrand.net>
>>>> wrote:
>>>>>
>>>>> I'll be honest, I'm not a fan... Given that D3D10 has one defined
>>>>> behavior,
>>>>> D3D9 has another, and GL doesn't specify, I don't really think we should
>>>>> be
>>>>> making a global change to all drivers to do the D3D9 behavior just to fix
>>>>> one app.  Sure, other apps probably have the same bug, but are we going
>>>>> to
>>>>> have apps that expect the D3D10 behavior that we've now explicitly made
>>>>> not
>>>>> work?
>>>>>
>>>>> If we're going to hack around an app bug, I would really rather see it
>>>>> behind a driconf option rather than a global change to driver behavior.
>>>>> Even better, it'd be cool if we could see the app get fixed. (Yes, I know
>>>>> that's not likely).
>>>>
>>>>
>>>> I think we are not in a position to refuse this workaround, or put
>>>> more precisely, to have a different behavior from everybody else. By
>>>> "we", I mean i965, radeonsi, svga. All closed drivers use abs. Many
>>>> Mesa drivers also use abs internally (r300, r600, nv30, nv50/nvc0).
>>>> This is not really a workaround for a specific application, even
>>>> though it's strongly motivated by that. It's a fix to align the few
>>>> remaining drivers with all others.
>>>>
>>>> We talked with the publisher about this a very long time ago. While I
>>>> don't remember the details (Nicolai?), I think they refused to fix it
>>>> because radeonsi appeared to be the only driver not doing abs.
>>>
>>>
>>> If I remember correctly, it wasn't so much a refusal as a lack of
>>> follow-through. They even had an option in their framework to add the
>>> abs(...) when translating shaders, but somehow didn't turn it on
>>> unconditionally for some reason...
>>
>> VP even says so here:
>> https://github.com/virtual-programming/specops-linux/issues/20
>>
>> They recommend against patching mesa to do abs, though.
>
> We should still patch Mesa to align the behavior with closed drivers
> and gallium drivers like r600g and nouveau. In other words, it's too
> late to tell us not to patch Mesa, because r600g and nouveau have been
> "patched" since the beginning.
>
> We only need to decide whether we should do it in the GLSL compiler or
> radeonsi, i.e. whether we should exclude i965 and svga.

I do agree with that.

>
> Marek
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>