[Mesa-dev] [PATCH] glsl: Make blend_colordodge compare against 1.0 - FLT_EPSILON.

[Alejandro Piñeiro]

On 02/09/16 23:13, Kenneth Graunke wrote:
> On Friday, August 26, 2016 10:49:18 PM PDT Kenneth Graunke wrote:
>> This fixes a numerical precision issue that was causing two CTS
>> failures:
>>
>> ES31-CTS.blend_equation_advanced.blend_specific.GL_COLORBURN_KHR
>> ES31-CTS.blend_equation_advanced.blend_all.GL_COLORBURN_KHR_all_qualifier

FWIW: it fixes the equivalent GL44 tests.

>>
>> When blending with GL_COLORDODGE_KHR and these colors:
> 
> This should be GL_COLORBURN_KHR and the subject should be
> blend_colorburn.  Fixed locally.

Gentle reminder to use the fixed version.

>>
>>    dst = <0.372549027, 0.372549027, 0.372549027, 0.372549027>
>>    src = <0.09375, 0.046875, 0.0, 0.375>
>>
>> the normalized dst value became 0.99999994 (due to imprecisions in the
>> alpha scaling, presumably), which failed the dst >= 1.0 comparison.
>> The blue channel would then fall through to the dst < 1.0 && src >= 0
>> comparison, which was true, since src.b == 0.  This produced a factor
>> of 0.0 instead of 1.0.
>>
>> To work around this, compare with 1.0 - FLT_EPSILON.

Makes sense:
Reviewed-by: Alejandro Piñeiro <apinheiro at igalia.com>

Having said so, Kenneth mentioned on IRC that Francisco has some doubts
on this patch. CCing him just in case he wants to mention something.

>>
>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>> ---
>>  src/compiler/glsl/lower_blend_equation_advanced.cpp | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/compiler/glsl/lower_blend_equation_advanced.cpp b/src/compiler/glsl/lower_blend_equation_advanced.cpp
>> index a998df1..f8b0261 100644
>> --- a/src/compiler/glsl/lower_blend_equation_advanced.cpp
>> +++ b/src/compiler/glsl/lower_blend_equation_advanced.cpp
>> @@ -28,6 +28,7 @@
>>  #include "program/prog_instruction.h"
>>  #include "program/prog_statevars.h"
>>  #include "util/bitscan.h"
>> +#include <float.h>
>>  
>>  using namespace ir_builder;
>>  
>> @@ -101,7 +102,7 @@ blend_colorburn(ir_variable *src, ir_variable *dst)
>>      *   1 - min(1,(1-Cd)/Cs), if Cd < 1 and Cs > 0
>>      *   0, if Cd < 1 and Cs <= 0
>>      */
>> -   return csel(gequal(dst, imm3(1)), imm3(1),
>> +   return csel(gequal(dst, imm3(1 - FLT_EPSILON)), imm3(1),
>>                 csel(lequal(src, imm3(0)), imm3(0),
>>                      sub(imm3(1), min2(imm3(1), div(sub(imm3(1), dst), src)))));
>>  }
>>
> 
> 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 

-- 
Alejandro Piñeiro <apinheiro at igalia.com>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160908/3a05a00c/attachment.sig>