[Mesa-dev] [PATCH 04/13] nouveau: Fix non-1x post-scale factor with DOT3 combiner

[Ian Romanick]

On 08/24/2016 11:53 AM, Ilia Mirkin wrote:
> On Wed, Aug 24, 2016 at 2:46 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Fixes long standing bug on NV10 and NV20 where using a non-1x RGB or A
>> post-scale with GL_DOT3_RGB or GL_DOT3_RGBA texture environment would
>> not work.
>>
>> The old combiner math uses HALF_BIAS_NORMAL and HALF_BIAS_NEGATE.  The
>> GL_NV_register_combiners defines these as
>>
>>     HALF_BIAS_NORMAL_NV       max(0.0, e) - 0.5
>>     HALF_BIAS_NEGATE_NV       -max(0.0, e) + 0.5
>>
>> In order to get the correct result from the dot-product, the
>> intermediate dot-product must be multiplied by 4.  This is a literal
>> implementation of the GL_ARB_texture_env_dot3 spec.  It also requires
>> using the register combiner post-scale.  As a result, the post-scale
>> cannot be used for the post-scale set by the application.
>>
>> The new combiner math uses EXPAND_NORMAL and EXPAND_NEGATE.  The
>> GL_NV_register_combiners defines these as
>>
>>     EXPAND_NORMAL_NV          2.0 * max(0.0, e) - 1.0
>>     EXPAND_NEGATE_NV          -2.0 * max(0.0, e) + 1.0
>>
>> Since this fully expands the value to [-1, 1] range, the intermediate
>> dot-product result is the desired value.  This leaves the register
>> combiner post-scale available for application use.
>>
>> NOTE: I have not actually tested this.
> 
> I can confirm that this is what the GL_NV_register_combiners specs
> say, but I have no clue if this is 1:1 with how the hw works (seems
> quite likely though). Is there a piglit test that you're aware of that
> would be sensitive to this? If so, I may test it out. In the
> meanwhile, have a

As far as I can tell, the only tests for DOT3 texture environment is
glean texcombine... and that appears to use the EXT version.  The next
patch enables the EXT, so those tests should start running.

> Acked-by: Ilia Mirkin <imirkin at alum.mit.edu>
> 
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>>  src/mesa/drivers/dri/nouveau/nv10_state_frag.c | 18 ++++++++----------
>>  1 file changed, 8 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/nouveau/nv10_state_frag.c b/src/mesa/drivers/dri/nouveau/nv10_state_frag.c
>> index c007c6c..e78eac3 100644
>> --- a/src/mesa/drivers/dri/nouveau/nv10_state_frag.c
>> +++ b/src/mesa/drivers/dri/nouveau/nv10_state_frag.c
>> @@ -135,7 +135,7 @@ get_input_source(struct combiner_state *rc, int source)
>>  /* Get the RC input mapping for the specified texture_env_combine
>>   * operand, possibly inverted or biased. */
>>  #define INVERT 0x1
>> -#define HALF_BIAS 0x2
>> +#define NORMALIZE 0x2
>>
>>  static uint32_t
>>  get_input_mapping(struct combiner_state *rc, int operand, int flags)
>> @@ -148,12 +148,12 @@ get_input_mapping(struct combiner_state *rc, int operand, int flags)
>>                 map |= RC_IN_USAGE(ALPHA);
>>
>>         if (is_negative_operand(operand) == !(flags & INVERT))
>> -               map |= flags & HALF_BIAS ?
>> -                       RC_IN_MAPPING(HALF_BIAS_NEGATE) :
>> +               map |= flags & NORMALIZE ?
>> +                       RC_IN_MAPPING(EXPAND_NEGATE) :
>>                         RC_IN_MAPPING(UNSIGNED_INVERT);
>>         else
>> -               map |= flags & HALF_BIAS ?
>> -                       RC_IN_MAPPING(HALF_BIAS_NORMAL) :
>> +               map |= flags & NORMALIZE ?
>> +                       RC_IN_MAPPING(EXPAND_NORMAL) :
>>                         RC_IN_MAPPING(UNSIGNED_IDENTITY);
>>
>>         return map;
>> @@ -270,12 +270,10 @@ setup_combiner(struct combiner_state *rc)
>>
>>         case GL_DOT3_RGB:
>>         case GL_DOT3_RGBA:
>> -               INPUT_ARG(rc, A, 0, HALF_BIAS);
>> -               INPUT_ARG(rc, B, 1, HALF_BIAS);
>> +               INPUT_ARG(rc, A, 0, NORMALIZE);
>> +               INPUT_ARG(rc, B, 1, NORMALIZE);
>>
>> -               rc->out = RC_OUT_DOT_AB | RC_OUT_SCALE_4;
>> -
>> -               assert(!rc->logscale);
>> +               rc->out = RC_OUT_DOT_AB;
>>                 break;
>>
>>         default:
>> --
>> 2.5.5
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>