Skip to content

Commit 73d7893

Browse files
committed
fix arc geometry calculations
Borrowed from Chromium instead of reinventing the wheel. Firefox's is similar: https://searchfox.org/mozilla-central/source/gfx/2d/PathHelpers.h#127 Fixes #1736 Fixes #1808
1 parent a484cf2 commit 73d7893

File tree

3 files changed

+82
-1
lines changed

3 files changed

+82
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ project adheres to [Semantic Versioning](http://semver.org/).
1515
* Typo in `PngConfig.filters` types. ([#2072](https://github.com/Automattic/node-canvas/issues/2072))
1616
* `createPattern()` always used "repeat" mode; now supports "repeat-x" and "repeat-y". ([#2066](https://github.com/Automattic/node-canvas/issues/2066))
1717
* Crashes and hangs when using non-finite values in `context.arc()`. ([#2055](https://github.com/Automattic/node-canvas/issues/2055))
18+
* Incorrect `context.arc()` geometry logic for full ellipses. ([#1808](https://github.com/Automattic/node-canvas/issues/1808), ([#1736](https://github.com/Automattic/node-canvas/issues/1736)))
1819

1920
2.9.3
2021
==================

src/CanvasRenderingContext2d.cc

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ Nan::Persistent<FunctionTemplate> Context2d::constructor;
3636
double width = args[2]; \
3737
double height = args[3];
3838

39+
constexpr double twoPi = M_PI * 2.;
40+
3941
/*
4042
* Text baselines.
4143
*/
@@ -2935,6 +2937,52 @@ NAN_METHOD(Context2d::Rect) {
29352937
}
29362938
}
29372939

2940+
// Adapted from https://chromium.googlesource.com/chromium/blink/+/refs/heads/main/Source/modules/canvas2d/CanvasPathMethods.cpp
2941+
static void canonicalizeAngle(double& startAngle, double& endAngle) {
2942+
// Make 0 <= startAngle < 2*PI
2943+
double newStartAngle = std::fmod(startAngle, twoPi);
2944+
if (newStartAngle < 0) {
2945+
newStartAngle += twoPi;
2946+
// Check for possible catastrophic cancellation in cases where
2947+
// newStartAngle was a tiny negative number (c.f. crbug.com/503422)
2948+
if (newStartAngle >= twoPi)
2949+
newStartAngle -= twoPi;
2950+
}
2951+
double delta = newStartAngle - startAngle;
2952+
startAngle = newStartAngle;
2953+
endAngle = endAngle + delta;
2954+
}
2955+
2956+
// Adapted from https://chromium.googlesource.com/chromium/blink/+/refs/heads/main/Source/modules/canvas2d/CanvasPathMethods.cpp
2957+
static double adjustEndAngle(double startAngle, double endAngle, bool counterclockwise) {
2958+
double newEndAngle = endAngle;
2959+
/* http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-context-2d-arc
2960+
* If the counterclockwise argument is false and endAngle-startAngle is equal to or greater than 2pi, or,
2961+
* if the counterclockwise argument is true and startAngle-endAngle is equal to or greater than 2pi,
2962+
* then the arc is the whole circumference of this ellipse, and the point at startAngle along this circle's circumference,
2963+
* measured in radians clockwise from the ellipse's semi-major axis, acts as both the start point and the end point.
2964+
*/
2965+
if (!counterclockwise && endAngle - startAngle >= twoPi)
2966+
newEndAngle = startAngle + twoPi;
2967+
else if (counterclockwise && startAngle - endAngle >= twoPi)
2968+
newEndAngle = startAngle - twoPi;
2969+
/*
2970+
* Otherwise, the arc is the path along the circumference of this ellipse from the start point to the end point,
2971+
* going anti-clockwise if the counterclockwise argument is true, and clockwise otherwise.
2972+
* Since the points are on the ellipse, as opposed to being simply angles from zero,
2973+
* the arc can never cover an angle greater than 2pi radians.
2974+
*/
2975+
/* NOTE: When startAngle = 0, endAngle = 2Pi and counterclockwise = true, the spec does not indicate clearly.
2976+
* We draw the entire circle, because some web sites use arc(x, y, r, 0, 2*Math.PI, true) to draw circle.
2977+
* We preserve backward-compatibility.
2978+
*/
2979+
else if (!counterclockwise && startAngle > endAngle)
2980+
newEndAngle = startAngle + (twoPi - std::fmod(startAngle - endAngle, twoPi));
2981+
else if (counterclockwise && startAngle < endAngle)
2982+
newEndAngle = startAngle - (twoPi - std::fmod(endAngle - startAngle, twoPi));
2983+
return newEndAngle;
2984+
}
2985+
29382986
/*
29392987
* Adds an arc at x, y with the given radii and start/end angles.
29402988
*/
@@ -2960,7 +3008,10 @@ NAN_METHOD(Context2d::Arc) {
29603008
Context2d *context = Nan::ObjectWrap::Unwrap<Context2d>(info.This());
29613009
cairo_t *ctx = context->context();
29623010

2963-
if (counterclockwise && M_PI * 2 != endAngle) {
3011+
canonicalizeAngle(startAngle, endAngle);
3012+
endAngle = adjustEndAngle(startAngle, endAngle, counterclockwise);
3013+
3014+
if (counterclockwise) {
29643015
cairo_arc_negative(ctx, x, y, radius, startAngle, endAngle);
29653016
} else {
29663017
cairo_arc(ctx, x, y, radius, startAngle, endAngle);

test/public/tests.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,35 @@ tests['arc() 2'] = function (ctx) {
146146
}
147147
}
148148

149+
tests['arc()() #1736'] = function (ctx) {
150+
let centerX = 512
151+
let centerY = 512
152+
let startAngle = 6.283185307179586 // exactly 2pi
153+
let endAngle = 7.5398223686155035
154+
let innerRadius = 359.67999999999995
155+
let outerRadius = 368.64
156+
157+
ctx.scale(0.2, 0.2)
158+
159+
ctx.beginPath()
160+
ctx.moveTo(centerX + Math.cos(startAngle) * innerRadius, centerY + Math.sin(startAngle) * innerRadius)
161+
ctx.lineTo(centerX + Math.cos(startAngle) * outerRadius, centerY + Math.sin(startAngle) * outerRadius)
162+
ctx.arc(centerX, centerY, outerRadius, startAngle, endAngle, false)
163+
ctx.lineTo(centerX + Math.cos(endAngle) * innerRadius, centerY + Math.sin(endAngle) * innerRadius)
164+
ctx.arc(centerX, centerY, innerRadius, endAngle, startAngle, true)
165+
ctx.closePath()
166+
ctx.stroke()
167+
}
168+
169+
tests['arc()() #1808'] = function (ctx) {
170+
ctx.scale(0.5, 0.5)
171+
ctx.beginPath()
172+
ctx.arc(256, 256, 50, 0, 2 * Math.PI, true)
173+
ctx.arc(256, 256, 25, 0, 2 * Math.PI, false)
174+
ctx.closePath()
175+
ctx.fill()
176+
}
177+
149178
tests['arcTo()'] = function (ctx) {
150179
ctx.fillStyle = '#08C8EE'
151180
ctx.translate(-50, -50)

0 commit comments

Comments
 (0)