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

[Laurent Bourgès]

Dear Jim,

Here is the new webrev:
http://cr.openjdk.java.net/~lbourges/path2D/Path2D.3/

> Looks great.

Thanks, I hope this patch is ready to go in.

> - A couple of methods have "/**" even though the comment is just
informational to the programmer and not representing an API.  It looks like
"/**" is the standard practice for the @test comment at the top of the
class even though jtreg isn't spec'd to require it, but elsewhere it's just
noise.

I removed javadoc and superfluous comments.

> - In general tests shouldn't print out a lot of noise (see "verbosity" in
http://openjdk.java.net/jtreg/writetests.html).  Ideally an automated test
would produce no output if it is successful, but some of our automated
tests do print out a short summary (mostly if they were stress-bashers).
Some of them can be run in "verbose mode" to print out details in case an
engineer is diagnosing a failure and wants to know what failed and why - I
saw a few in the AffineTransform directory that had a static boolean
verbose that was set if there was an argument handed to main() for
instance.  A verbose option can help when running the test in manual mode
for debugging.

Ok, I added the verbose flag as indicated.

> - I'd add a testAddCubic() and testAddClose() to the tests.  It looks
like the "testAddQuad" method does add both types of curve, but the cubic
curve is added after the quad so we don't get a case that adds a cubic to a
freshly cloned path - which is the main state in question to be tested here.

Done.

> - equalsArray has some tests in it that don't make a lot of sense since
you created the arrays and so you know that they aren't == or null or the
wrong length.  I'd rather see a "number of coords" parameter added to the
method, get rid of the ==/null/length tests and only test the number of
coords that were appropriate to the segment type.  Technically you are
testing entries in the arrays that weren't required to be equal by the
PathIterator contract, but in practice it would be odd for the iterator to
scribble random values into the unused entries (not disallowed, but odd).

Fixed: I only check the correct subpart according to the segment type.

> - What am I missing on the difference between the two test files?  Can
they be combined into a single test file?

Big one: I figured out a bug in Path2d happening with 0 capacity:
rectCrossing and ptCrossing fail.

Finally I merged both tests into the same bugid that may be backported
later to jdk8...

Regards,
Laurent
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/graphics-rasterizer-dev/attachments/20150329/c1e17cc7/attachment-0001.html>