Creating holes in .arc via the counter-clockwise flag does not work

[abetusk]

Issue or Feature

Creating a circle with a hole in it using arc and the counter-clockwise flags does not work.

I'm not sure but this could be related to #1736.

Steps to Reproduce

let fs = require("fs");
let Canvas = require('canvas');
let canvas = Canvas.createCanvas(512, 512);
let ctx = canvas.getContext('2d');

function fill_bug0() {
  ctx.clearRect(0, 0, canvas.width, canvas.height);

  ctx.fillStyle = "rgb( 255, 0, 0 )";
  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();

  var buf = canvas.toBuffer("image/png");
  fs.writeFileSync("./fillbug0.png", buf);
}

fill_bug0();

The above produces a completely filled in circle of radius 50.
Were it to render through a browser canvas, this would be a circle with a hole in it.

Here is a more complete example showing the different permutations of the angle and flag:

let fs = require("fs");
let Canvas = require('canvas');
let canvas = Canvas.createCanvas(512, 512);
let ctx = canvas.getContext('2d');

function fill_bug0() {
  ctx.clearRect(0, 0, canvas.width, canvas.height);

  ctx.fillStyle = "rgb( 255, 0, 0 )";
  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();

  var buf = canvas.toBuffer("image/png");
  fs.writeFileSync("./fillbug0.png", buf);
}

function fill_bug1() {
  ctx.clearRect(0, 0, canvas.width, canvas.height);

  ctx.fillStyle = "rgb( 255, 0, 0 )";
  ctx.beginPath();
  ctx.arc(256,256, 50, 0, 2 * Math.PI, false );
  ctx.arc(256,256, 25, 0, 2 * Math.PI, true );
  ctx.closePath();
  ctx.fill();

  var buf = canvas.toBuffer("image/png");
  fs.writeFileSync("./fillbug1.png", buf);
}

function fill_bug2() {
  ctx.clearRect(0, 0, canvas.width, canvas.height);

  ctx.fillStyle = "rgb( 255, 0, 0 )";
  ctx.beginPath();
  ctx.arc(256,256, 50, 0, -2 * Math.PI, true );
  ctx.arc(256,256, 25, 0, 2 * Math.PI, true );
  ctx.closePath();
  ctx.fill();

  var buf = canvas.toBuffer("image/png");
  fs.writeFileSync("./fillbug2.png", buf);
}

function fill_bug3() {
  ctx.clearRect(0, 0, canvas.width, canvas.height);

  ctx.fillStyle = "rgb( 255, 0, 0 )";
  ctx.beginPath();
  ctx.arc(256,256, 50, 0, -2 * Math.PI, false );
  ctx.arc(256,256, 25, 0, 2 * Math.PI, false );
  ctx.closePath();
  ctx.fill();

  var buf = canvas.toBuffer("image/png");
  fs.writeFileSync("./fillbug3.png", buf);
}

function outline() {
  ctx.clearRect(0, 0, canvas.width, canvas.height);

  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.stroke();

  var buf = canvas.toBuffer("image/png");
  fs.writeFileSync("./outline.png", buf);
}

fill_bug0();
fill_bug1();
fill_bug2();
fill_bug3()
outline();

Producing the following pictures:

Function	Parameters	Picture
fill_bug0	outer counterclockwise, inner clockwise
fill_bug1	outer clockwise, inner counterclockwise
fill_bug2	outer counterclockwise but with negative angle, inner counerclockwise
fill_bug3	outer clockwise but with negative angle, inner clockwise
outline	outer counterclockwise, inner clockwise

Note that the negative angles work as expected as do does doing the outline, whereas using the counterclockwise flags to draw the hole do not.

Your Environment

$ npm list canvas
js@ /home/XXX
└── canvas@2.8.0
$ npm version
{
  npm: '7.13.0',
  node: '14.17.0',
  v8: '8.4.371.23-node.63',
  uv: '1.41.0',
  zlib: '1.2.11',
  brotli: '1.0.9',
  ares: '1.17.1',
  modules: '83',
  nghttp2: '1.42.0',
  napi: '8',
  llhttp: '2.1.3',
  openssl: '1.1.1k',
  cldr: '38.1',
  icu: '68.2',
  tz: '2020d',
  unicode: '13.0'
}
$ cat /etc/lsb-release 
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=16.04
DISTRIB_CODENAME=xenial
DISTRIB_DESCRIPTION="Ubuntu 16.04.7 LTS"
$ uname -a
Linux mill 4.4.0-210-generic #242-Ubuntu SMP Fri Apr 16 09:57:56 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

[abetusk]

Well, I think the issue is in src/CanvasRenderingContext2d.cc:

/*
 * Adds an arc at x, y with the given radis and start/end angles.
 */

NAN_METHOD(Context2d::Arc) {
  if (!info[0]->IsNumber()
    || !info[1]->IsNumber()
    || !info[2]->IsNumber()
    || !info[3]->IsNumber()
    || !info[4]->IsNumber()) return;

  bool anticlockwise = Nan::To<bool>(info[5]).FromMaybe(false);

  Context2d *context = Nan::ObjectWrap::Unwrap<Context2d>(info.This());
  cairo_t *ctx = context->context();

  if (anticlockwise && M_PI * 2 != Nan::To<double>(info[4]).FromMaybe(0)) {
    cairo_arc_negative(ctx
      , Nan::To<double>(info[0]).FromMaybe(0)
      , Nan::To<double>(info[1]).FromMaybe(0)
      , Nan::To<double>(info[2]).FromMaybe(0)
      , Nan::To<double>(info[3]).FromMaybe(0)
      , Nan::To<double>(info[4]).FromMaybe(0));
  } else {
    cairo_arc(ctx
      , Nan::To<double>(info[0]).FromMaybe(0)
      , Nan::To<double>(info[1]).FromMaybe(0)
      , Nan::To<double>(info[2]).FromMaybe(0)
      , Nan::To<double>(info[3]).FromMaybe(0)
      , Nan::To<double>(info[4]).FromMaybe(0));
  }
}

Specifically, the line:

  ...
  if (anticlockwise && M_PI * 2 != Nan::To<double>(info[4]).FromMaybe(0)) {
  ..

It's not pretty, but here's a potential fix that "works for me":

/*
 * Adds an arc at x, y with the given radis and start/end angles.
 */

NAN_METHOD(Context2d::Arc) {
  if (!info[0]->IsNumber()
    || !info[1]->IsNumber()
    || !info[2]->IsNumber()
    || !info[3]->IsNumber()
    || !info[4]->IsNumber()) return;

  bool anticlockwise = Nan::To<bool>(info[5]).FromMaybe(false);

  Context2d *context = Nan::ObjectWrap::Unwrap<Context2d>(info.This());
  cairo_t *ctx = context->context();

  if (anticlockwise && ((M_PI * 2) == Nan::To<double>(info[4]).FromMaybe(0)) ) {
    cairo_arc_negative(ctx
      , Nan::To<double>(info[0]).FromMaybe(0)
      , Nan::To<double>(info[1]).FromMaybe(0)
      , Nan::To<double>(info[2]).FromMaybe(0)
      , Nan::To<double>(info[3]).FromMaybe(0)
      , -M_PI * 2);
  }
  else if (anticlockwise && ((M_PI * 2) != Nan::To<double>(info[4]).FromMaybe(0)) ) {
    cairo_arc_negative(ctx
      , Nan::To<double>(info[0]).FromMaybe(0)
      , Nan::To<double>(info[1]).FromMaybe(0)
      , Nan::To<double>(info[2]).FromMaybe(0)
      , Nan::To<double>(info[3]).FromMaybe(0)
      , Nan::To<double>(info[4]).FromMaybe(0));
  } else {
    cairo_arc(ctx
      , Nan::To<double>(info[0]).FromMaybe(0)
      , Nan::To<double>(info[1]).FromMaybe(0)
      , Nan::To<double>(info[2]).FromMaybe(0)
      , Nan::To<double>(info[3]).FromMaybe(0)
      , Nan::To<double>(info[4]).FromMaybe(0));
  }
}

Using git diff:

diff --git a/src/CanvasRenderingContext2d.cc b/src/CanvasRenderingContext2d.cc
index 98d0372..13a44a7 100644
--- a/src/CanvasRenderingContext2d.cc
+++ b/src/CanvasRenderingContext2d.cc
@@ -2932,7 +2932,15 @@ NAN_METHOD(Context2d::Arc) {
   Context2d *context = Nan::ObjectWrap::Unwrap<Context2d>(info.This());
   cairo_t *ctx = context->context();
 
-  if (anticlockwise && M_PI * 2 != Nan::To<double>(info[4]).FromMaybe(0)) {
+  if (anticlockwise && ((M_PI * 2) == Nan::To<double>(info[4]).FromMaybe(0)) ) {
+    cairo_arc_negative(ctx
+      , Nan::To<double>(info[0]).FromMaybe(0)
+      , Nan::To<double>(info[1]).FromMaybe(0)
+      , Nan::To<double>(info[2]).FromMaybe(0)
+      , Nan::To<double>(info[3]).FromMaybe(0)
+      , -M_PI * 2);
+  }
+  else if (anticlockwise && ((M_PI * 2) != Nan::To<double>(info[4]).FromMaybe(0)) ) {
     cairo_arc_negative(ctx
       , Nan::To<double>(info[0]).FromMaybe(0)
       , Nan::To<double>(info[1]).FromMaybe(0)