Thanks for the reviw.
On 04/23/2014 02:37 PM, Jim Graham wrote:
Shouldn't Order2, lines 50,77 be "<Curve>"?
Ditto for Order3, lines 56,108?
I don't think we ever use these methods with any other type of Vector,
but I guess I can see that the choice you made might be a more general
choice.
Right, I took a more general approach.
This concludes my review of the geom files per Phil's request. They
look fine, other than my suggestions for returning an empty curve list
from pruneEdges and the above comments...
I changed to return empty curve list, but not static one as there is no
immutable Vector I can use without implementing one.
While I see the result is private member curves in java.awt.geom.Area, I
am not 100% certain that this cannot be leaked. I am afraid it get
tainted somehow thus I make a new instance just to be cautious. However,
if you feel strongly it's fine to return a static instance, we can do that.
Following is the diff to address your comments, let me know if other
changes are needed.
Cheers,
Henry
diff --git a/src/share/classes/sun/awt/geom/AreaOp.java
b/src/share/classes/sun/awt/geom/AreaOp.java
--- a/src/share/classes/sun/awt/geom/AreaOp.java
+++ b/src/share/classes/sun/awt/geom/AreaOp.java
@@ -198,11 +198,8 @@
private Vector<Curve> pruneEdges(Vector<Edge> edges) {
int numedges = edges.size();
if (numedges < 2) {
- Vector<Curve> rt = new Vector<>();
- for (Edge edge: edges) {
- rt.add(edge.getCurve());
- }
- return rt;
+ // empty vector is expected with less than 2 edges
+ return new Vector<>();
}
Edge[] edgelist = edges.toArray(new Edge[numedges]);
Arrays.sort(edgelist, YXTopComparator);
diff --git a/src/share/classes/sun/awt/geom/Order2.java
b/src/share/classes/sun/awt/geom/Order2.java
--- a/src/share/classes/sun/awt/geom/Order2.java
+++ b/src/share/classes/sun/awt/geom/Order2.java
@@ -47,7 +47,7 @@
private double ycoeff1;
private double ycoeff2;
- public static void insert(Vector<? super Order2> curves, double tmp[],
+ public static void insert(Vector<Curve> curves, double tmp[],
double x0, double y0,
double cx0, double cy0,
double x1, double y1,
@@ -74,7 +74,7 @@
tmp[i1 + 4], tmp[i1 + 5], direction);
}
- public static void addInstance(Vector<? super Order2> curves,
+ public static void addInstance(Vector<Curve> curves,
double x0, double y0,
double cx0, double cy0,
double x1, double y1,
diff --git a/src/share/classes/sun/awt/geom/Order3.java
b/src/share/classes/sun/awt/geom/Order3.java
--- a/src/share/classes/sun/awt/geom/Order3.java
+++ b/src/share/classes/sun/awt/geom/Order3.java
@@ -53,7 +53,7 @@
private double ycoeff2;
private double ycoeff3;
- public static void insert(Vector<? super Order3> curves, double tmp[],
+ public static void insert(Vector<Curve> curves, double tmp[],
double x0, double y0,
double cx0, double cy0,
double cx1, double cy1,
@@ -105,7 +105,7 @@
}
}
- public static void addInstance(Vector<? super Order3> curves,
+ public static void addInstance(Vector<Curve> curves,
double x0, double y0,
double cx0, double cy0,
double cx1, double cy1,
Cheers,
Henry
...jim
On 4/7/14 1:46 PM, Henry Jen wrote:
Hi,
Please review the webrev cleans up raw and unchecked warnings in sun.awt,
http://cr.openjdk.java.net/~henryjen/jdk9/8039342/0/webrev/
The following changes in AreaOp::pruneEdges() is particular worth
attention, when numedges < 2, two different type are mixed up in the
past with use of rawtypes; However, I think it could only work if the
Vector is empty?
Cheers,
Henry
@@ -193,16 +193,20 @@
}
return 1;
}
};
- private Vector pruneEdges(Vector edges) {
+ private Vector<Curve> pruneEdges(Vector<Edge> edges) {
int numedges = edges.size();
if (numedges < 2) {
- return edges;
+ Vector<Curve> rt = new Vector<>();
+ for (Edge edge: edges) {
+ rt.add(edge.getCurve());
}
- Edge[] edgelist = (Edge[]) edges.toArray(new Edge[numedges]);
+ return rt;
+ }
+ Edge[] edgelist = edges.toArray(new Edge[numedges]);
Arrays.sort(edgelist, YXTopComparator);
if (false) {
System.out.println("pruning: ");
for (int i = 0; i < numedges; i++) {
System.out.println("edgelist["+i+"] = "+edgelist[i]);
