[Mesa-dev] [PATCH 2/2] r600g: use SIMPLE_FLOAT for blending to avoid NaNs in RTs

[Ilia Mirkin]

On Mon, Nov 6, 2017 at 6:21 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 06.11.2017 05:22, Ilia Mirkin wrote:
>>
>> Radeonsi also sets this flag.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103544
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>
>> This needs testing with the fbo-float-nan piglit that was recently added.
>> Just
>> guessing that this is the right flag to set here.
>
>
> Assuming that the test passes:

Well, the test can't pass or fail. The behavior is undefined. But Dave
ran it last night to see what would happen. It appears that with the
current code, if you have a float RB with a infinity or nan in it, and
then you set the dst blend factor to GL_ZERO, then you'll still end up
with a NaN in the result.

With this change, you'll end up with what most people would expect
with a GL_ZERO dst blend factor. Unless your src blend factor is e.g.
GL_DST_ALPHA, the dst op is multiplied. So if e.g. the RB has
(0,0,0,infinity), and shader output is (0,0,0,infinity), then src *
GL_DST_ALPHA + dst * GL_ZERO = (nan, nan, nan, nan). However src *
GL_SRC_ALPHA + dst + GL_ZERO = (0, 0, 0, infinity).

Perhaps there's another flag which controls 0 * inf = 0 vs NaN thing
for blending, but it wasn't immediately apparent from the docs. NVIDIA
blob drivers configure the hw for 0 * inf = 0.

>
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

Let me know if this stands, given the above info.

>
>
>
>>
>>   src/gallium/drivers/r600/evergreen_state.c | 1 +
>>   src/gallium/drivers/r600/r600_state.c      | 1 +
>>   2 files changed, 2 insertions(+)
>>
>> diff --git a/src/gallium/drivers/r600/evergreen_state.c
>> b/src/gallium/drivers/r600/evergreen_state.c
>> index 96eb35a9818..131778dea9f 100644
>> --- a/src/gallium/drivers/r600/evergreen_state.c
>> +++ b/src/gallium/drivers/r600/evergreen_state.c
>> @@ -1211,6 +1211,7 @@ static void
>> evergreen_set_color_surface_common(struct r600_context *rctx,
>>                 S_028C70_COMP_SWAP(swap) |
>>                 S_028C70_BLEND_CLAMP(blend_clamp) |
>>                 S_028C70_BLEND_BYPASS(blend_bypass) |
>> +               S_028C70_SIMPLE_FLOAT(1) |
>>                 S_028C70_NUMBER_TYPE(ntype) |
>>                 S_028C70_ENDIAN(endian);
>>   diff --git a/src/gallium/drivers/r600/r600_state.c
>> b/src/gallium/drivers/r600/r600_state.c
>> index c21e8dabb1f..0c331537460 100644
>> --- a/src/gallium/drivers/r600/r600_state.c
>> +++ b/src/gallium/drivers/r600/r600_state.c
>> @@ -898,6 +898,7 @@ static void r600_init_color_surface(struct
>> r600_context *rctx,
>>                 S_0280A0_COMP_SWAP(swap) |
>>                 S_0280A0_BLEND_BYPASS(blend_bypass) |
>>                 S_0280A0_BLEND_CLAMP(blend_clamp) |
>> +               S_0280A0_SIMPLE_FLOAT(1) |
>>                 S_0280A0_NUMBER_TYPE(ntype) |
>>                 S_0280A0_ENDIAN(endian);
>>
>
>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.