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

[Phil Race]

Hi,

Two more things regarding the test
1. It was pointed out that you are using GPL+CP .. tests just use GPL
2. There's no @bug tag the test. In fact there's no bug ID here at all :-)

A bug ID is needed in order to push.
You have a JBS account now so could have created it yourself .. but
by the time I remembered that I'd already created it for you :-
https://bugs.openjdk.java.net/browse/JDK-8074587

But I did assign it to you :-)

-phil.

On 03/06/2015 02:03 PM, Laurent Bourgès wrote:
> Phil,
>
> Here is a new webrev:
> http://cr.openjdk.java.net/~lbourges/webrev_Path2D_1/ 
> <http://cr.openjdk.java.net/%7Elbourges/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/335de143/attachment-0001.html>