[Mesa-dev] [PATCH 2/2] glsl: Improve the accuracy of the acos() approximation.

[Iago Toral]

On Thu, 2016-03-03 at 16:23 -0800, Francisco Jerez wrote:
> Iago Toral <itoral at igalia.com> writes:
> 
> > On Thu, 2016-03-03 at 01:30 -0800, Kenneth Graunke wrote:
> >> From: Francisco Jerez <currojerez at riseup.net>
> >> 
> >> The adjusted polynomial coefficients come from the numerical
> >> minimization of the L2 norm of the relative error.  The old
> >> coefficients would give a maximum relative error of about 15000 ULP in
> >> the neighborhood around acos(x) = 0, the new ones give a relative
> >> error bounded by less than 2000 ULP in the same neighborhood.
> >> 
> >> Fixes four dEQP subtests:
> >> dEQP-GLES31.functional.shaders.builtin_functions.precision.acos.
> >> highp_compute.{scalar,vec2,vec3,vec4}
> >
> > Cool! I thought we would have to use higher order polynomials. Series
> > is:
> >
> > Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
> >
> > What about asin? It looks like the new coefficients should be useful
> > there as well.
> >
> That's likely to be more harmful than helpful, the new polynomial
> coefficients are optimized to minimize the relative error of acos
> *only*, which has a different magnitude profile from asin -- The
> relative error of acos(x) used to explode around x=1, while the relative
> error of asin(x) is likely to be highest around x=0, because that's
> where its magnitude is lowest.  If you switch asin over to the acos
> coefficients its overall relative error will likely increase, because it
> will improve its accuracy around x=1 where it barely matters and worsen
> it around x=0 where it's the most sensitive.  Feel free to put my R-b on
> the series:

I see, makes sense. Thanks for the explanation Curro!

> Reviewed-by: Francisco Jerez <currojerez at riseup.net>
> 
> >> ---
> >>  src/compiler/glsl/builtin_functions.cpp | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> Curro originally wrote this code for the SPIR-V front-end when working
> >> on Vulkan conformance.  I copy and pasted it over to GLSL, but all of
> >> the actual work is Curro's.
> >> 
> >> diff --git a/src/compiler/glsl/builtin_functions.cpp b/src/compiler/glsl/builtin_functions.cpp
> >> index 0d289c1..10fb21b 100644
> >> --- a/src/compiler/glsl/builtin_functions.cpp
> >> +++ b/src/compiler/glsl/builtin_functions.cpp
> >> @@ -3262,7 +3262,7 @@ builtin_builder::_acos(const glsl_type *type)
> >>     ir_variable *x = in_var(type, "x");
> >>     MAKE_SIG(type, always_available, 1, x);
> >>  
> >> -   body.emit(ret(sub(imm(M_PI_2f), asin_expr(x, 0.086566724f, -0.03102955f))));
> >> +   body.emit(ret(sub(imm(M_PI_2f), asin_expr(x, 0.08132463f, -0.02363318f))));
> >>  
> >>     return sig;
> >>  }
> >
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev