[OpenJDK 2D-Dev] [OpenJDK Rasterizer] Path2D optimizations

Jim Graham james.graham at oracle.com
Sat Mar 7 01:43:20 UTC 2015


First, the test cases that I asked for were to verify that a path that 
was cloned was still operational.  I see no tests that verify that the 
returned objects will still function, only tests that examine internal 
data structures for a metric that may not be appropriate in the future.

On 3/6/15 11:06 AM, Phil Race wrote:
> So the test should go in the anonymous package and avoid accessing
> internals.
> It should be possible to use just public API  to verify the arrays  of a
> shape
> being cloned are trimmed .

We should not be testing if the cloned arrays are trimmed.  We don't 
care from a testing standpoint.  It is an implementation detail of a 
performance feature and we should not write any test that says "Object A 
should do its work by doing X, Y, and Z".

If we want to test performance then we should write a benchmark that 
tests how long it takes to do an operation.  That will likely be a 
manual test, though.  It should not be examining internals to take its 
measurements, though.  Benchmarks tend to only use public APIs as they 
should be benchmarking operations that Java programs encounter.

If we want to test if your fix will break something, then we need to 
test that.  Currently your tests do not do that.  We only need public 
APIs to do that.

What I would like to see in the test:

- Delete all of the existing tests in there.  None of them are 
appropriate to our testing goals.

- Write tests that create paths in various forms and run them through 
the following sub-tests:
      - each of the following should be tested on a fresh clone...
      - get a PathIterator and iterate it through until it is done
        (optional - compare to the expected segments that were in the 
original)
      - get a flattened PathIterator using "getPathIterator(flatness,)" 
and make sure it works
        (optional - compare to original path if the original was already 
flat)
        (but, also run it on a path with curves to make sure flattening 
engine didn't break)
      - add various kinds of segments to the cloned path
      - get the bounds of the cloned path
      - use the transform() method on the cloned path
      - call intersects(point), intersects(rect) and contains(rect) on a 
cloned path
      - call getCurrentPoint() on the cloned path

- Perform each of the above tests on a (set of) clone(s) of the 
following paths:
      - each of the following executed on both a Float and a Double 
instance...
      - an empty path
      - a path with just a moveto
      - a path with a moveto+some lines
      - a path with a moveto+some curves

The way I tend to write tests like this is to use cascading methods like:

void testEqual(Path2D pathA, Path2D path2D pathB) {
     // Grab 2 path iterators and test for equality with float coords[]
}

void test(Path2D p2d, boolean isEmpty) {
     testEqual(new Path2D.Float(p2d), p2d);
     testEqual(new Path2D.Double(p2d), p2d);
     testEqual(new GeneralPath(p2d), p2d);

     testIterator(new Path2D.Float(p2d));
     testIterator(new Path2D.Double(p2d));
     testIterator((Path2D) p2d.clone());

     testFlattening(... 3 clone variants ...);

     testAddMove(... 3 clone variants ...);
     // These should expect exception if empty
     testAddLine(... 3 clone variants ..., isEmpty);
     testAddQuad(... 3 clone variants ..., isEmpty);
}

interface PathFactory {
     Path2D makePath();
}

void test(PathFactory pf) {
     test(pf.makePath(), true);
     test(addMove(pf.makePath()), false);
     test(addLines(pf.makePath()), false);
     test(addCurves(pf.makePath())), false);
}

@Test
void testFloatPaths() {
     test(() -> new Path2D.Float());
}

@Test
void testDoublePaths() {
     test(() -> new Path2D.Double());
}

@Test
void testGeneralPath() {
     test(() -> new GeneralPath());
}

			...jim



More information about the 2d-dev mailing list