[OpenJDK Rasterizer] Marlin artefact issue (Pisces too ?)

[Jim Graham]

It's been a while since I went through that code, but the *mxy numbers 
tend to be the perpendicular vectors at the vertices (multiplied by the 
line width).

omx*mx + omy*my looks like the dot product of two vectors which produces 
"len(v1) * len(v2) * cos(angle v1,v2)", so dividing it by lineWidth2 
twice reduces it to the cosine of the two vectors and the extra factor 
of 2 in the denominator means it is actually half of the cosine of the 
angles at the start and end of the arc - i.e. cos(ext)/2.

It should be in the indicated range, but rounding error could mean it 
may be occasionally outside of that range.  I can see where very long 
line widths might cause problems.

Since cv is a function only of cosext2 it might be better to simply 
assign the value of cv directly, as in:

if (cosext2 <= -0.5f) {
     cv = the answer for -0.5
     // 4/3?
} else if (cosext >= 0.5f) {
     cv = the answer for +0.5
     // 0.0?
} else {
     cv = current formula
}

Note that a cv value of 0.0 would insert an empty cubic so we could 
probably just return right there instead of setting cv to 0.0 and 
computing a bunch of useless values.

Also, the function should only ever be called with a maximum of a right 
angle so we should never see negative numbers in any case - are you 
seeing -0.5 values or just +0.5 values?

The fix could be as simple as "if (cosext2 >= 0.5) return;"

			...jim

On 12/3/15 12:49 PM, Laurent Bourgès wrote:
> Jim,
>
> I should create a new bug concerning both Marlin & Pisces but I prefer
> discussing the problem first.
>
> A Marlin user reported me an issue with (text outline) rendering
> artefacts (due to partial cleanup in arrays).
> Using Marlin with -Dsun.java2d.renderer.doChecks=true (logs re-enabled),
> we detected that edgeBucketCounts arrays were not properly zero-filled
> due to NaN coordinates ie array[0] > 0 !
>
> As it only happens with very large coordinates and round joins, we
> finally got a reproducer test class.
>
> However, we tracked the problem down into the
> Stroker.drawBezApproxForArc() method (from Pisces):
> cv = NaN.
>
> I am not very good at curve maths but I figured out that cosext2 means
> cos(ext)^2 as there is below sqrt(0.5 +/- cosext2) !
>
> Of course, the problem is sqrt(negative) gives NaN !
>
> Moreover, it only happens with very large out-of-clip coordinates (2M),
> see the test class !
> It probably means that float values have not enough precision in
> previous math operations and it finally overflows 0.5 !
>
> Is it correct to clamp cosext2 in [-0.5, 0.5] range as I propose ?
>
>      // the input arc defined by omx,omy and mx,my must span <= 90 degrees.
>      private void drawBezApproxForArc(final float cx, final float cy,
>                                       final float omx, final float omy,
>                                       final float mx, final float my,
>                                       boolean rev)
>      {
>          float cosext2 = (omx * mx + omy * my) / (2f * lineWidth2 *
> lineWidth2);
>
> // PROPOSED FIX TO BE CONFIRMED:
>          // clamp value within [-0.5, 0.5] range:
>          if (cosext2 < -0.5f) {
>              cosext2 = -0.5f;
>          } else if (cosext2 > 0.5f) {
>              cosext2 = 0.5f;
>          }
>
>          // cv is the length of P1-P0 and P2-P3 divided by the radius of
> the arc
>          // (so, cv assumes the arc has radius 1). P0, P1, P2, P3 are
> the points that
>          // define the bezier curve we're computing.
>          // It is computed using the constraints that P1-P0 and P3-P2
> are parallel
>          // to the arc tangents at the endpoints, and that |P1-P0|=|P3-P2|.
>          float cv = (float) ((4.0 / 3.0) * sqrt(0.5 - cosext2) /
>                              (1.0 + sqrt(cosext2 + 0.5)));
>
> If you have any explanations,  please tell me !
>
> PS: Anyway NaN handling must be properly tested and filtered in Marlin's
> pipeline (like DuctusRenderingEngine does) ...
> but here it concerns intermediate points (not inputs).
>
> Test code:
> /*
>   * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>   *
>   * This code is free software; you can redistribute it and/or modify it
>   * under the terms of the GNU General Public License version 2 only, as
>   * published by the Free Software Foundation.
>   *
>   * This code is distributed in the hope that it will be useful, but WITHOUT
>   * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>   * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>   * version 2 for more details (a copy is included in the LICENSE file that
>   * accompanied this code).
>   *
>   * You should have received a copy of the GNU General Public License
> version
>   * 2 along with this work; if not, write to the Free Software Foundation,
>   * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
>   *
>   * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
>   * or visit www.oracle.com <http://www.oracle.com> if you need
> additional information or have any
>   * questions.
>   */
>
> import java.awt.BasicStroke;
> import java.awt.Color;
> import java.awt.Font;
> import java.awt.Graphics2D;
> import java.awt.RenderingHints;
> import java.awt.font.FontRenderContext;
> import java.awt.font.GlyphVector;
> import java.awt.geom.AffineTransform;
> import java.awt.image.BufferedImage;
> import java.io.File;
> import java.io.IOException;
> import javax.imageio.ImageIO;
>
> /**
>   * @test
>   * @summary Check the Stroker.drawBezApproxForArc() bug:
>   * abs(cosext2) > 0.5 generates curves with NaN coordinates
>   * @run main TextClipErrorTest
>   */
> public class TextClipErrorTest {
>
>      public static void main(String[] args) {
>          BufferedImage image = new BufferedImage(256, 256,
>                  BufferedImage.TYPE_INT_ARGB);
>
>          Graphics2D g2d = image.createGraphics();
>          g2d.setColor(Color.red);
>          try {
>              g2d.setRenderingHint(RenderingHints.KEY_ANTIALIASING,
>                      RenderingHints.VALUE_ANTIALIAS_ON);
>
>              Font font = g2d.getFont();
>              FontRenderContext frc = new FontRenderContext(
>                      new AffineTransform(), true, true);
>
>              GlyphVector gv1 = font.createGlyphVector(frc, "\u00d6");
>
>              g2d.setStroke(new BasicStroke(4.0f,
>                      BasicStroke.CAP_ROUND, BasicStroke.JOIN_ROUND));
>
>              AffineTransform at1 = AffineTransform.getTranslateInstance(
>                      -2091202.554154681, 5548.601436981691);
>              g2d.draw(at1.createTransformedShape(gv1.getOutline()));
>
>              GlyphVector gv2 = font.createGlyphVector(frc, "Test 2");
>
>              AffineTransform at2 = AffineTransform.getTranslateInstance(
> //                    -218.1810476789251, 85.12774919422463);
>                      10, 50);
>              g2d.draw(at2.createTransformedShape(gv2.getOutline()));
>
>
>              final File file = new File("TextClipErrorTest.png");
>              System.out.println("Writing file: " + file.getAbsolutePath());
>              ImageIO.write(image, "PNG", file);
>
>          } catch (IOException ex) {
>              ex.printStackTrace();
>          }
>          finally {
>              g2d.dispose();
>          }
>      }
> }
>
>
> Cheers,
> Laurent