[OpenJDK 2D-Dev] X11 uniform scaled wide lines and dashed lines; STROKE_CONTROL in Pisces

[Jim Graham]

HI Denis,

I'm just now getting down to the nitty gritty of your webrevs (sigh).

On 10/6/2010 1:36 PM, Denis Lila wrote:
>
> webrev:
> http://icedtea.classpath.org/~dlila/webrevs/noflatten/webrev/

PiscesRenderingEngine.java:
line 278 - the det calculation is missing "b".

line 296 - is there an "epsilon" that we can use?  "==" with floating 
point often fails with calculations.

line 308 - miterlimit is a ratio of lengths and should not need to be 
scaled.

line 332 - I think you can use a null transform for the same result.

line 338 - null here too

TransformingPolyIOHelper should be in its own file - we consider more 
than one class per file to be bad form these days, especially if the 
class is used outside of that file.

I'm a little troubled by how the PolyIOHelper fits into the design. 
It's odd to talk to the same object for both input and output.  I have 
some ideas there, but I think I'll leave it for a followon email and effort.

Dasher.java:

lines 110,111 - shouldn't you check if there are any "first segments" 
before writing the extra move?

lines 150-152 - starting should be left true until you reach the end of 
the dash, no?  Otherwise you only hold back the starting segments up 
until the first "piece" of a curve.  Everything should be held back 
until you reach an "off" piece.  I don't think the logic for these 
variables is right yet.  Here is what I see:

    boolean needsMoveto;

in moveTo and pathDone:
     if (firstSegBuf is not empty) {
         output moveto(sx,sy)
         output firstSegs
     }
     needsMoveto = true;  // not needed in pathDone

in goTo() {
     if (ON) {
         if (starting) {
             store it in firstSegBuf
         } else {
             if (needsMoveto) {
                 output moveto(x0,y0);
                 needsMoveto = false;
             }
             send it to output
         }
     } else {
         starting = false;
         needsMoveto = true;
         // nothing goes to output
     }
}

and in ClosePath:
     lineToImpl(sx, sy, LINE);
     if (firstSegBuf is not empty) {
         if (!ON) {  // Or "if (needsMoveto)"
             output moveTo(sx, sy)
         }
         output firstSegs
     }

I don't see a need for firstDashOn or fullCurve

line 228 - Lazy allocate lc?  Polygons, rectangles, and lines won't need 
it to be dashed (though dashing is already expensive enough it might not 
be noticeable, still waste is waste and there is only one piece of code 
that uses lc so it is easy to protect with a lazy allocation)

line 235 - is there a reason to output a curve of 0 length?  lines of 0 
length are omitted...

line 257 - shouldn't the left and right indices always be at 0 and 
type-curCurveoff?  It looks like after the first time through this loop 
you are storing the right half on top of the left half (see line 262)? 
That would output odd values if any curve gets subdivided more than 
once, though, right?

line 289 - LenComputer always allocates maxcurves segements which is 
8*1024 words (32K) + object overhead * 1024 + 2 more arrays of 1025 
words.  That's a lot of memory for the worst case scenario.  It might be 
nice to come back to this and have it be more dynamic (and it is more of 
a push to have the "lc" variable be lazily allocated above).  Also, if 
you are clever enough, you never need storage for more than about 10 
(maybe 11) curves even if you produce 1024 t's and len's - due to the 
recursive nature of the subdivision that leaves one half dormant while 
the other half is explored.

line 347,352 - it might be cleaner to have the calling function keep 
track of how far into the curve you are and communicate this to the 
method so it doesn't have to clobber its data based on an assumption of 
how the calling function is structured.  But, this arrangement works 
fine for the current purposes and you do have a "TODO" comment in there 
about this.

Stroker.java:

line 129 - proof that miterLimit does not need to be scaled... ;-)

I'm going to send this buffer of comments off now and continue on with 
Stroker.java in a future email...

			...jim