Bug 1271112 - Check transformed gradient end points for fixed point overflow, not the size of the gradient. r=jrmuizel, a=sledru
authorMarkus Stange <mstange@themasta.com>
Thu, 12 May 2016 17:45:38 -0400
changeset 333063 8ceaff1404be27309bf6fa0e127f9bd9ce36a3fc
parent 333062 4fd7d6aeb9b4052ca530813086d0117554175942
child 333064 b8e3e5ba3736217fda1fb1b9f26b97a13216a74a
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel, sledru
bugs1271112, 32000
milestone48.0a2
Bug 1271112 - Check transformed gradient end points for fixed point overflow, not the size of the gradient. r=jrmuizel, a=sledru The gradient on this website had, the float values of xdim and ydim were about 7500, which is definitely representable by 16.16 fixed point values. But the matrix had a large (> 32000) translation on it. MozReview-Commit-ID: 1WVhZQLF99g
gfx/cairo/cairo/src/cairo-image-surface.c
gfx/cairo/cairo/src/cairo-pattern.c
layout/reftests/css-gradients/large-gradient-5-ref.html
layout/reftests/css-gradients/large-gradient-5.html
layout/reftests/css-gradients/reftest.list
--- a/gfx/cairo/cairo/src/cairo-image-surface.c
+++ b/gfx/cairo/cairo/src/cairo-image-surface.c
@@ -1122,41 +1122,41 @@ static pixman_image_t *
 	pixman_stops[i].color.green = pattern->stops[i].color.green_short;
 	pixman_stops[i].color.blue  = pattern->stops[i].color.blue_short;
 	pixman_stops[i].color.alpha = pattern->stops[i].color.alpha_short;
     }
 
     if (pattern->base.type == CAIRO_PATTERN_TYPE_LINEAR) {
 	cairo_linear_pattern_t *linear = (cairo_linear_pattern_t *) pattern;
 	pixman_point_fixed_t p1, p2;
-	cairo_fixed_t xdim, ydim;
-
-	xdim = fabs (linear->p2.x - linear->p1.x);
-	ydim = fabs (linear->p2.y - linear->p1.y);
+	double x0, y0, x1, y1, maxabs;
 
 	/*
 	 * Transform the matrix to avoid overflow when converting between
 	 * cairo_fixed_t and pixman_fixed_t (without incurring performance
 	 * loss when the transformation is unnecessary).
 	 *
-	 * XXX: Consider converting out-of-range co-ordinates and transforms.
 	 * Having a function to compute the required transformation to
 	 * "normalize" a given bounding box would be generally useful -
 	 * cf linear patterns, gradient patterns, surface patterns...
 	 */
-	if (_cairo_fixed_integer_ceil (xdim) > PIXMAN_MAX_INT ||
-	    _cairo_fixed_integer_ceil (ydim) > PIXMAN_MAX_INT)
+	x0 = _cairo_fixed_to_double (linear->p1.x);
+	y0 = _cairo_fixed_to_double (linear->p1.y);
+	x1 = _cairo_fixed_to_double (linear->p2.x);
+	y1 = _cairo_fixed_to_double (linear->p2.y);
+	cairo_matrix_transform_point (&matrix, &x0, &y0);
+	cairo_matrix_transform_point (&matrix, &x1, &y1);
+	maxabs = MAX (MAX (fabs (x0), fabs (x1)), MAX (fabs (y0), fabs (y1)));
+
+	if (maxabs > PIXMAN_MAX_INT)
 	{
 	    double sf;
 	    cairo_matrix_t scale;
 
-	    if (xdim > ydim)
-		sf = PIXMAN_MAX_INT / _cairo_fixed_to_double (xdim);
-	    else
-		sf = PIXMAN_MAX_INT / _cairo_fixed_to_double (ydim);
+	    sf = PIXMAN_MAX_INT / maxabs;
 
 	    p1.x = _cairo_fixed_16_16_from_double (_cairo_fixed_to_double (linear->p1.x) * sf);
 	    p1.y = _cairo_fixed_16_16_from_double (_cairo_fixed_to_double (linear->p1.y) * sf);
 	    p2.x = _cairo_fixed_16_16_from_double (_cairo_fixed_to_double (linear->p2.x) * sf);
 	    p2.y = _cairo_fixed_16_16_from_double (_cairo_fixed_to_double (linear->p2.y) * sf);
 
 	    /* cairo_matrix_scale does a pre-scale, we want a post-scale */
 	    cairo_matrix_init_scale (&scale, sf, sf);
--- a/gfx/cairo/cairo/src/cairo-pattern.c
+++ b/gfx/cairo/cairo/src/cairo-pattern.c
@@ -1362,42 +1362,42 @@ static cairo_int_status_t
 	if (! CAIRO_ALPHA_SHORT_IS_OPAQUE (pixman_stops[i].color.alpha))
 	    opaque = FALSE;
     }
 
     if (pattern->base.type == CAIRO_PATTERN_TYPE_LINEAR)
     {
 	cairo_linear_pattern_t *linear = (cairo_linear_pattern_t *) pattern;
 	pixman_point_fixed_t p1, p2;
-	cairo_fixed_t xdim, ydim;
-
-	xdim = linear->p2.x - linear->p1.x;
-	ydim = linear->p2.y - linear->p1.y;
+        double x0, y0, x1, y1, maxabs;
 
 	/*
 	 * Transform the matrix to avoid overflow when converting between
 	 * cairo_fixed_t and pixman_fixed_t (without incurring performance
 	 * loss when the transformation is unnecessary).
 	 *
-	 * XXX: Consider converting out-of-range co-ordinates and transforms.
 	 * Having a function to compute the required transformation to
 	 * "normalize" a given bounding box would be generally useful -
 	 * cf linear patterns, gradient patterns, surface patterns...
 	 */
+	x0 = _cairo_fixed_to_double (linear->p1.x);
+	y0 = _cairo_fixed_to_double (linear->p1.y);
+	x1 = _cairo_fixed_to_double (linear->p2.x);
+	y1 = _cairo_fixed_to_double (linear->p2.y);
+	cairo_matrix_transform_point (&matrix, &x0, &y0);
+	cairo_matrix_transform_point (&matrix, &x1, &y1);
+	maxabs = MAX (MAX (fabs (x0), fabs (x1)), MAX (fabs (y0), fabs (y1)));
+
 #define PIXMAN_MAX_INT ((pixman_fixed_1 >> 1) - pixman_fixed_e) /* need to ensure deltas also fit */
-	if (_cairo_fixed_integer_ceil (xdim) > PIXMAN_MAX_INT ||
-	    _cairo_fixed_integer_ceil (ydim) > PIXMAN_MAX_INT)
+	if (maxabs > PIXMAN_MAX_INT)
 	{
 	    double sf;
 	    cairo_matrix_t scale;
 
-	    if (xdim > ydim)
-		sf = PIXMAN_MAX_INT / _cairo_fixed_to_double (xdim);
-	    else
-		sf = PIXMAN_MAX_INT / _cairo_fixed_to_double (ydim);
+            sf = PIXMAN_MAX_INT / maxabs;
 
 	    p1.x = _cairo_fixed_16_16_from_double (_cairo_fixed_to_double (linear->p1.x) * sf);
 	    p1.y = _cairo_fixed_16_16_from_double (_cairo_fixed_to_double (linear->p1.y) * sf);
 	    p2.x = _cairo_fixed_16_16_from_double (_cairo_fixed_to_double (linear->p2.x) * sf);
 	    p2.y = _cairo_fixed_16_16_from_double (_cairo_fixed_to_double (linear->p2.y) * sf);
 
 	    /* cairo_matrix_scale does a pre-scale, we want a post-scale */
 	    cairo_matrix_init_scale (&scale, sf, sf);
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-gradients/large-gradient-5-ref.html
@@ -0,0 +1,19 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<!DOCTYPE html>
+<html lang="en">
+<meta charset="utf-8">
+<title>Reference for: Make sure that large gradient backgrounds are painted even at extreme scroll positions</title>
+<!-- See https://bugzilla.mozilla.org/show_bug.cgi?id=1271112 -->
+
+<style>
+
+html {
+  background-color: lime;
+}
+
+</style>
+
+<body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-gradients/large-gradient-5.html
@@ -0,0 +1,30 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<!DOCTYPE html>
+<html lang="en">
+<meta charset="utf-8">
+<title>Make sure that large gradient backgrounds are painted even at extreme scroll positions</title>
+<!-- See https://bugzilla.mozilla.org/show_bug.cgi?id=1271112 -->
+
+<style>
+
+html, body {
+  overflow: hidden;
+}
+
+body {
+  margin: 0;
+  height: 100000px;
+  /* a green gradient that is not opaque on top of a red background color */
+  background: repeating-linear-gradient(45deg, rgba(0, 255, 0, 1.0) 0%, rgba(0, 255, 0, 0.99) 10%, rgba(0, 255, 0, 1.0) 20%) red;
+}
+
+</style>
+
+<body>
+
+<script>
+document.documentElement.scrollTop = 35000;
+</script>
--- a/layout/reftests/css-gradients/reftest.list
+++ b/layout/reftests/css-gradients/reftest.list
@@ -145,9 +145,10 @@ fuzzy-if(d2d,47,400) == linear-onestoppo
 == repeating-radial-onestopposition-1a.html orange-square.html
 == repeating-radial-onestopposition-1b.html orange-square.html
 == repeating-radial-onestopposition-1c.html orange-square.html
 == bug-916535-background-repeat-linear.html bug-916535-background-repeat-linear-ref.html
 fuzzy(1,800000) == large-gradient-1.html large-gradient-1-ref.html
 fuzzy-if(Android,4,1) == large-gradient-2.html large-gradient-2-ref.html # Bug 1182082
 fuzzy(1,800000) == large-gradient-3.html large-gradient-3-ref.html
 == large-gradient-4.html large-gradient-4-ref.html
+fuzzy(2,800000) == large-gradient-5.html large-gradient-5-ref.html
 == 1224761-1.html 1224761-1-ref.html