Hello.
I think I have a fix for this bug:
https://bugzilla.redhat.com/show_bug.cgi?id=597227
The bug was caused by a few integer overflows in sun/java2d/pisces/Dasher.java
and sun/java2d/pisces/Stroker.java which would cause the whole container to
fill up with whatever colour the line had. Dasher.java was prone to 2 overflows:
one when computing the x and y coordinate lengths of the line at hand, and
another which actually happened in PiscesMath.hypot(int,int). I fixed these
two by turning the variables into longs and using PiscesMath.hypot(long,long).
Stroker.java was only prone to overflows when drawing end caps or when adding
offsets to points. I fixed these by making sure lineTo and moveTo never moved
too close to the boundaries set by Integer.MAX_VALUE and Integer.MIN_VALUE.
I welcome any feedback.
Thank you,
Denis Lila.
exporting patch:
# HG changeset patch
# User Denis Lila <dl...@redhat.com>
# Date 1276118857 14400
# Node ID 93b0b1eb56fbfa77f2ba9855e4d44ca601aa578c
# Parent 4d55419ce99e749da5037fa4d8247117f1a5cc2e
Fix for bug where drawing long dashed lines fills a large part of the window with the line's colour.
diff --git a/src/share/classes/sun/java2d/pisces/Dasher.java b/src/share/classes/sun/java2d/pisces/Dasher.java
--- a/src/share/classes/sun/java2d/pisces/Dasher.java
+++ b/src/share/classes/sun/java2d/pisces/Dasher.java
@@ -181,33 +181,37 @@
}
public void lineTo(int x1, int y1) {
+ long MAX = java.lang.Integer.MAX_VALUE;
+ long MIN = java.lang.Integer.MIN_VALUE;
while (true) {
+ // We use longs in this method to avoid overflows.
+
int d = dash[idx] - phase;
- int lx = x1 - x0;
- int ly = y1 - y0;
+ long xLen = (long)x1 - x0;
+ long yLen = (long)y1 - y0;
// Compute segment length in the untransformed
// coordinate system
// IMPL NOTE - use fixed point
- int l;
+ long origLen;
if (symmetric) {
- l = (int)((PiscesMath.hypot(lx, ly)*65536L)/ldet);
+ origLen = (int)((PiscesMath.hypot(xLen, yLen)*65536L)/ldet);
} else{
- long la = ((long)ly*m00 - (long)lx*m10)/ldet;
- long lb = ((long)ly*m01 - (long)lx*m11)/ldet;
- l = (int)PiscesMath.hypot(la, lb);
+ long origXLen = (m11*xLen - m01*yLen)/ldet;
+ long origYLen = (m00*yLen - m10*xLen)/ldet;
+ origLen = PiscesMath.hypot(origXLen, origYLen);
}
- if (l < d) {
+ if (origLen < d) {
goTo(x1, y1);
// Advance phase within current dash segment
- phase += l;
+ phase += origLen;
return;
}
- long t;
- int xsplit, ysplit;
+ long t; // the ratio (olddash/oldlinelength) * 2^16
+ long xsplit, ysplit;
// // For zero length dashses, SE appears to move 1/8 unit
// // in device space
// if (d == 0) {
@@ -220,11 +224,26 @@
// xsplit = (int)(dxsplit*65536.0);
// ysplit = (int)(dysplit*65536.0);
// } else {
- t = ((long)d << 16)/l;
- xsplit = x0 + (int)(t*(x1 - x0) >> 16);
- ysplit = y0 + (int)(t*(y1 - y0) >> 16);
+ t = ((long)d << 16)/origLen;
+ xsplit = x0 + (t * xLen >> 16);
+ ysplit = y0 + (t * yLen >> 16);
// }
- goTo(xsplit, ysplit);
+
+ // If java math had infinite precision, xsplit and ysplit would
+ // be guaranteed never to go outside of the range [MIN, MAX], but
+ // it isn't, and an overflow here would be hard to diagnose, so
+ // the efficiency trade off is worth it.
+ // Note: I have not proven mathematicaly there there is, indeed,
+ // some input that could would overflow here, so it might be
+ // nice to try to prove that overflow is impossible. If it is,
+ // the next 6 lines can be eliminated.
+ if (xsplit > MAX || xsplit < MIN) {
+ xsplit = (xsplit > MAX) ? MAX : MIN;
+ }
+ if (ysplit > MAX || ysplit < MIN) {
+ ysplit = (ysplit > MAX) ? MAX : MIN;
+ }
+ goTo((int)xsplit, (int)ysplit);
// Advance to next dash segment
idx = (idx + 1) % dash.length;
diff --git a/src/share/classes/sun/java2d/pisces/Stroker.java b/src/share/classes/sun/java2d/pisces/Stroker.java
--- a/src/share/classes/sun/java2d/pisces/Stroker.java
+++ b/src/share/classes/sun/java2d/pisces/Stroker.java
@@ -113,6 +113,10 @@
double m10_2_m11_2;
double m00_m10_m01_m11;
+ // An upper bound on the width the the transformed line and the
+ // untransformed line.
+ int wub;
+
/**
* Empty constructor. <code>setOutput</code> and
* <code>setParameters</code> must be called prior to calling any
@@ -227,6 +231,9 @@
pen_dy[i] = (int)(r*(dm10*cos + dm11*sin));
}
+ wub = (int)((lineWidth * PiscesMath.hypot((long)m01+m00, (long)m10+m11)) >> 16);
+ wub = (wub > lineWidth) ? wub : lineWidth;
+
prev = CLOSE;
rindex = 0;
started = false;
@@ -276,11 +283,12 @@
private boolean isCCW(int x0, int y0,
int x1, int y1,
int x2, int y2) {
- int dx0 = x1 - x0;
- int dy0 = y1 - y0;
- int dx1 = x2 - x1;
- int dy1 = y2 - y1;
- return (long)dx0*dy1 < (long)dy0*dx1;
+ // These need to be longs to avoid overflows.
+ long dx0 = (long)x1 - x0;
+ long dy0 = (long)y1 - y0;
+ long dx1 = (long)x2 - x1;
+ long dy1 = (long)y2 - y1;
+ return dx0*dy1 < dy0*dx1;
}
private boolean side(int x, int y, int x0, int y0, int x1, int y1) {
@@ -472,10 +480,35 @@
}
}
+ private static void makeOverflowSafe(int[] coords, int wub) {
+ int x = coords[0];
+ int y = coords[1];
+ if ((long)x + wub != x + wub) {
+ x -= wub;
+ }
+ if ((long)y + wub != y + wub) {
+ y -= wub;
+ }
+ if ((long)x - wub != x - wub) {
+ x += wub;
+ }
+ if ((long)y - wub != y - wub) {
+ y += wub;
+ }
+ coords[0] = x;
+ coords[1] = y;
+ }
public void moveTo(int x0, int y0) {
// System.out.println("Stroker.moveTo(" + x0/65536.0 + ", " + y0/65536.0 + ")");
+ // We can't move too close to the borders set by Integer.MAX_VALUE or
+ // MIN_VALUE, because when we try to draw an endcap, or even just adding
+ // offsets to (x0,y0), overflows might happen.
+ int[] coords = {x0, y0};
+ makeOverflowSafe(coords, wub);
+ x0 = coords[0]; y0 = coords[1];
+
if (lineToOrigin) {
// not closing the path, do the previous lineTo
lineToImpl(sx0, sy0, joinToOrigin);
@@ -531,6 +564,16 @@
int mx = offset[0];
int my = offset[1];
+ // Now we want to make sure that none of the operations involving line
+ // drawing will overflow. These include adding pen points to (x1,y1),
+ // drawing a square end cap, and adding/subtracting mx and my to x1
+ // and y1 (in the calls to emitLineTo). However, it does not protect
+ // against possible overflows when drawing miters.
+ // TODO: protect against possible overflows when drawing miters.
+ int[] coords = {x1, y1};
+ makeOverflowSafe(coords, wub);
+ x1 = coords[0]; y1 = coords[1];
+
if (!started) {
emitMoveTo(x0 + mx, y0 + my);
this.sx1 = x1;
