Skip to content
Permalink
Browse files
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
…ol points in bounding box

Reviewed-by: prr
  • Loading branch information
mickleness authored and prrace committed Apr 27, 2022
1 parent 05dac5a commit 8a16842b4e906b2eede0c01914f41010cabc51c2
Show file tree
Hide file tree
Showing 6 changed files with 599 additions and 119 deletions.
@@ -311,23 +311,6 @@ public void setCurve(float x1, float y1,
this.y2 = y2;
}

/**
* {@inheritDoc}
* @since 1.2
*/
public Rectangle2D getBounds2D() {
float left = Math.min(Math.min(x1, x2),
Math.min(ctrlx1, ctrlx2));
float top = Math.min(Math.min(y1, y2),
Math.min(ctrly1, ctrly2));
float right = Math.max(Math.max(x1, x2),
Math.max(ctrlx1, ctrlx2));
float bottom = Math.max(Math.max(y1, y2),
Math.max(ctrly1, ctrly2));
return new Rectangle2D.Float(left, top,
right - left, bottom - top);
}

/**
* Use serialVersionUID from JDK 1.6 for interoperability.
*/
@@ -558,23 +541,6 @@ public void setCurve(double x1, double y1,
this.y2 = y2;
}

/**
* {@inheritDoc}
* @since 1.2
*/
public Rectangle2D getBounds2D() {
double left = Math.min(Math.min(x1, x2),
Math.min(ctrlx1, ctrlx2));
double top = Math.min(Math.min(y1, y2),
Math.min(ctrly1, ctrly2));
double right = Math.max(Math.max(x1, x2),
Math.max(ctrlx1, ctrlx2));
double bottom = Math.max(Math.max(y1, y2),
Math.max(ctrly1, ctrly2));
return new Rectangle2D.Double(left, top,
right - left, bottom - top);
}

/**
* Use serialVersionUID from JDK 1.6 for interoperability.
*/
@@ -1509,6 +1475,15 @@ public boolean contains(Rectangle2D r) {
return contains(r.getX(), r.getY(), r.getWidth(), r.getHeight());
}


/**
* {@inheritDoc}
* @since 1.2
*/
public Rectangle2D getBounds2D() {
return Path2D.getBounds2D(getPathIterator(null));
}

/**
* {@inheritDoc}
* @since 1.2
@@ -802,23 +802,7 @@ public final void transform(AffineTransform at) {
* @since 1.6
*/
public final synchronized Rectangle2D getBounds2D() {
float x1, y1, x2, y2;
int i = numCoords;
if (i > 0) {
y1 = y2 = floatCoords[--i];
x1 = x2 = floatCoords[--i];
while (i > 0) {
float y = floatCoords[--i];
float x = floatCoords[--i];
if (x < x1) x1 = x;
if (y < y1) y1 = y;
if (x > x2) x2 = x;
if (y > y2) y2 = y;
}
} else {
x1 = y1 = x2 = y2 = 0.0f;
}
return new Rectangle2D.Float(x1, y1, x2 - x1, y2 - y1);
return getBounds2D(getPathIterator(null));
}

/**
@@ -1594,23 +1578,7 @@ public final void transform(AffineTransform at) {
* @since 1.6
*/
public final synchronized Rectangle2D getBounds2D() {
double x1, y1, x2, y2;
int i = numCoords;
if (i > 0) {
y1 = y2 = doubleCoords[--i];
x1 = x2 = doubleCoords[--i];
while (i > 0) {
double y = doubleCoords[--i];
double x = doubleCoords[--i];
if (x < x1) x1 = x;
if (y < y1) y1 = y;
if (x > x2) x2 = x;
if (y > y2) y2 = y;
}
} else {
x1 = y1 = x2 = y2 = 0.0;
}
return new Rectangle2D.Double(x1, y1, x2 - x1, y2 - y1);
return getBounds2D(getPathIterator(null));
}

/**
@@ -2131,6 +2099,88 @@ public final Rectangle getBounds() {
return getBounds2D().getBounds();
}

/**
* Returns a high precision bounding box of the specified PathIterator.
* <p>
* This method provides a basic facility for implementors of the {@link Shape} interface to
* implement support for the {@link Shape#getBounds2D()} method.
* </p>
*
* @param pi the specified {@code PathIterator}
* @return an instance of {@code Rectangle2D} that is a high-precision bounding box of the
* {@code PathIterator}.
* @see Shape#getBounds2D()
*/
static Rectangle2D getBounds2D(final PathIterator pi) {
final double[] coeff = new double[4];
final double[] deriv_coeff = new double[3];

final double[] coords = new double[6];

// bounds are stored as {leftX, rightX, topY, bottomY}
double[] bounds = null;
double lastX = 0.0;
double lastY = 0.0;
double endX = 0.0;
double endY = 0.0;

for (; !pi.isDone(); pi.next()) {
final int type = pi.currentSegment(coords);
switch (type) {
case PathIterator.SEG_MOVETO:
if (bounds == null) {
bounds = new double[] { coords[0], coords[0], coords[1], coords[1] };
}
endX = coords[0];
endY = coords[1];
break;
case PathIterator.SEG_LINETO:
endX = coords[0];
endY = coords[1];
break;
case PathIterator.SEG_QUADTO:
endX = coords[2];
endY = coords[3];
break;
case PathIterator.SEG_CUBICTO:
endX = coords[4];
endY = coords[5];
break;
case PathIterator.SEG_CLOSE:
default:
continue;
}

if (endX < bounds[0]) bounds[0] = endX;
if (endX > bounds[1]) bounds[1] = endX;
if (endY < bounds[2]) bounds[2] = endY;
if (endY > bounds[3]) bounds[3] = endY;

switch (type) {
case PathIterator.SEG_QUADTO:
Curve.accumulateExtremaBoundsForQuad(bounds, 0, lastX, coords[0], coords[2], coeff, deriv_coeff);
Curve.accumulateExtremaBoundsForQuad(bounds, 2, lastY, coords[1], coords[3], coeff, deriv_coeff);
break;
case PathIterator.SEG_CUBICTO:
Curve.accumulateExtremaBoundsForCubic(bounds, 0, lastX, coords[0], coords[2], coords[4], coeff, deriv_coeff);
Curve.accumulateExtremaBoundsForCubic(bounds, 2, lastY, coords[1], coords[3], coords[5], coeff, deriv_coeff);
break;
default:
break;
}

lastX = endX;
lastY = endY;
}
if (bounds != null) {
return new Rectangle2D.Double(bounds[0], bounds[2], bounds[1] - bounds[0], bounds[3] - bounds[2]);
}

// there's room to debate what should happen here, but historically we return a zeroed
// out rectangle here. So for backwards compatibility let's keep doing that:
return new Rectangle2D.Double();
}

/**
* Tests if the specified coordinates are inside the closed
* boundary of the specified {@link PathIterator}.
@@ -238,19 +238,6 @@ public void setCurve(float x1, float y1,
this.y2 = y2;
}

/**
* {@inheritDoc}
* @since 1.2
*/
public Rectangle2D getBounds2D() {
float left = Math.min(Math.min(x1, x2), ctrlx);
float top = Math.min(Math.min(y1, y2), ctrly);
float right = Math.max(Math.max(x1, x2), ctrlx);
float bottom = Math.max(Math.max(y1, y2), ctrly);
return new Rectangle2D.Float(left, top,
right - left, bottom - top);
}

/**
* Use serialVersionUID from JDK 1.6 for interoperability.
*/
@@ -428,19 +415,6 @@ public void setCurve(double x1, double y1,
this.y2 = y2;
}

/**
* {@inheritDoc}
* @since 1.2
*/
public Rectangle2D getBounds2D() {
double left = Math.min(Math.min(x1, x2), ctrlx);
double top = Math.min(Math.min(y1, y2), ctrly);
double right = Math.max(Math.max(x1, x2), ctrlx);
double bottom = Math.max(Math.max(y1, y2), ctrly);
return new Rectangle2D.Double(left, top,
right - left, bottom - top);
}

/**
* Use serialVersionUID from JDK 1.6 for interoperability.
*/
@@ -1335,6 +1309,14 @@ public boolean contains(Rectangle2D r) {
return contains(r.getX(), r.getY(), r.getWidth(), r.getHeight());
}

/**
* {@inheritDoc}
* @since 1.2
*/
public Rectangle2D getBounds2D() {
return Path2D.getBounds2D(getPathIterator(null));
}

/**
* {@inheritDoc}
* @since 1.2
@@ -712,6 +712,126 @@ public static int rectCrossingsForCubic(int crossings,
return crossings;
}

/**
* Accumulate the quadratic extrema into the pre-existing bounding array.
* <p>
* This method focuses on one dimension at a time, so to get both the x and y
* dimensions you'll need to call this method twice.
* </p>
* <p>
* Whenever we have to examine cubic or quadratic extrema that change our bounding
* box: we run the risk of machine error that may produce a box that is slightly
* too small. But the contract of {@link Shape#getBounds2D()} says we should err
* on the side of being too large. So to address this: we'll apply a margin based
* on the upper limit of numerical error caused by the polynomial evaluation (horner
* scheme).
* </p>
*
* @param bounds the bounds to update, which are expressed as: { minX, maxX }
* @param boundsOffset the index in boundsof the minimum value
* @param x1 the starting value of the bezier curve where t = 0.0
* @param ctrlX the control value of the bezier curve
* @param x2 the ending value of the bezier curve where t = 1.0
* @param coeff an array of at least 3 elements that will be overwritten and reused
* @param deriv_coeff an array of at least 2 elements that will be overwritten and reused
*/
public static void accumulateExtremaBoundsForQuad(double[] bounds, int boundsOffset, double x1, double ctrlX, double x2, double[] coeff, double[] deriv_coeff) {
if (ctrlX < bounds[boundsOffset] ||
ctrlX > bounds[boundsOffset + 1]) {

final double dx21 = ctrlX - x1;
coeff[2] = (x2 - ctrlX) - dx21; // A = P3 - P0 - 2 P2
coeff[1] = 2.0 * dx21; // B = 2 (P2 - P1)
coeff[0] = x1; // C = P1

deriv_coeff[0] = coeff[1];
deriv_coeff[1] = 2.0 * coeff[2];

final double t = -deriv_coeff[0] / deriv_coeff[1];
if (t > 0.0 && t < 1.0) {
final double v = coeff[0] + t * (coeff[1] + t * coeff[2]);

// error condition = sum ( abs (coeff) ):
final double margin = Math.ulp(Math.abs(coeff[0])
+ Math.abs(coeff[1]) + Math.abs(coeff[2]));

if (v - margin < bounds[boundsOffset]) {
bounds[boundsOffset] = v - margin;
}
if (v + margin > bounds[boundsOffset + 1]) {
bounds[boundsOffset + 1] = v + margin;
}
}
}
}

/**
* Accumulate the cubic extrema into the pre-existing bounding array.
* <p>
* This method focuses on one dimension at a time, so to get both the x and y
* dimensions you'll need to call this method twice.
* </p>
* <p>
* Whenever we have to examine cubic or quadratic extrema that change our bounding
* box: we run the risk of machine error that may produce a box that is slightly
* too small. But the contract of {@link Shape#getBounds2D()} says we should err
* on the side of being too large. So to address this: we'll apply a margin based
* on the upper limit of numerical error caused by the polynomial evaluation (horner
* scheme).
* </p>
*
* @param bounds the bounds to update, which are expressed as: { minX, maxX }
* @param boundsOffset the index in boundsof the minimum value
* @param x1 the starting value of the bezier curve where t = 0.0
* @param ctrlX1 the first control value of the bezier curve
* @param ctrlX1 the second control value of the bezier curve
* @param x2 the ending value of the bezier curve where t = 1.0
* @param coeff an array of at least 3 elements that will be overwritten and reused
* @param deriv_coeff an array of at least 2 elements that will be overwritten and reused
*/
public static void accumulateExtremaBoundsForCubic(double[] bounds, int boundsOffset, double x1, double ctrlX1, double ctrlX2, double x2, double[] coeff, double[] deriv_coeff) {
if (ctrlX1 < bounds[boundsOffset] ||
ctrlX1 > bounds[boundsOffset + 1] ||
ctrlX2 < bounds[boundsOffset] ||
ctrlX2 > bounds[boundsOffset + 1]) {
final double dx32 = 3.0 * (ctrlX2 - ctrlX1);
final double dx21 = 3.0 * (ctrlX1 - x1);
coeff[3] = (x2 - x1) - dx32; // A = P3 - P0 - 3 (P2 - P1) = (P3 - P0) + 3 (P1 - P2)
coeff[2] = (dx32 - dx21); // B = 3 (P2 - P1) - 3(P1 - P0) = 3 (P2 + P0) - 6 P1
coeff[1] = dx21; // C = 3 (P1 - P0)
coeff[0] = x1; // D = P0

deriv_coeff[0] = coeff[1];
deriv_coeff[1] = 2.0 * coeff[2];
deriv_coeff[2] = 3.0 * coeff[3];

// reuse this array, give it a new name for readability:
final double[] tExtrema = deriv_coeff;

// solveQuadratic should be improved to get correct t extrema (1 ulp):
final int tExtremaCount = QuadCurve2D.solveQuadratic(deriv_coeff, tExtrema);
if (tExtremaCount > 0) {
// error condition = sum ( abs (coeff) ):
final double margin = Math.ulp(Math.abs(coeff[0])
+ Math.abs(coeff[1]) + Math.abs(coeff[2])
+ Math.abs(coeff[3]));

for (int i = 0; i < tExtremaCount; i++) {
final double t = tExtrema[i];
if (t > 0.0 && t < 1.0) {
final double v = coeff[0] + t * (coeff[1] + t * (coeff[2] + t * coeff[3]));
if (v - margin < bounds[boundsOffset]) {
bounds[boundsOffset] = v - margin;
}
if (v + margin > bounds[boundsOffset + 1]) {
bounds[boundsOffset + 1] = v + margin;
}
}
}
}
}
}

public Curve(int direction) {
this.direction = direction;
}

1 comment on commit 8a16842

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.