JDK-8170030 : Code in Marlin-based rasterizers may have an off-by-1 bug (causing extra work, no failures)
  • Type: Bug
  • Component: javafx
  • Sub-Component: graphics
  • Affected Version: 9
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2016-11-18
  • Updated: 2023-09-22
  • Resolved: 2016-12-12
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9Fixed
Related Reports
Relates :  
JDK-8169270 - Leverage new Java2D Marlin rasterizer for JavaFX
Description
The following code appears in the 3 versions of the Marlin rasterizer endRendering() method (Marlin2D, MarlinFX_AA, MarlinFX_NoAA):

1310         // bounds as inclusive intervals
1311         final int spminX = FloatMath.max(FloatMath.ceil_int(edgeMinX - 0.5f), boundsMinX);
1312         final int spmaxX = FloatMath.min(FloatMath.ceil_int(edgeMaxX - 0.5f), boundsMaxX - 1);

The spminX uses inclusive values for both the edgeMinX and boundsMinX values, but the spmaxX uses a mixture of inclusive/exclusive values.  "ceil(x-0.5)" on a right edge is an exclusive value (that coordinate is not rendered), but boundsMaxX-1 is an inclusive value.

I don't think this leads to a bug since we clip against the "inclusive edge" of the clip box so we never process beyond the addressable pixels in the destination, but it may lead to processing one additional provably-0-coverage sample for each row since the edgeMaxX value represents the last recorded edge sample.

If this is indeed an issue (causing us to process an extra column), then a similar fix would be needed in 2 files in the MarlinFX code and 1 file in the Marlin2D code - a separate related bug would need to be filed against Marlin2D since they are in separate projects.
Comments
http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/4412f7597150
12-12-2016
+1 to the second webrev.
12-12-2016
Laurent sent another webrev to review to the list as: http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-8170030.1/
12-12-2016
Please review this simple patch (inspired from openpisces): http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-8170030.0/ Changes: - use half-open intervals in endRendering() that simplifies a lot the logic, thanks to Jim's comments - minor syntax changes (to prepare the Double pipeline) in comments / indentation Review thread: http://mail.openjdk.java.net/pipermail/openjfx-dev/2016-December/020066.html
09-12-2016
I succeeded in understanding the issue and the current openpisces implementation on that specific problem. I already fixed it in both MarlinFX / Marlin2D and that works perfectly: actually it is slightly faster as Marlin does not process an extra subpixel per scanline. Will submit a webrev asap
09-12-2016
The interior inclusiveness policy includes points along the left and top edges of a shape, but not the points along the right and bottom edges. Any calculation which produces a value that will feed into that interior inclusiveness algorithm thus produces a half-open value. ceil(xy - 0.5) is performed on all edges whether or not they are top, left, right, or bottom and the value is then evaluated as a half-open value only when its role is known in the overall collection of edges in the shape (is this a transition from out-to-in or a transition from in-to-out). If it is applied to a minimum X or Y value then we know it is a left or top edge and so is on the inclusive side of an interval. If it is applied to a max value then we know it is on the right or bottom edge and therefore is exclusive. Only when it is applied to a non-min-max value do we have to scan from the top down or left-right and apply the EVEN_ODD/NON_ZERO rules to figure out if it is a start or end of an interior section to know if the value is inclusive or exclusive. The comment applies to the edgeMinX because that is a left value and so the calculation produces an inclusive value. The comment does not apply to the edgeMaxX value because that is a right value and is thus exclusive.
24-11-2016
I think there is a misunderstanding in the following lines even if I left the comment at line 1310: 1310 // bounds as inclusive intervals 1311 final int spminX = FloatMath.max(FloatMath.ceil_int(edgeMinX - 0.5f), boundsMinX); 1312 final int spmaxX = FloatMath.min(FloatMath.ceil_int(edgeMaxX - 0.5f), boundsMaxX - 1); "The spminX uses inclusive values for both the edgeMinX and boundsMinX values, but the spmaxX uses a mixture of inclusive/exclusive values. "ceil(x-0.5)" on a right edge is an exclusive value (that coordinate is not rendered), but boundsMaxX-1 is an inclusive value. " I have doubts as I think ceil(edgeMaxX - 0.5f) is for me an inclusive value as it corresponds to the rightmost subpixel value. In conclusion, there is no mixture when computing spmaxX as both values are inclusive values. Please give me an example to prove me I am wrong. Laurent
24-11-2016