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

[Jim Graham]

Hi Laurent,

The message size filter is there for a reason - this list is not the 
place for large files and webrevs, those should be published and a 
pointer sent to the list.  Dalibor has been working on getting you the 
access you need to publish the webrevs in the right way.

The webrevs you sent also trip our firewall email filters so sending 
them via email wouldn't make them visible to us in any case (D'oh!).

I pinged Dalibor on how it is going to get you the accesses you need. 
Hopefully it will be in the next day or two...

			...jim

On 2/26/15 12:31 AM, Laurent Bourgès wrote:
> Jim,
>
> I made a Path2D patch but I sent the webrev by email.
> Did you got it ? Could you review it ?
>
> I got a size limit issue: Your message to 2d-dev awaits moderator approval !
>
> Looking forward having an openjdk id ro push my webrev to cr.openjdk.net
> <http://cr.openjdk.net> !
>
> Regards,
> Laurent
>
> Le 25 févr. 2015 02:06, "Jim Graham" <james.graham at oracle.com
> <mailto:james.graham at oracle.com>> a écrit :
>
>     Those changes were exactly what I was referring to.  I don't see why
>     we shouldn't make trimmed arrays when copying the shape.  I'm pretty
>     sure that the copy constructors are going to be overwhelmingly used
>     to make a protected copy of an existing shape/path2d which is likely
>     meant mostly for reading.  In particular, in the case of the return
>     value from createStrokedShape() I don't think the intention is to
>     create the shape and then scribble on it, the intent is to treat the
>     answer as if it were immutable - at least the 99.9% case - so I
>     think a perfectly sized shape is OK.
>
>     Be sure to add a test case that creates an empty Path2D, clones it,
>     copy constructs it (to both .Double() and .Float() variants) and
>     then tries to add new segments to it - to make sure that the array
>     growth code doesn't get ArrayIndexOutOfBounds exceptions due to
>     making assumptions about the lengths of the arrays (I eyeballed the
>     makeRoom() code and it looks good, but we should test it if we are
>     making arrays that are potentially zero length or very tiny)...
>
>                              ...jim
>
>     On 2/24/15 9:58 AM, Laurent Bourgčs wrote:
>
>         Jim,
>
>             Ah, wait, those constructors do copy the arrays without
>             having to iterate the segments and grow the arrays, but they
>             don't trim them.  I'm trying to remember if there was a
>             specific reason that we decided not to trim the arrays in
>             those constructors, but the only "advantage" I can think of
>             is that the new copy will have the same potential spare room
>             for growth that the original had.  But that is of
>             questionable value so we should probably just patch the
>             existing "construct from a Shape" constructors to trim the
>             arrays to the required length instead...
>
>
>         In marlin github, I have the patched Path2D class (not used at
>         runtime):
>
>         public Float(Shape s, AffineTransform at) {
>                       super(); // LBO: invoke empty constructor
>         explicitely !
>                       if (s instanceof Path2D) {
>                           Path2D p2d = (Path2D) s;
>                           setWindingRule(p2d.__windingRule);
>                           this.numTypes = p2d.numTypes;
>                           // LBO: trim arrays:
>                           this.pointTypes = Arrays.copyOf(p2d.pointTypes,
>         this.numTypes);
>         // this.pointTypes = Arrays.copyOf(p2d.pointTypes,
>         // p2d.pointTypes.length);
>                           this.numCoords = p2d.numCoords;
>                           this.floatCoords = p2d.cloneCoordsFloat(at);
>                       } else {
>                           PathIterator pi = s.getPathIterator(at);
>                           setWindingRule(pi.__getWindingRule());
>                           this.pointTypes = new byte[INIT_SIZE];
>                           this.floatCoords = new float[INIT_SIZE * 2];
>                           append(pi, false);
>                       }
>                   }
>
>         float[] cloneCoordsFloat(__AffineTransform at) {
>                       float ret[];
>                       if (at == null) {
>                           // LBO: trim arrays:
>                           ret = Arrays.copyOf(floatCoords, numCoords);
>         // ret = Arrays.copyOf(this.__floatCoords, this.floatCoords.length);
>                       } else {
>                           // LBO: trim arrays:
>                           ret = new float[numCoords];
>         // ret = new float[floatCoords.length];
>                           at.transform(floatCoords, 0, ret, 0, numCoords
>         / 2);
>                       }
>                       return ret;
>                   }
>
>         What do you think?
>
>         FYI my use case in createStrokedShape () is to allocate (and
>         reuse) a
>         path2d (4k arrays), fill it and then return a new path whose
>         arrays are
>         trimmed.
>
>         Laurent
>