From dc2f483f1fd81c190fdb8fcd216608b6c42771bd Mon Sep 17 00:00:00 2001 From: Zach Bjornson Date: Wed, 27 Jul 2022 14:32:35 -0700 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + src/CanvasRenderingContext2d.cc | 53 ++++++++++++++++++++++++++++++++- test/public/tests.js | 29 ++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f19144f6..6c354c6bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ project adheres to [Semantic Versioning](http://semver.org/). * Typo in `PngConfig.filters` types. ([#2072](https://github.com/Automattic/node-canvas/issues/2072)) * `createPattern()` always used "repeat" mode; now supports "repeat-x" and "repeat-y". ([#2066](https://github.com/Automattic/node-canvas/issues/2066)) * Crashes and hangs when using non-finite values in `context.arc()`. ([#2055](https://github.com/Automattic/node-canvas/issues/2055)) +* 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))) 2.9.3 ================== diff --git a/src/CanvasRenderingContext2d.cc b/src/CanvasRenderingContext2d.cc index 2264ea6fd..831f47652 100644 --- a/src/CanvasRenderingContext2d.cc +++ b/src/CanvasRenderingContext2d.cc @@ -36,6 +36,8 @@ Nan::Persistent Context2d::constructor; double width = args[2]; \ double height = args[3]; +constexpr double twoPi = M_PI * 2.; + /* * Text baselines. */ @@ -2935,6 +2937,52 @@ NAN_METHOD(Context2d::Rect) { } } +// Adapted from https://chromium.googlesource.com/chromium/blink/+/refs/heads/main/Source/modules/canvas2d/CanvasPathMethods.cpp +static void canonicalizeAngle(double& startAngle, double& endAngle) { + // Make 0 <= startAngle < 2*PI + double newStartAngle = std::fmod(startAngle, twoPi); + if (newStartAngle < 0) { + newStartAngle += twoPi; + // Check for possible catastrophic cancellation in cases where + // newStartAngle was a tiny negative number (c.f. crbug.com/503422) + if (newStartAngle >= twoPi) + newStartAngle -= twoPi; + } + double delta = newStartAngle - startAngle; + startAngle = newStartAngle; + endAngle = endAngle + delta; +} + +// Adapted from https://chromium.googlesource.com/chromium/blink/+/refs/heads/main/Source/modules/canvas2d/CanvasPathMethods.cpp +static double adjustEndAngle(double startAngle, double endAngle, bool counterclockwise) { + double newEndAngle = endAngle; + /* http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-context-2d-arc + * If the counterclockwise argument is false and endAngle-startAngle is equal to or greater than 2pi, or, + * if the counterclockwise argument is true and startAngle-endAngle is equal to or greater than 2pi, + * then the arc is the whole circumference of this ellipse, and the point at startAngle along this circle's circumference, + * measured in radians clockwise from the ellipse's semi-major axis, acts as both the start point and the end point. + */ + if (!counterclockwise && endAngle - startAngle >= twoPi) + newEndAngle = startAngle + twoPi; + else if (counterclockwise && startAngle - endAngle >= twoPi) + newEndAngle = startAngle - twoPi; + /* + * Otherwise, the arc is the path along the circumference of this ellipse from the start point to the end point, + * going anti-clockwise if the counterclockwise argument is true, and clockwise otherwise. + * Since the points are on the ellipse, as opposed to being simply angles from zero, + * the arc can never cover an angle greater than 2pi radians. + */ + /* NOTE: When startAngle = 0, endAngle = 2Pi and counterclockwise = true, the spec does not indicate clearly. + * We draw the entire circle, because some web sites use arc(x, y, r, 0, 2*Math.PI, true) to draw circle. + * We preserve backward-compatibility. + */ + else if (!counterclockwise && startAngle > endAngle) + newEndAngle = startAngle + (twoPi - std::fmod(startAngle - endAngle, twoPi)); + else if (counterclockwise && startAngle < endAngle) + newEndAngle = startAngle - (twoPi - std::fmod(endAngle - startAngle, twoPi)); + return newEndAngle; +} + /* * Adds an arc at x, y with the given radii and start/end angles. */ @@ -2960,7 +3008,10 @@ NAN_METHOD(Context2d::Arc) { Context2d *context = Nan::ObjectWrap::Unwrap(info.This()); cairo_t *ctx = context->context(); - if (counterclockwise && M_PI * 2 != endAngle) { + canonicalizeAngle(startAngle, endAngle); + endAngle = adjustEndAngle(startAngle, endAngle, counterclockwise); + + if (counterclockwise) { cairo_arc_negative(ctx, x, y, radius, startAngle, endAngle); } else { cairo_arc(ctx, x, y, radius, startAngle, endAngle); diff --git a/test/public/tests.js b/test/public/tests.js index e079ad827..50c8358b7 100644 --- a/test/public/tests.js +++ b/test/public/tests.js @@ -146,6 +146,35 @@ tests['arc() 2'] = function (ctx) { } } +tests['arc()() #1736'] = function (ctx) { + let center_x = 512; + let center_y = 512; + let start_angle = 6.283185307179586 ; // exactly 2pi + let end_angle = 7.5398223686155035; + let inner_radius = 359.67999999999995; + let outer_radius = 368.64; + + ctx.scale(0.2, 0.2); + + ctx.beginPath(); + ctx.moveTo(center_x + Math.cos(start_angle) * inner_radius, center_y + Math.sin(start_angle) * inner_radius); + ctx.lineTo(center_x + Math.cos(start_angle) * outer_radius, center_y + Math.sin(start_angle) * outer_radius); + ctx.arc(center_x, center_y, outer_radius, start_angle, end_angle, false); + ctx.lineTo(center_x + Math.cos(end_angle) * inner_radius, center_y + Math.sin(end_angle) * inner_radius); + ctx.arc(center_x, center_y, inner_radius, end_angle, start_angle, true); + ctx.closePath(); + ctx.stroke(); +} + +tests['arc()() #1808'] = function (ctx) { + ctx.scale(0.5, 0.5); + ctx.beginPath(); + ctx.arc(256,256, 50, 0, 2 * Math.PI, true ); + ctx.arc(256,256, 25, 0, 2 * Math.PI, false); + ctx.closePath(); + ctx.fill(); +} + tests['arcTo()'] = function (ctx) { ctx.fillStyle = '#08C8EE' ctx.translate(-50, -50)