Bug 5824 - Rounding error introduced in recent commit
Summary: Rounding error introduced in recent commit
Status: RESOLVED NOTABUG
Alias: None
Product: cairo
Classification: Unclassified
Component: pdf backend (show other bugs)
Version: 1.1.1
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Kristian Høgsberg
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-02-07 02:16 UTC by Jens Taprogge
Modified: 2006-02-06 07:33 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Jens Taprogge 2006-02-07 02:16:55 UTC
I think a recent commit introduced a rounding error in the PDF backend:

Index: cairo-pdf-surface.c
===================================================================
RCS file: /cvs/cairo/cairo/src/cairo-pdf-surface.c,v
retrieving revision 1.69
retrieving revision 1.70
diff -u -d -r1.69 -r1.70
--- cairo-pdf-surface.c	16 Dec 2005 11:02:35 -0000	1.69
+++ cairo-pdf-surface.c	5 Jan 2006 13:06:50 -0000	1.70
@@ -971,11 +971,11 @@
     n_stops = pattern->n_stops;
     
     for (i = 0; i < pattern->n_stops; i++) {
-	stops[i].color_char[0] = pattern->stops[i].color.red   * 0xff + 0.5;
-	stops[i].color_char[1] = pattern->stops[i].color.green * 0xff + 0.5;
-	stops[i].color_char[2] = pattern->stops[i].color.blue  * 0xff + 0.5;
-	stops[i].color_char[3] = pattern->stops[i].color.alpha * 0xff + 0.5;
-	stops[i].offset = _cairo_fixed_to_double (pattern->stops[i].offset);
+	stops[i].color_char[0] = pattern->stops[i].color.red   >> 8;
+	stops[i].color_char[1] = pattern->stops[i].color.green >> 8;
+	stops[i].color_char[2] = pattern->stops[i].color.blue  >> 8;
+	stops[i].color_char[3] = pattern->stops[i].color.alpha >> 8;
+	stops[i].offset = _cairo_fixed_to_double (pattern->stops[i].x);
 	stops[i].id = i;
     }

The colour conversion should rather be:
	stops[i].color_char[0] = (pattern->stops[i].color.red   + 0x80) >> 8;
	stops[i].color_char[1] = (pattern->stops[i].color.green + 0x80) >> 8;
	stops[i].color_char[2] = (pattern->stops[i].color.blue  + 0x80) >> 8;
	stops[i].color_char[3] = (pattern->stops[i].color.alpha + 0x80) >> 8;
(Think of a color value such as 0x12ff which should obviously be converted to 0x13)

Jens
Comment 1 Behdad Esfahbod 2006-02-07 02:33:23 UTC
Given the old code, the correct conversion is what I noted here:

  http://lists.freedesktop.org/archives/cairo/2005-October/005433.html

ie, 

        stops[i].color_char[0] = pattern->stops[i].color.red   * 0xff + 0.5;

should be changed to something like this:

        stops[i].color_char[0] = pattern->stops[i].color.red   * (256 - 1e-5);


but now that stops[i].color_char[0] are uchars, the current code is correct,
cause, we want to have a uniform mapping, and that's what we've got:

  0000-00ff -> 00
  0100-01ff -> 01
  ...
  ff00-ffff -> ff

So each target bin is stuffed with exactly 0x100 source items.  Perfect.
In other words, no, 0x12ff should map to 0x12, cause we cannot map 0xffff to
0x100 anyway...


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.