[9] Review request: JDK-8170030 Code in Marlin-based rasterizers may have an off-by-1 bug

[Jim Graham]

Hi Laurent,

Normally we'd isolate fixes for different bugs even if they are just preparatory formatting changes on comments.

Some comments on the formatting changes:

line 35 - you should also upper-case the E

line 148 - I don't understand why this "float" was a formatting danger, but the one on line 328 isn't?  Not a big issue, 
but curious since I think the edges array does contain floats in it, doesn't it?

line 328 - (ints) => [ints] to match?  (Also would match line 148)


Comments on the bug fix:

line 1401 - why does maxY want to be bounds+1?
           - later this is used to calculate buckets_maxY
           - do we want an extra Y bucket for some reason?
           - and why only in case of spmaxY was clipped?
           - a comment about that would help
Other than that one odd case of using "+1" there, that whole block of code is just:
     spmaxY = min(edgeMaxY, _boundsMaxY)
which is much easier to read.
Perhaps we need an extra bucket only when there were clipped edges below the clip?

line 1456 - I can understand why we lose the +1, but not the min() with pmaxY?


NoAA comments are identical, though line numbers may be different...

			...jim

On 12/9/16 8:46 AM, Laurent Bourgès wrote:
> Please review this simple fix for MarlinFX:
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8170030
> webrev: http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-8170030.0/
>
> PS: I will provide asap a Marlin2D patch incorporating changes made in
> MarlinFX (including this one)
>
> Cheers,
> Laurent
>