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

[Ilia Mirkin]

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

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