[OpenJDK Rasterizer] [OpenJDK 2D-Dev] Openjdk java2d rasterizer JEP for pisces (marlin) enhancements ?

[Laurent Bourgès]

Phil,

Here is a new webrev:
http://cr.openjdk.java.net/~lbourges/webrev_Path2D_1/

See my comments below:

> you placed the test in the java.awt.geom package.
>>
>>   25 package java.awt.geom;
>>
>> and are accessing internals of that package.
>>
>> In jigsaw/modular mode that won't even compile.
>>
>
>  Ok it is annoying:
> as all Path2D fields are package protected, I designed the test using
> direct access to any fields ...
>
>
>> 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 .
>>
>
>  No, it is not possible to use Shape API to access arrays nor fields
> (numTypes ...):
> only getPathIterator() could give me data but it won't tell me if the
> underlying arrays or fields are correct.
>
>
> That is true ..
>
> Well, if you need it to be in java.awt.geom, I think even today you'll
> find it won't work
> unless you jump through some jtreg hoops to install it on the
> bootclasspath.
> I think its something like "@run main/othervm -Xbootclasspath/a:. "
> And later in the modular JDK it will need to be modified again.
>
> I'd say either update the test to work with jtreg today - and test it to
> be sure,
> or provide the test without an @test tag, or with an @ignore tag, so
> people can
> still manually verify it but the harness won't run it.
>

I removed the @test tag but added comments indicating to run the test
manually.

>
>
>  Maybe I could use introspection to getDeclaredField(name) and
> setAccessible(true) to get internal data.
>
>
> That won't work either. So maybe this is a "noreg-hard" or "noreg-cleanup"
> bug.
> We add those labels to the JBS/JIRA bug when something isn't testable.
>

Nevermind.


>    Any idea or utility class I could use
>
>
>
>> Why is it necessary to explicitly add the call to super(); ?
>>
>> 223             super();
>>
>
>  I agree it is not necessary but it explicitely says that I use the empty
> constructor:
>
>     /**
>      * Constructs a new empty {@code Path2D} object.
>      * It is assumed that the package sibling subclass that is
>      * defaulting to this constructor will fill in all values.
>      *
>      * @since 1.6
>      */
>     /* private protected */
>     Path2D() {
>     }
>
>
> If we all did this, all of the time, there'd be a lot of extra lines in
> the code, that the compiler
> would fill in for us anyway.
>

I removed the superfluous super() calls.

Thanks for your review,

Laurent
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/graphics-rasterizer-dev/attachments/20150306/59ec5978/attachment.html>