Bug 4188

Summary:	cairo_arc_to error bound calculation is flawed
Product:	cairo	Reporter:	Bertram Felgenhauer <bertram.felgenhauer>
Component:	general	Assignee:	Carl Worth <cworth>
Status:	RESOLVED FIXED	QA Contact:	cairo-bugs mailing list <cairo-bugs>
Severity:	normal
Priority:	high
Version:	0.9.3
Hardware:	x86 (IA32)
OS:	Linux (All)
Whiteboard:
i915 platform:		i915 features:
Attachments:	pull out cairo_matrix_transformed_circle_major_axis change cairo_arc_to to use the major axis calculation. remove cairo_matrix_compute_eigenvalues ok, fix the matrix transpose first

Description Bertram Felgenhauer 2005-08-22 11:17:50 UTC

The cairo_arc_to error bound calculation uses eigen values in an attempt to
determine the length of the major axis of the ellipse (which is, actually,
possible; the eigenvalues of A A' for a given transformation A are the squares
of the length of the major axes of the transformed unit circle); in many cases,
for example for rotations, this will results in taking square roots of negative
numbers.
Fortunately, the calculation in _arc_max_angle_for_tolerance_normalized uses
error > tolerance in its second loop which is always false if tolerance is NaN.

Comment 1 Bertram Felgenhauer 2005-08-22 11:21:11 UTC

Created attachment 2983 [details] [review]
pull out cairo_matrix_transformed_circle_major_axis

This patch pulls out the major axis calculation from cairo-pen.c to be
reused in cairo-arc.c.

Changelog:

	* src/cairo-pen.c (_cairo_pen_vertices_needed): use new function.
	strip comment of derivation for major axis length.

	* src/cairo-matrix.c (_cairo_matrix_transformed_circle_major_axis):
	use the correctly transposed version of the matrix.

	* src/cairoint.h, src/cairo-matrix.c
	(_cairo_matrix_transformed_circle_major_axis): new function split out
	of cairo-pen.c. UTF8-ify the comment that explains the calculation.

Comment 2 Bertram Felgenhauer 2005-08-22 11:22:54 UTC

Created attachment 2984 [details] [review]
change cairo_arc_to to use the major axis calculation.

This is the actual bug fix.

Changelog:

2005-08-22  Bertram Felgenhauer  <int-e@gmx.de>

	* src/cairo-arc.c (_arc_segments_needed): correct the calculation of
	the error bound.

Comment 3 Bertram Felgenhauer 2005-08-22 11:25:19 UTC

Created attachment 2985 [details] [review]
remove cairo_matrix_compute_eigenvalues

I'm not sure about this, but I can see no use for this function in a computer
graphics context; there are no users of this function left.

Changelog:

2005-08-22  Bertram Felgenhauer  <int-e@gmx.de>

	* src/cairoint.h, src/cairo-matrix.c
	(_cairo_matrix_compute_eigen_values): remove.

Comment 4 Bertram Felgenhauer 2005-08-22 16:23:10 UTC

Created attachment 2996 [details] [review]
ok, fix the matrix transpose first

ok, this only fixes the matrix transposition bug.

Comment 5 Carl Worth 2005-08-22 16:28:29 UTC

(In reply to comment #4)
> Created an attachment (id=2996) [edit]
> ok, fix the matrix transpose first
> 
> ok, this only fixes the matrix transposition bug.

Perfect. Thanks for doing it this way. This can definitely go in right away.

And the rest should be very straightforward from here.

Comment 6 Carl Worth 2005-08-22 17:15:43 UTC

Move bugs against "cvs" version to "0.9.3" so we can remove the "cvs" version.

Comment 7 Bertram Felgenhauer 2005-08-22 17:24:18 UTC

All the patches are commited with minor fixups.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.