Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8176501: Method Shape.getBounds2D() incorrectly includes Bezier control points in bounding box #6227

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
0da9594
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 3, 2021
5a0e2bd
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 5, 2021
410cd6c
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 5, 2021
e617c72
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 9, 2021
b7ca69c
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 9, 2021
4b9d87d
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 11, 2021
8db26a2
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 12, 2021
af4c5f0
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 12, 2021
40bda06
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 12, 2021
8c5e772
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 16, 2021
0e2b529
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 16, 2021
b3e84a5
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 17, 2021
7680533
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Nov 19, 2021
2b7a132
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Dec 16, 2021
e346df0
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Dec 16, 2021
637c769
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Dec 16, 2021
fcf9d35
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Dec 16, 2021
d79f067
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Dec 16, 2021
faeb241
8176501: Method Shape.getBounds2D() incorrectly includes Bezier contr…
mickleness Dec 16, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 9 additions & 34 deletions src/java.desktop/share/classes/java/awt/geom/CubicCurve2D.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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
Expand Down
118 changes: 84 additions & 34 deletions src/java.desktop/share/classes/java/awt/geom/Path2D.java
Original file line number Diff line number Diff line change
Expand Up @@ -800,23 +800,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));
}

/**
Expand Down Expand Up @@ -1592,23 +1576,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));
}

/**
Expand Down Expand Up @@ -2129,6 +2097,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:
Copy link
Contributor

Choose a reason for hiding this comment

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

add explicitely the SEG_CLOSE case (skip = continue) before the default case (impossible case)

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;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

close the shortcut test
}

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}.
Expand Down
34 changes: 8 additions & 26 deletions src/java.desktop/share/classes/java/awt/geom/QuadCurve2D.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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
Expand Down
120 changes: 120 additions & 0 deletions src/java.desktop/share/classes/sun/awt/geom/Curve.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Loading