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

2016-12-09 Thread 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



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

2016-12-09 Thread Laurent Bourgès
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