[Mesa-dev] [PATCH 1/2] gallivm: Add no_rho_opt debug option

[Roland Scheidegger]

Am 16.04.2013 21:41, schrieb Brian Paul:
> On 04/16/2013 01:07 PM, sroland at vmware.com wrote:
>> From: Roland Scheidegger<sroland at vmware.com>
>>
>> This will calculate rho correctly as
>> sqrt(max((ds/dx)^2 + (dt/dx)^2 + (dr/dx)^2), (ds/dx)^2 + (dt/dx)^2 +
>> (dr/dx)^2))
>> instead of max(|ds/dx|,|dt/dx|,|dr/dx|,|ds/dy|,|dt/dy,|dr/dy|)
>> (for 3 coords - 2 coords work analogous, for 1 coord there's no point
>> doing
>> the exact version), for both implicit and explicit derivatives.
>> While such approximation seems to be allowed in OpenGL some APIs may
>> be less
>> forgiving, and the error can be quite large (sqrt(2) for 2 coords,
>> sqrt(3) for
>> 3 coords so wrong by nearly one mip level in the latter case).
>> This also helps to single out "real" bugs from "expected" ones, so it
>> is debug
>> only (though at least combined with no_brilinear I didn't really see
>> much of a
>> performance difference but only tested with a debug build - at least with
>> implicit mipmaps the instruction count is almost exactly the same
>> though the
>> instructions are more complex (1 sqrt and mul/adds instead of and/max
>> mostly).
>> The code when the option isn't set stays exactly the same.
> 
> It sounds like a better name for the option would be something like
> "sqrt_rho" because you're just computing rho with more precision.
Hmm I wanted to name this somewhat along the lines of "no_brilinear"
(which ultimately clamps the rho value to exact mip levels whereas this
makes rho calculation itself more accurate) but I guess it's not a very
good name. Maybe "no_rho_approx" would be better? I'd like to have a
somewhat consistent naming for all switches which disable somewhat fishy
(but enabled by default) optimizations, and "sqrt_rho" doesn't quite fit
that bill. I could live with it though.

> 
> 
>> ---
>>   src/gallium/auxiliary/gallivm/lp_bld_debug.h  |    3 +-
>>   src/gallium/auxiliary/gallivm/lp_bld_init.c   |    1 +
>>   src/gallium/auxiliary/gallivm/lp_bld_sample.c |  299
>> +++++++++++++++----------
>>   3 files changed, 185 insertions(+), 118 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_debug.h
>> b/src/gallium/auxiliary/gallivm/lp_bld_debug.h
>> index da873f3..b65a1f7 100644
>> --- a/src/gallium/auxiliary/gallivm/lp_bld_debug.h
>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_debug.h
>> @@ -42,7 +42,8 @@
>>   #define GALLIVM_DEBUG_NO_OPT        (1<<  3)
>>   #define GALLIVM_DEBUG_PERF          (1<<  4)
>>   #define GALLIVM_DEBUG_NO_BRILINEAR  (1<<  5)
>> -#define GALLIVM_DEBUG_GC            (1<<  6)
>> +#define GALLIVM_DEBUG_NO_RHO_OPT    (1<<  6)
> 
> GALLIVM_DEBUG_SQRT_RHO?
Yes, should be the same as the as the env var name.

Roland


> 
> Looks good otherwise.
> 
> -Brian
>