[Mesa-dev] [PATCH v3] r600g: Fix special negative immediate constants when using ABS modifier.

[Ivan Kalvachev]

On 10/29/15, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Wed, Oct 28, 2015 at 8:52 PM, Ivan Kalvachev <ikalvachev at gmail.com>
> wrote:
>> I'm attaching v3 of the patch. Same as v2, but without the extra empty
>> line.
>
> FYI, there's a lot of overhead to reviewing an attached patch (in
> fact, gmail makes it extra difficult since you can't even see the
> thing in the name of protection from the unknown). Please use 'git
> send-email' (or equivalent mechanism) to make the patches appear
> inline.
>
> Cheers,
>
>   -ilia

Sorry about that.
I did inline my v1 patch, to easy the review.

Configuring smtp server is too much hassle for a single patch and I
would like to avoid writing my email credentials if possible.
If I'm about to send more patches, then I guess I would have to do that.

I suspect that it is not possible to send mail directly to
lists.freedesktop.org mail server, is it?


On 10/29/15, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 29.10.2015 01:52, Ivan Kalvachev wrote:
>> ---------- Forwarded message ----------
>> From: Ivan Kalvachev <ikalvachev at gmail.com>
>> Date: Wed, 28 Oct 2015 23:46:44 +0200
>> Subject: [PATCH v3] r600g: Fix special negative immediate constants
>> when using ABS modifier.
>> To: Nicolai Hähnle <nhaehnle at gmail.com>
>>
>> On 10/26/15, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>>> Hi Ivan,
>>>
>>> On 25.10.2015 02:00, Ivan Kalvachev wrote:
>>>> Some constants (like 1.0 and 0.5) could be inlined as immediate inputs
>>>> without using their literal value. The
>>>> r600_bytecode_special_constants()
>>>> function emulates the negative of these constants by using NEG
>>>> modifier.
>>>>
>>>> However some shaders define -1.0 constant and want to use it as 1.0.
>>>> They do so by using ABS modifier. But r600_bytecode_special_constants()
>>>> set NEG in addition to ABS. Since NEG modifier have priority over ABS
>>>> one,
>>>> we get -|1.0| as result, instead of |1.0|.
>>>>
>>>> The patch simply prevents the additional switching of NEG when ABS is
>>>> set.
>>>
>>> Nice catch. Is there a simple test case (e.g. in piglit) that exposes
>>> the incorrect behavior?
>>
>> Not that I know of.
>>
>> I've located the bug investigating visual problem in Nine.
>> https://github.com/iXit/Mesa-3D/issues/126
>> https://github.com/iXit/Mesa-3D/issues/127
>>
>> I also heard that it fixes artifacts in "Need for Speed: Undercover"
>> and "Skyrim", once again, when using Nine.
>
> I see. I guess it's not too surprising that Nine creates shaders that
> look a bit different from the Mesa statetracker's.
>
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> This should probably also go to stable.
>
> Do you need somebody to push this for you or can you do it yourself?
>
> Cheers,
> Nicolai

Yes, please.
I'm not developer and I cannot push it myself.