JDK-8191035 : WebView Canvas Graphics2D arc renders incorrectly
  • Type: Bug
  • Component: javafx
  • Sub-Component: web
  • Affected Version: 8u144,9,10
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2017-11-09
  • Updated: 2020-01-31
  • Resolved: 2017-12-22
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 10 JDK 8
10Fixed 8u172Fixed
Description
We have been attempting to use JavaFx WebView with canvas and 2d Graphics for 
trying to recreate a pie chart, but a simple arc does not draw correctly in WebView, 
Since our application requires use of drawing canvas arcs, we need this fixed. 

Here is codepen which you can use to recreate the problem: 

https://codepen.io/blueShell/pen/MENyRJ 

Here is the same source HTML 
{code} 
<html> 
<head> 
  <title></title> 
</head> 

<body> 
    <canvas id="myCanvas" width="578" height="250"></canvas> 
    <script> 
      var canvas = document.getElementById('myCanvas'); 
var context = canvas.getContext('2d'); 
var x = canvas.width / 2; 
var y = canvas.height / 2; 
var radius = 75; 
var startAngle = 0; 
startAngle = startAngle - (Math.PI / 2.0); 
var ratio = 0.59; 
var endAngle = ratio * Math.PI * 2.0; 
endAngle = endAngle - (Math.PI / 2.0); 
var counterClockwise = false; 

context.beginPath(); 
context.arc(x, y, radius, startAngle, endAngle, counterClockwise); 
context.lineWidth = 15; 

// line color 
context.strokeStyle = 'black'; 
context.stroke(); 
    </script> 
</body> 
</html> 
{code} 

Attached are screen shots comparing the arc in different browsers showing WebView is incorrect when compared in Chrome and IE11. 
Comments
changeset: 1af84f01b569 user: arajkumar date: Fri Dec 22 11:51:15 2017 +0530 description: 8191035: WebView Canvas Graphics2D arc renders incorrectly Reviewed-by: ghb, kcr, mbilla Contributed-by: arunprasad.rajkumar@oracle.com, guru.hb@oracle.com URL: http://hg.openjdk.java.net/openjfx/10-dev/rt/rev/1af84f01b569
22-12-2017

Looks like single line comment is used across the codebase (e.g. WebEngine.java, BaseShaderGraphics.java, ..etc). Probably multiline comment is reserved for documentation.
20-12-2017

+1 with below nit: In WCPathImpl.java, you can consider multi line comments /* .. */ instead of single line as similar to blink code ?
20-12-2017

Looks fine to me. +1
20-12-2017

+1
15-12-2017

Found a way to fix "arc.html" test as well. Also simplified the "endAngle" computation code by adopting Blink's code[1]. WebCore already implemented the normalisation logic partially, only the remaining part(endAngle) is adopted from Blink[1]. http://cr.openjdk.java.net/~arajkumar/8191035/webrev.01 [~ghb], [~kcr], Please take a look. Thanks. [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/canvas/canvas2d/CanvasPath.cpp?gsn=SkScalarNearlyEqual&pv=1&l=162
14-12-2017

Thanks [~arajkumar] +1
14-12-2017

>> 1) Is there any reason why we are comparing with if (endAngle < eps) instead of if (endAngle < 0.0) ? I thought of consider value "less than 1e-5" as "less than zero" since we convert from highest precision(double) to lowest precision(float). Both are *almost* same in double comparison, but *not* same. >> 2) I see that the existing eps value is 0.001, where as now it is 0.00001. New epsilon value is required to make "canvas/philip/tests/2d.path.arc.twopie.1.html" pass.
14-12-2017

Tested the patch and looks fine..But i have below concerns and can you please clarify: 1) Is there any reason why we are comparing with if (endAngle < eps) instead of if (endAngle < 0.0) ? 2) I see that the existing eps value is 0.001, where as now it is 0.00001.
14-12-2017

Looks fine. +1
13-12-2017

Current changes looks good to me. Please file another JBS to fix 'arc.html' which you have posted earlier.
13-12-2017

http://cr.openjdk.java.net/~arajkumar/8191035/webrev [~kcr], [~ghb], Please take a look. (Note: "arc.html" rendering is inconsistent across browsers)
13-12-2017

[~ghb], Please refer the attached file(arc-simple-2.diff). Change is only in WCPathImpl.java, no other changes are required. I tested it on Windows and those tests are passing.
12-12-2017

Thanks [~arajkumar] I did a quick test and could see these below are failing drt-int/LayoutTests/canvas/philip/tests/2d.path.arc.angle.5.html drt-int/LayoutTests/fast/canvas/canvas-arc-connecting-line.html // Check for path direction and its allignment drt-int/LayoutTests/canvas/philip/tests/2d.path.arc.scale.2.html drt-int/LayoutTests/canvas/philip/tests/2d.path.arc.shape.3.html drt-int/LayoutTests/canvas/philip/tests/2d.path.arc.angle.5.html drt-int/LayoutTests/fast/canvas/canvas-arc-360-winding.html // Check for rendering Could you please provide the diff so i can re-confirm the Arc2D obj for both clockwise and anti clockwise ?
12-12-2017

[~ghb] I thought of another way to convert from WebKit's arc param to JFX's arc param. 1. Normalize the -ve endAngle to +ve. (endAngle = endAngle % TWO_PI + TWO_PI) (startAngle is already normalized by WebCore) 2. Calculate the arc length ( d = startAngle > endAngle ? TWO_PI - startAngle + endAngle : endAngle - startAngle) by assuming the direction as clockwise. 2.1. if direction is anticlockwise, new arc length would be (TWO_PI - d) Also I see the bunch of redundant code in WCPathImpl.java(I may be wrong) to render the arc in clockwise, which could be removed and implemented by negating the arc length(-d) and startAngle. I also tried the above approach which seems to be working. (verified by running DRT tests you have mentioned) public void addArc(double x, double y, double r, double startAngle, double endAngle, boolean aclockwise) { if (log.isLoggable(Level.FINE)) { log.log(Level.FINE, "WCPathImpl({0}).addArc(x={1},y={2},r={3},sa=|{4}|, ea=|{5}|,aclock={6})", new Object[] {getID(), x, y, r, startAngle, endAngle, aclockwise}); } hasCP = true; final double eps = 1e-5; final double TWO_PI = 2 * Math.PI; if (endAngle < eps) { // Convert to +ve angle(clockwise) endAngle = (endAngle % TWO_PI) + TWO_PI; } else if (endAngle > TWO_PI) { endAngle = endAngle % TWO_PI; } double d = startAngle > endAngle ? (TWO_PI - startAngle + endAngle) : endAngle - startAngle; final boolean isMultipleOf2PIor0 = Math.abs(Math.IEEEremainder(d, TWO_PI)) < eps; if (aclockwise) { d = isMultipleOf2PIor0 ? TWO_PI : TWO_PI - d; } else { // Negate the direction, prism renders anti-clockwise by default. d = -(isMultipleOf2PIor0 ? TWO_PI : d); } path.append(new Arc2D((float)(x - r), (float)(y - r), (float)(2*r), (float)(2*r), (float) ((-startAngle * 180.0) / Math.PI), (float) ((d * 180.0) / Math.PI), Arc2D.OPEN), true); }
12-12-2017

Thanks [~arajkumar] please find updated webrev : http://cr.openjdk.java.net/~ghb/8191035/webrev.03/ Note : arc1.html renders different on Chrome and Firefox and neither arc1.html and arc.html renders in safari. These DRT are passing (earlier failed) and didn't see any regression. running canvas/philip/tests/2d.path.arc.angle.4.html -> succeeded running canvas/philip/tests/2d.path.arc.angle.6.html -> succeeded running canvas/philip/tests/2d.path.arc.twopie.1.html -> succeeded running fast/canvas/canvas_arc_largeangles.html -> succeeded
12-12-2017

While reviewing I found that attached tests(arc.html and arc1.html) are not rendered like Chrome or FF.
09-12-2017

Thanks [~ghb] +1
07-12-2017

Thanks [~rkamath] Updated webrev with review comments incorporated : http://cr.openjdk.java.net/~ghb/8191035/webrev.02
06-12-2017

Few minor nits : File : WCPathImpl.java #49 - Feel renaming the constant to all caps with underscore(TWO_PI) would be better #128 - You can use shorthand operator here in line with other places #217 - Space after comma Copyright year updation File : CanvasTest.java Think we can rename "testCanvasArcTest" to "testCanvasArc" Rest look fine to me.
06-12-2017

Update webrev : http://cr.openjdk.java.net/~ghb/8191035/webrev.01/ I Didn't see any bug in WebKit's logic, and the implementation is based on HTML5 Spec and its details comments can be found in "https://bugs.webkit.org/show_bug.cgi?id=82791". Root cause : ----- Normalizes the angles as described in the HTML spec. Ensuring the startAngle is greater than 0 and less than 2pi, and the the endAngle is at most 2pi from the start angle. Note : https://html.spec.whatwg.org/#dom-context-2d-ellipse : "If anticlockwise is false and endAngle-startAngle is equal to or greater than 2�� ..." For any given start and end angle the Native normalization make sure Start angle : 0 -- 2�� End angle : Offset of normalized start angle i.e if start angle < 0 Or start angle > 2�� Test case in description input from js sa : -1.570796 (-89��), ea : 2.136283 (112.39��) sa = 4.712 (269.99��) (0 ��� sa < 2�� i.e 6.28) ea = 8.419 (482.4��) endAngle is at most 2pi from the start angle //https://trac.webkit.org/changeset/180790/webkit ---Expected direction and convention----- arc(0, ��/4, counterClockwise = true) . | . . | * . | --------------*-------- . | . . | . | | arc(0, ��/4, counterClockwise = false) | | *. | . --------------*-------- | | | Note : Native angles are measured in counter clock wise. Arc2D(x, y, w, h, start, extent), start -> is measured as Counter clockwise. --- end ---- Test case provided in description and my previous fix for removing endAngle normalization. 'arc(-1.57, 2.13, counterClockwise = false)' // from Javascript arc(4.71, 8.41, 0) // after normalized in native if (8.41 > 2�� + 0.001) : // end angle is normalized by 2��, bug in existing code and rendering starts from endangle. 8.41 (normalized by 2��) -> 2.13 (updated End angle, without considering the starting angle which is normalized in native) delta = 4.71 - 2.13 delta = 2.57 // This should be 4.71 - 8.4 -> 3.69 ---------------------------------------------------- Solution : Removed normalization (only applicable for endAngle now). Added comments for clarity. Test : No regression found in drt, "fast/canvas/arc360.html" is now passing Or Renders as expected (Failed --> Succeeded).
29-11-2017

For the given test case, I'm seeing the below print. FINE: WCPathImpl(6).addArc(289,125,75,4.712,8.419,false) Normalisation ensures the given angles lies in between (0, 2��), but looks like endAngle(8.419 [in radians]) is not normalised. Also double normalisation should not cause any issue, because the second one will be become no-op. Seems like a bug either in our normalisation logic or WebKit's.
21-11-2017

Webrev : http://cr.openjdk.java.net/~ghb/8191035/webrev.00/ Root Cause : End angle was normalized twice in Native as well as in WCPathImpl.addArc. Solution : Native WebCore::normalizeAngles (Implemented in https://trac.webkit.org/changeset/180790/webkit in according to http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-context-2d-ellipse) does endAngle computation based on HTML spec. Removed the end angle correction in WCPathImpl.addArc if the endAngle overlaps 360 deg (as its already taken care in native). Executed DRT and didn't find any regression. Note: I tried removing End angle normalisation for clockwise (WCPathImpl.addArc) which lead to regression in drt test case for "2d.path.arc.angle.3.html".
20-11-2017