Using sincos (was Re: [cairo] Questions and optimizations in cairo-arc)

Owen Taylor otaylor at
Mon Aug 22 14:43:08 PDT 2005

On Tue, 2005-07-26 at 20:20 -0400, Behdad Esfahbod wrote:
> > > Please feel free to put a patch together for this change alone, (along
> > > with the necessary configure magic to define HAVE_SINCOS
> > > appropriately).
> Infact the check for sincos was already in configure and in one
> place in the code it was conditionally used.  The attached patch
> defines a macro named _cairo_sincos and uses it all over the
> code. (not that many places really)
> I had to introduce AC_GNU_SOURCE in, as sincos is
> not declared in math.h unless __USE_GNU is defined...  But we do
> want to use GNU extensions, so AC_GNU_SOURCE should be harmless.

It does make it easier for stuff to creep in, but that's hard to
prevent anyways. I'd probably just #define _GNU_SOURCE in cairoint.h
though rather than using a configure check ... it's less complex
and easily amenable to a comment like

#define _GNU_SOURCE               /* Needed for sincos() in math.h */

The AC_CHECK_FUNCS call doesn't use header files, so it doesn't
need the define.

+AC_CHECK_FUNCS(vasnprintf, sincos)

I'd put the sincos check after the libm check ... I'm not sure how
it is working before without libm in LIBS so I don't trust it to
stay working.

+    double sin_, cos_;
+    _cairo_sincos (angle / 4, &sin_, &cos_);
+    return 2.0/27.0 * pow (sin_, 6) / pow (cos_, 2);

I don't know Carl's opinion, but I find the sin_, cos_ names sort
of ugly like this. My personal preference would be to use 's'
and 'c' - since the scope of the use is just over two lines, the
short names don't hurt.

-       double dx = radius * cos (reflect ? -theta : theta);
-       double dy = radius * sin (reflect ? -theta : theta);
+       double dx, dy;
+       _cairo_sincos (reflect ? -theta : theta, &dx, &dy);
+       dx *= radius;
+       dy *= radius;

I'd use separate variables here - let the compiler do the 
register allocation, it's better at it. :-)

+# define _cairo_sincos(angle,s,c) sincos((angle), (s), (c))
+# define _cairo_sincos(angle,s,c) do { \
+       double _sincos_angle = (angle); \
+       *(s) = sin(_sincos_angle); \
+       *(c) = cos(_sincos_angle); \
+       } while (0)

Needs some 'fn((' => fn ()' fixes.

Other than those issues, the patch looks fine to me. If you don't get
to it immediately, you might want to file a bug in bugzilla so we
don't forget.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url :

More information about the cairo mailing list