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

[Jim Graham]

Hi Denis,

I think the first version is a better choice for now since you said that 
the performance difference isn't noticeable.  I think the lower level 
flattening might look a little different if we ever decide to upgrade 
the pipeline to deal with curves.  In particular, you are still 
flattening above the dashing/stroking code and I think the flattening 
should be done below that code (i.e. in Renderer).

So, I'd go with the first one with the following comments:

- You indent by 8 spaces in a few places.  Is that a tabs vs. spaces 
issue?  We try to stick to 4 space indentations with no tabs for 
consistentcy.

- I'd make the internal error message a little less personal. ;-) 
"normalization not needed in OFF mode" or something.

- lines 362,363 - you don't need to set cur_adjust variables here, they 
are already being set below.

Other than that, it looks good to go...

			...jim

Denis Lila wrote:
> Hi Jim.
> 
> So, I have the nicer webrevs.
> FlatteningIterator version:
> http://icedtea.classpath.org/~dlila/webrevs/fpWithStrokeControl/webrev/
> 
> Pisces flattening version:
> http://icedtea.classpath.org/~dlila/webrevs/fpWithSCandPiscesFlattening/webrev/
> 
> I dealt with the issue of handling OFF by just not accepting it as an input.
> After all, a normalizing iterator only needs to be created, and is only created
> if the normalization mode is not OFF.
> 
> Thanks,
> Denis.
> 
> ----- "Jim Graham" <james.graham at oracle.com> wrote:
> 
>> Hi Denis,
>>
>> I'll wait for some clean webrevs once you get the float stuff in for a
>>
>> final review.  I did take a really quick look and thought that a
>> better 
>> way to handle "OFF" would be to set rval to -1 and then check "rval <
>> 0" 
>> as the (quicker) test for OFF in the currentSegment() method.  Does
>> that 
>> make sense?
>>
>> In any case, let's wait for cleaner webrevs to go further on this 
>> (hopefully in a day or so?)...
>>
>> 			...jim
>>
>> On 8/5/2010 8:06 AM, Denis Lila wrote:
>>> Hi Jim.
>>>
>>> I made all the suggested changes.
>>> Links:
>>>
>> http://icedtea.classpath.org/~dlila/webrevs/fpWithStrokeControl/webrev/
>> http://icedtea.classpath.org/~dlila/webrevs/fpWithSCandPiscesFlattening/webrev/
>>> Thanks,
>>> Denis.
>>>
>>> ----- "Jim Graham"<james.graham at oracle.com>  wrote:
>>>
>>>> Hi Denis,
>>>>
>>>> First, comments on the high level normalizer (Normalizing
>> iterator):
>>>> - If there is no normalization going on, I would use the Shape's
>> own
>>>> flattening (i.e. getPathIterator(at, flat)).  The reason being
>> that
>>>> some
>>>> shapes may know how to flatten themselves better, or faster, than
>> a
>>>> Flattening Iterator.  In particular, rectangles and polygons would
>>>> simply ignore the argument and save themselves the cost of
>> wrapping
>>>> with
>>>> an extra iterator.  This would probably only be a big issue for
>> very
>>>> long Polygons.
>>>>
>>>> - Line 331 - the initializations to NaN aren't necessary as far as
>> I
>>>> can
>>>> tell...?
>>>>
>>>> - Rather than saving "mode" in the normalizing iterator, how about
>>>> saving 2 constants: (0.0, 0.5) for AA and (0.25, 0.25) for non-AA
>> and
>>>> then simply add those constants in rather than having to have the
>>>> conditional with the 2 different equations?
>>>>
>>>> 			...jim