[cairo] [PATCH] Use floating-point offsets for color stops
Carl Worth
cworth at cworth.org
Wed Mar 26 11:40:33 PDT 2008
A kind user and gtk-engines hacker, (Andrea "Cimi" Cimitan), recently
pointed out a rendering change of some gradients between the cairo
1.5.10 and cairo 1.5.12 snapshots. Cimi also kindly bisected the
problem down to the change from 16.16 to 24.8 fixed-point values
within cairo.
What I found in Cimi's case was that a color stop was being positioned
at (or very near) a half-integer position on the device-pixel grid. I
also found that we were using fixed-point values for the gradient
color stops.
Of course, gradient color stop values are always within the range
[0.0, 1.0]. So using a 16.16 fixed-point value means we only have 16
interesting bits of information for each offset. And with the change
to 24.8 values, we only have 8 bits for each offset. That's very
little precision for no good reason, (23 of the other 24 bits are
always 0).
Meanwhile, the code that is actually using these offsets almost always
wants to do floating-point arithmetic and actually jumps through quite
a few hoops to do so. For example, (from the svg backend):
stops[i + pattern->n_stops - 1].x =
_cairo_fixed_from_double (0.5 + 0.5
* _cairo_fixed_to_double (stops[i + pattern->n_stops - 1].x));
So we're losing a ton of precision by misapplying fixed-point storage
here, and all the manipulation we're doing is floating-point
anyway. This is really nonsense, (and certainly my fault to begin
with). The only fixed-point usage of color-stop offsets I could find
was in converting to pixman and glitz offsets, (and I think the glitz
conversion has likely been broken since the 16.16->24.8 change as
well).
So anyway, here's a patch to convert the gradient color-stop offsets
from fixed-point to floating-point. I also changed the poor name "x"
to a better name "offset", (which also helped me ensure I made all the
changes necessary). Notice how much nicer the above code snippet is:
stops[i + pattern->n_stops - 1].offset =
0.5 + 0.5 * stops[i + pattern->n_stops - 1].offset;
This change does touch glitz, quartz, and win32-printing surface
backends which I haven't tested at all, (not even compile tested). So
I could use some help in ensuring I didn't break any of those.
Meanwhile, the patch does eliminate the regression that Cimi
noticed. And I'm currently running the test suite to ensure that the
image, ps, pdf, and svg backend changes didn't cause any problems. It
looks like the gradient-using reference images will all need to be
updated again, (perhaps to the same images we had before the
16.16->24.8 change).
-Carl
-------------- next part --------------
From 616e453db3dd17524bbd05a3e5c0ceaabdd3100c Mon Sep 17 00:00:00 2001
From: Carl Worth <cworth at cworth.org>
Date: Wed, 26 Mar 2008 11:31:04 -0700
Subject: [PATCH] Use floating-point offsets for color stops
Previously we were using the cairo_fixed_t type which meant we've
historically only been using 16 bits of precision for these offsets,
and recently only 8 bits. Meanwhile, all manipulatons of offsets
have been in floating-point anyway, so we might as well store them
that way.
This change also prevents a rendering regression introduced with the
24.8->16.16 change betwen snapshots 1.5.10 and 1.5.12 .
---
src/cairo-glitz-surface.c | 2 +-
src/cairo-pattern.c | 10 ++++------
src/cairo-pdf-surface.c | 6 +++---
src/cairo-ps-surface.c | 6 +++---
src/cairo-quartz-surface.c | 6 +++---
src/cairo-svg-surface.c | 31 ++++++++++++++-----------------
src/cairo-win32-printing-surface.c | 2 +-
src/cairoint.h | 2 +-
8 files changed, 30 insertions(+), 35 deletions(-)
diff --git a/src/cairo-glitz-surface.c b/src/cairo-glitz-surface.c
index 5144895..592dd13 100644
--- a/src/cairo-glitz-surface.c
+++ b/src/cairo-glitz-surface.c
@@ -817,7 +817,7 @@ _cairo_glitz_pattern_acquire_surface (cairo_pattern_t *pattern,
(((int) (gradient->stops[i].color.green_short >> 8)) << 8) |
(((int) (gradient->stops[i].color.blue_short >> 8)));
- params[n_base_params + 3 * i + 0] = gradient->stops[i].x;
+ params[n_base_params + 3 * i + 0] = _cairo_fixed_16_16_from_double (gradient->stops[i].offset);
params[n_base_params + 3 * i + 1] = i << 16;
params[n_base_params + 3 * i + 2] = 0;
}
diff --git a/src/cairo-pattern.c b/src/cairo-pattern.c
index 8ec3987..bd67163 100644
--- a/src/cairo-pattern.c
+++ b/src/cairo-pattern.c
@@ -821,7 +821,6 @@ _cairo_pattern_add_color_stop (cairo_gradient_pattern_t *pattern,
double alpha)
{
cairo_gradient_stop_t *stops;
- cairo_fixed_t x;
unsigned int i;
if (pattern->n_stops >= pattern->stops_size) {
@@ -834,10 +833,9 @@ _cairo_pattern_add_color_stop (cairo_gradient_pattern_t *pattern,
stops = pattern->stops;
- x = _cairo_fixed_from_double (offset);
for (i = 0; i < pattern->n_stops; i++)
{
- if (x < stops[i].x)
+ if (offset < stops[i].offset)
{
memmove (&stops[i + 1], &stops[i],
sizeof (cairo_gradient_stop_t) * (pattern->n_stops - i));
@@ -846,7 +844,7 @@ _cairo_pattern_add_color_stop (cairo_gradient_pattern_t *pattern,
}
}
- stops[i].x = x;
+ stops[i].offset = offset;
stops[i].color.red = red;
stops[i].color.green = green;
@@ -1208,7 +1206,7 @@ _cairo_pattern_acquire_surface_for_gradient (cairo_gradient_pattern_t *pattern,
}
for (i = 0; i < pattern->n_stops; i++) {
- pixman_stops[i].x = _cairo_fixed_to_16_16 (pattern->stops[i].x);
+ pixman_stops[i].x = _cairo_fixed_16_16_from_double (pattern->stops[i].offset);
pixman_stops[i].color.red = pattern->stops[i].color.red_short;
pixman_stops[i].color.green = pattern->stops[i].color.green_short;
pixman_stops[i].color.blue = pattern->stops[i].color.blue_short;
@@ -2155,7 +2153,7 @@ cairo_pattern_get_color_stop_rgba (cairo_pattern_t *pattern,
return _cairo_error (CAIRO_STATUS_INVALID_INDEX);
if (offset)
- *offset = _cairo_fixed_to_double(gradient->stops[index].x);
+ *offset = gradient->stops[index].offset;
if (red)
*red = gradient->stops[index].color.red;
if (green)
diff --git a/src/cairo-pdf-surface.c b/src/cairo-pdf-surface.c
index cce4b3c..7dad2f5 100644
--- a/src/cairo-pdf-surface.c
+++ b/src/cairo-pdf-surface.c
@@ -1981,7 +1981,7 @@ _cairo_pdf_surface_emit_pattern_stops (cairo_pdf_surface_t *surface,
stops[i].color[3] = pattern->stops[i].color.alpha;
if (!CAIRO_ALPHA_IS_OPAQUE (stops[i].color[3]))
emit_alpha = TRUE;
- stops[i].offset = _cairo_fixed_to_double (pattern->stops[i].x);
+ stops[i].offset = pattern->stops[i].offset;
}
if (pattern->base.extend == CAIRO_EXTEND_REPEAT ||
@@ -2220,8 +2220,8 @@ _cairo_pdf_surface_emit_linear_pattern (cairo_pdf_surface_t *surface,
assert (status == CAIRO_STATUS_SUCCESS);
cairo_matrix_multiply (&pat_to_pdf, &pat_to_pdf, &surface->cairo_to_pdf);
- first_stop = _cairo_fixed_to_double (gradient->stops[0].x);
- last_stop = _cairo_fixed_to_double (gradient->stops[gradient->n_stops - 1].x);
+ first_stop = gradient->stops[0].offset;
+ last_stop = gradient->stops[gradient->n_stops - 1].offset;
if (pattern->base.base.extend == CAIRO_EXTEND_REPEAT ||
pattern->base.base.extend == CAIRO_EXTEND_REFLECT) {
diff --git a/src/cairo-ps-surface.c b/src/cairo-ps-surface.c
index bfe8b19..b771048 100644
--- a/src/cairo-ps-surface.c
+++ b/src/cairo-ps-surface.c
@@ -2467,7 +2467,7 @@ _cairo_ps_surface_emit_pattern_stops (cairo_ps_surface_t *surface,
stops[i].color[1] = stop->color.green;
stops[i].color[2] = stop->color.blue;
stops[i].color[3] = stop->color.alpha;
- stops[i].offset = _cairo_fixed_to_double (pattern->stops[i].x);
+ stops[i].offset = pattern->stops[i].offset;
}
if (pattern->base.extend == CAIRO_EXTEND_REPEAT ||
@@ -2586,8 +2586,8 @@ _cairo_ps_surface_emit_linear_pattern (cairo_ps_surface_t *surface,
assert (status == CAIRO_STATUS_SUCCESS);
cairo_matrix_multiply (&pat_to_ps, &pat_to_ps, &surface->cairo_to_ps);
- first_stop = _cairo_fixed_to_double (gradient->stops[0].x);
- last_stop = _cairo_fixed_to_double (gradient->stops[gradient->n_stops - 1].x);
+ first_stop = gradient->stops[0].offset;
+ last_stop = gradient->stops[gradient->n_stops - 1].offset;
if (pattern->base.base.extend == CAIRO_EXTEND_REPEAT ||
pattern->base.base.extend == CAIRO_EXTEND_REFLECT) {
diff --git a/src/cairo-quartz-surface.c b/src/cairo-quartz-surface.c
index 4c2e9b5..5296eb5 100644
--- a/src/cairo-quartz-surface.c
+++ b/src/cairo-quartz-surface.c
@@ -556,7 +556,7 @@ ComputeGradientValue (void *info, const float *in, float *out)
}
for (i = 0; i < grad->n_stops; i++) {
- if (_cairo_fixed_to_double (grad->stops[i].x) > fdist)
+ if (grad->stops[i].offset > fdist)
break;
}
@@ -568,8 +568,8 @@ ComputeGradientValue (void *info, const float *in, float *out)
out[2] = grad->stops[i].color.blue;
out[3] = grad->stops[i].color.alpha;
} else {
- float ax = _cairo_fixed_to_double(grad->stops[i-1].x);
- float bx = _cairo_fixed_to_double(grad->stops[i].x) - ax;
+ float ax = grad->stops[i-1].offset;
+ float bx = grad->stops[i].offset - ax;
float bp = (fdist - ax)/bx;
float ap = 1.0 - bp;
diff --git a/src/cairo-svg-surface.c b/src/cairo-svg-surface.c
index a610b12..1b7b3e8 100644
--- a/src/cairo-svg-surface.c
+++ b/src/cairo-svg-surface.c
@@ -1294,7 +1294,7 @@ _cairo_svg_surface_emit_pattern_stops (cairo_output_stream_t *output,
"<stop offset=\"%f\" style=\""
"stop-color: rgb(%f%%,%f%%,%f%%); "
"stop-opacity: %f;\"/>\n",
- _cairo_fixed_to_double (pattern->stops[0].x),
+ pattern->stops[0].offset,
pattern->stops[0].color.red * 100.0,
pattern->stops[0].color.green * 100.0,
pattern->stops[0].color.blue * 100.0,
@@ -1311,22 +1311,20 @@ _cairo_svg_surface_emit_pattern_stops (cairo_output_stream_t *output,
for (i = 0; i < pattern->n_stops; i++) {
if (reverse_stops) {
stops[i] = pattern->stops[pattern->n_stops - i - 1];
- stops[i].x = _cairo_fixed_from_double (1.0 - _cairo_fixed_to_double (stops[i].x));
+ stops[i].offset = 1.0 - stops[i].offset;
} else
stops[i] = pattern->stops[i];
if (emulate_reflect) {
- stops[i].x /= 2;
+ stops[i].offset /= 2;
if (i > 0 && i < (pattern->n_stops - 1)) {
if (reverse_stops) {
stops[i + pattern->n_stops - 1] = pattern->stops[i];
- stops[i + pattern->n_stops - 1].x =
- _cairo_fixed_from_double (0.5 + 0.5
- * _cairo_fixed_to_double (stops[i + pattern->n_stops - 1].x));
+ stops[i + pattern->n_stops - 1].offset =
+ 0.5 + 0.5 * stops[i + pattern->n_stops - 1].offset;
} else {
stops[i + pattern->n_stops - 1] = pattern->stops[pattern->n_stops - i - 1];
- stops[i + pattern->n_stops - 1].x =
- _cairo_fixed_from_double (1 - 0.5
- * _cairo_fixed_to_double (stops [i + pattern->n_stops - 1].x));
+ stops[i + pattern->n_stops - 1].offset =
+ 1 - 0.5 * stops[i + pattern->n_stops - 1].offset;
}
}
}
@@ -1338,8 +1336,7 @@ _cairo_svg_surface_emit_pattern_stops (cairo_output_stream_t *output,
if (start_offset >= 0.0)
for (i = 0; i < n_stops; i++) {
- offset = start_offset + (1 - start_offset ) *
- _cairo_fixed_to_double (stops[i].x);
+ offset = start_offset + (1 - start_offset ) * stops[i].offset;
_cairo_output_stream_printf (output,
"<stop offset=\"%f\" style=\""
"stop-color: rgb(%f%%,%f%%,%f%%); "
@@ -1356,14 +1353,14 @@ _cairo_svg_surface_emit_pattern_stops (cairo_output_stream_t *output,
cairo_color_t offset_color_start, offset_color_stop;
for (i = 0; i < n_stops; i++) {
- if (_cairo_fixed_to_double (stops[i].x) >= -start_offset) {
+ if (stops[i].offset >= -start_offset) {
if (i > 0) {
- if (stops[i].x != stops[i-1].x) {
+ if (stops[i].offset != stops[i-1].offset) {
double x0, x1;
cairo_color_t *color0, *color1;
- x0 = _cairo_fixed_to_double (stops[i-1].x);
- x1 = _cairo_fixed_to_double (stops[i].x);
+ x0 = stops[i-1].offset;
+ x1 = stops[i].offset;
color0 = &stops[i-1].color;
color1 = &stops[i].color;
offset_color_start.red = color0->red + (color1->red - color0->red)
@@ -1405,7 +1402,7 @@ _cairo_svg_surface_emit_pattern_stops (cairo_output_stream_t *output,
"<stop offset=\"%f\" style=\""
"stop-color: rgb(%f%%,%f%%,%f%%); "
"stop-opacity: %f;\"/>\n",
- _cairo_fixed_to_double (stops[i].x) + start_offset,
+ stops[i].offset + start_offset,
stops[i].color.red * 100.0,
stops[i].color.green * 100.0,
stops[i].color.blue * 100.0,
@@ -1416,7 +1413,7 @@ _cairo_svg_surface_emit_pattern_stops (cairo_output_stream_t *output,
"<stop offset=\"%f\" style=\""
"stop-color: rgb(%f%%,%f%%,%f%%); "
"stop-opacity: %f;\"/>\n",
- 1.0 + _cairo_fixed_to_double (stops[i].x) + start_offset,
+ 1.0 + stops[i].offset + start_offset,
stops[i].color.red * 100.0,
stops[i].color.green * 100.0,
stops[i].color.blue * 100.0,
diff --git a/src/cairo-win32-printing-surface.c b/src/cairo-win32-printing-surface.c
index d83b732..cc3a17b 100644
--- a/src/cairo-win32-printing-surface.c
+++ b/src/cairo-win32-printing-surface.c
@@ -757,7 +757,7 @@ _cairo_win32_printing_surface_paint_linear_pattern (cairo_win32_surface_t *surfa
}
stop = i%num_rects + 1;
- vert[i*2+1].x = (LONG)(d*(range_start + i/num_rects + _cairo_fixed_to_double (pattern->base.stops[stop].x)));
+ vert[i*2+1].x = (LONG)(d*(range_start + i/num_rects + pattern->base.stops[stop].offset));
vert[i*2+1].y = (LONG) clip.bottom;
if (extend == CAIRO_EXTEND_REFLECT && (range_start+(i/num_rects))%2)
stop = num_rects - stop;
diff --git a/src/cairoint.h b/src/cairoint.h
index ea47ed5..9fa4bde 100644
--- a/src/cairoint.h
+++ b/src/cairoint.h
@@ -790,7 +790,7 @@ typedef struct _cairo_surface_pattern {
} cairo_surface_pattern_t;
typedef struct _cairo_gradient_stop {
- cairo_fixed_t x;
+ double offset;
cairo_color_t color;
} cairo_gradient_stop_t;
--
1.5.5.rc0.24.g86c30
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.cairographics.org/archives/cairo/attachments/20080326/df1be40b/attachment-0001.pgp
More information about the cairo
mailing list