Merge pull request #7290 from martinleopold/ellipse-fix

Fix ellipseMode(CORNERS) and rectMode(CORNER)

.all-contributorsrc

@@ -6370,9 +6370,9 @@
     },
     {
       "login": "martinleopold",
-      "name": "Martin Leopold",
+      "name": "Martin Leopold Groedl",
       "avatar_url": "https://avatars.githubusercontent.com/u/1692826?v=4",
-      "profile": "https://github.com/martinleopold",
+      "profile": "https://groedl.xyz",
       "contributions": [
         "bug",
         "code"

README.md

@@ -1067,7 +1067,7 @@ We recognize all types of contributions. This project follows the [all-contribut
       <td align="center" valign="top" width="16.66%"><a href="http://computationalmama.xyz"><img src="https://avatars.githubusercontent.com/u/10388784?v=4?s=120" width="120px;" alt="computational mama"/><br /><sub><b>computational mama</b></sub></a><br /><a href="https://github.com/processing/p5.js/commits?author=computationalmama" title="Code">💻</a></td>
       <td align="center" valign="top" width="16.66%"><a href="https://fabianmoronzirfas.me"><img src="https://avatars.githubusercontent.com/u/315106?v=4?s=120" width="120px;" alt="Fabian Morón Zirfas"/><br /><sub><b>Fabian Morón Zirfas</b></sub></a><br /><a href="https://github.com/processing/p5.js/commits?author=ff6347" title="Documentation">📖</a> <a href="https://github.com/processing/p5.js/commits?author=ff6347" title="Code">💻</a></td>
       <td align="center" valign="top" width="16.66%"><a href="http://www.lukeplowden.com"><img src="https://avatars.githubusercontent.com/u/62835749?v=4?s=120" width="120px;" alt="Luke Plowden"/><br /><sub><b>Luke Plowden</b></sub></a><br /><a href="https://github.com/processing/p5.js/commits?author=lukeplowden" title="Code">💻</a></td>
-      <td align="center" valign="top" width="16.66%"><a href="https://github.com/martinleopold"><img src="https://avatars.githubusercontent.com/u/1692826?v=4?s=120" width="120px;" alt="Martin Leopold"/><br /><sub><b>Martin Leopold</b></sub></a><br /><a href="https://github.com/processing/p5.js/issues?q=author%3Amartinleopold" title="Bug reports">🐛</a> <a href="https://github.com/processing/p5.js/commits?author=martinleopold" title="Code">💻</a></td>
+      <td align="center" valign="top" width="16.66%"><a href="https://github.com/martinleopold"><img src="https://avatars.githubusercontent.com/u/1692826?v=4?s=120" width="120px;" alt="Martin Leopold Groedl"/><br /><sub><b>Martin Leopold Groedl</b></sub></a><br /><a href="https://github.com/processing/p5.js/issues?q=author%3Amartinleopold" title="Bug reports">🐛</a> <a href="https://github.com/processing/p5.js/commits?author=martinleopold" title="Code">💻</a></td>
     </tr>
     <tr>
       <td align="center" valign="top" width="16.66%"><a href="https://www.linkedin.com/in/ashish1729/"><img src="https://avatars.githubusercontent.com/u/10610651?v=4?s=120" width="120px;" alt="ashish singh"/><br /><sub><b>ashish singh</b></sub></a><br /><a href="https://github.com/processing/p5.js/commits?author=ashish1729" title="Code">💻</a></td>

src/core/helpers.js

@@ -4,16 +4,47 @@
 
 import * as constants from './constants';
 
+/*
+  This function normalizes the first four arguments given to rect, ellipse and arc
+  according to the mode.
+  It returns a 'bounding box' object containing the coordinates of the upper left corner (x, y),
+  and width and height (w, h). The returned width and height are always positive.
+*/
 function modeAdjust(a, b, c, d, mode) {
+  let bbox;
+
   if (mode === constants.CORNER) {
-    return { x: a, y: b, w: c, h: d };
+
+    // CORNER mode already corresponds to a bounding box (top-left corner, width, height)
+    bbox = { x: a, y: b, w: c, h: d };
+
   } else if (mode === constants.CORNERS) {
-    return { x: a, y: b, w: c - a, h: d - b };
+
+    // CORNERS mode uses two opposite corners, in any configuration.
+    // Make sure to get the top left corner by using the minimum of the x and y coordniates.
+    bbox = { x: Math.min(a, c), y: Math.min(b, d), w: c - a, h: d - b };
+
   } else if (mode === constants.RADIUS) {
-    return { x: a - c, y: b - d, w: 2 * c, h: 2 * d };
+
+    // RADIUS mode uses the center point and half the width and height.
+    // c (half width) and d (half height) could be negative, so use the absolute value
+    // in calculating the top left corner (x, y).
+    bbox = { x: a - Math.abs(c), y: b - Math.abs(d), w: 2 * c, h: 2 * d };
+
   } else if (mode === constants.CENTER) {
-    return { x: a - c * 0.5, y: b - d * 0.5, w: c, h: d };
+
+    // CENTER mode uses the center point, width and height.
+    // c (width) and d (height) could be negative, so use the absolute value
+    // in calculating the top-left corner (x,y).
+    bbox = { x: a - Math.abs(c * 0.5), y: b - Math.abs(d * 0.5), w: c, h: d };
+
   }
+
+  // p5 supports negative width and heights for rectangles, ellipses and arcs
+  bbox.w = Math.abs(bbox.w);
+  bbox.h = Math.abs(bbox.h);
+
+  return bbox;
 }
 
 export default { modeAdjust };

src/core/shape/2d_primitives.js

@@ -327,10 +327,6 @@ p5.prototype.arc = function(x, y, w, h, start, stop, mode, detail) {
   start = this._toRadians(start);
   stop = this._toRadians(stop);
 
-  // p5 supports negative width and heights for ellipses
-  w = Math.abs(w);
-  h = Math.abs(h);
-
   const vals = canvas.modeAdjust(x, y, w, h, this._renderer._ellipseMode);
   const angles = this._normalizeArcAngles(start, stop, vals.w, vals.h, true);
 
@@ -547,16 +543,9 @@ p5.prototype._renderEllipse = function(x, y, w, h, detailX) {
     return this;
   }
 
-  // p5 supports negative width and heights for rects
-  if (w < 0) {
-    w = Math.abs(w);
-  }
-
+  // Duplicate 3rd argument if only 3 given.
   if (typeof h === 'undefined') {
-    // Duplicate 3rd argument if only 3 given.
     h = w;
-  } else if (h < 0) {
-    h = Math.abs(h);
   }
 
   const vals = canvas.modeAdjust(x, y, w, h, this._renderer._ellipseMode);

test/node/helpers.js

@@ -15,7 +15,7 @@ suite('helpers/modeAdjust', function() {
   });
   test('should set mode to corners', function() {
     result = helpers.modeAdjust(a, b, c, d, constants.CORNERS);
-    expect(result).to.eql({ x: 100, y: 200, w: -50, h: -50 });
+    expect(result).to.eql({ x: 50, y: 150, w: 50, h: 50 });
   });
   test('should set mode to radius', function() {
     result = helpers.modeAdjust(a, b, c, d, constants.RADIUS);

test/unit/spec.js

@@ -52,7 +52,8 @@ var spec = {
     // Add the visual tests that you want run as part of CI here. Feel free
     // to omit some for speed if they should only be run manually.
     'webgl',
-    'typography'
+    'typography',
+    'shape_modes'
   ]
 };
 document.write(

test/unit/visual/cases/shape_modes.js

@@ -0,0 +1,118 @@
+/*
+  Helper function that draws a shape using the specified shape mode
+  p5 ............... The p5 Instance
+  shape ............ The shape to draw. Either 'ellipse', 'arc', or 'rect'
+  mode ............. The ellipseMode (for ellipse and arc), or rectMode (for rect)
+                     Either p5.CORNERS, p5.CORNER, p5.CENTER or p5.RADIUS
+  x1, x2, x2, y2 ... Coordinates specifying the shape in CORNERS mode,
+                     i.e. (x1, y1) and (x2, y2) specify two opposite corners P1 and P2
+*/
+function shapeCorners(p5, shape, mode, x1, y1, x2, y2) {
+  // Adjust coordinates for testing modes other than CORNERS
+  if (mode === p5.CORNER) {
+    // Find top left corner
+    let x = p5.min(x1, x2);  // x
+    let y = p5.min(y1, y2);  // y
+    // Calculate width and height
+    // Don't use abs(), so we get negative values as well
+    let w = x2 - x1; // w
+    let h = y2 - y1; // h
+    x1 = x; y1 = y; x2 = w; y2 = h;
+  } else if (mode === p5.CENTER) {
+    // Find center
+    let x = (x2 + x1) / 2; // x
+    let y = (y2 + y1) / 2; // y
+    // Calculate width and height
+    // Don't use abs(), so we get negative values as well
+    let w = x2 - x1;
+    let h = y2 - y1;
+    x1 = x; y1 = y; x2 = w; y2 = h;
+  } else if (mode === p5.RADIUS) {
+    // Find Center
+    let x = (x2 + x1) / 2; // x
+    let y = (y2 + y1) / 2; // y
+    // Calculate radii
+    // Don't use abs(), so we get negative values as well
+    let r1 = (x2 - x1) / 2; // r1;
+    let r2 = (y2 - y1) / 2; // r2
+    x1 = x; y1 = y; x2 = r1; y2 = r2;
+  }
+
+  if (shape === 'ellipse') {
+    p5.ellipseMode(mode);
+    p5.ellipse(x1, y1, x2, y2);
+  } else if (shape === 'arc') {
+    // Draw four arcs with gaps inbetween
+    const GAP = p5.radians(20);
+    p5.ellipseMode(mode);
+    p5.arc(x1, y1, x2, y2, 0 + GAP, p5.HALF_PI - GAP);
+    p5.arc(x1, y1, x2, y2, p5.HALF_PI + GAP, p5.PI - GAP);
+    p5.arc(x1, y1, x2, y2, p5.PI + GAP, p5.PI + p5.HALF_PI - GAP);
+    p5.arc(x1, y1, x2, y2, p5.PI + p5.HALF_PI + GAP, p5.TWO_PI - GAP);
+  } else if (shape === 'rect') {
+    p5.rectMode(mode);
+    p5.rect(x1, y1, x2, y2);
+  }
+}
+
+
+/*
+  Comprehensive test for rendering ellipse(), arc(), and rect()
+  with the different ellipseMode() / rectMode() values: CORNERS, CORNER, CENTER, RADIUS.
+  Each of the 3 shapes is tested with each of the 4 possible modes, resulting in 12 test.
+  Each test renders the shape in 16 different coordinate configurations,
+  testing combinations of positive and negative coordinate values.
+*/
+visualSuite('Shape Modes', function(...args) {
+  // Shapes to test
+  const SHAPES = [ 'ellipse', 'arc', 'rect' ];
+
+  // Modes to test (used with ellipseMode or rectMode, according to shape)
+  const MODES  = [ 'CORNERS', 'CORNER', 'CENTER', 'RADIUS' ];
+
+  for (let shape of SHAPES) {
+    visualSuite(`Shape ${shape}`, function() {
+
+      for (let mode of MODES) {
+        visualTest(`Mode ${mode}`, function(p5, screenshot) {
+          p5.createCanvas(60, 125);
+          p5.translate(p5.width/2, p5.height/2);
+
+          // Make the following calls to shapeCorners shorter
+          // by omitting p5, shape and mode parameters
+          function _shapeCorners(x1, y1, x2, y2) {
+            shapeCorners(p5, shape, p5[mode], x1, y1, x2, y2);
+          }
+
+          // Quadrant I (Bottom Right)
+          //              P1      P2
+          _shapeCorners( 5,  5, 25, 15); // P1 Top Left,     P2 Bottom Right
+          _shapeCorners( 5, 20, 25, 30); // P1 Bottom Left,  P2 Top Right
+          _shapeCorners(25, 45,  5, 35); // P1 Bottom Right, P2 Top Left
+          _shapeCorners(25, 50,  5, 60); // P1 Top Right,    P2 Bottom Left
+
+          // Quadrant II (Bottom Left)
+          _shapeCorners(-25,  5,  -5, 15);
+          _shapeCorners(-25, 20,  -5, 30);
+          _shapeCorners( -5, 45, -25, 35);
+          _shapeCorners( -5, 50, -25, 60);
+
+          // Quadrant III (Top Left)
+          _shapeCorners(-25, -60,  -5, -50);
+          _shapeCorners(-25, -35,  -5, -45);
+          _shapeCorners( -5, -20, -25, -30);
+          _shapeCorners( -5, -15, -25,  -5);
+
+          // Quadrant IV (Top Right)
+          _shapeCorners( 5, -60, 25, -50);
+          _shapeCorners( 5, -35, 25, -45);
+          _shapeCorners(25, -20,  5, -30);
+          _shapeCorners(25, -15,  5,  -5);
+
+          screenshot();
+        }); // End of: visualTest
+      } // End of: MODES loop
+
+    }); // End of: Inner visualSuite
+  } // End of: SHAPES loop
+}); // End of: Outer visualSuite

test/unit/visual/screenshots/Shape Modes/Shape arc/Mode CENTER/metadata.json

@@ -0,0 +1,3 @@
+{
+  "numScreenshots": 1
+}

test/unit/visual/screenshots/Shape Modes/Shape arc/Mode CORNER/metadata.json

@@ -0,0 +1,3 @@
+{
+  "numScreenshots": 1
+}

test/unit/visual/screenshots/Shape Modes/Shape arc/Mode CORNERS/metadata.json

@@ -0,0 +1,3 @@
+{
+  "numScreenshots": 1
+}

test/unit/visual/screenshots/Shape Modes/Shape arc/Mode RADIUS/metadata.json

@@ -0,0 +1,3 @@
+{
+  "numScreenshots": 1
+}

test/unit/visual/screenshots/Shape Modes/Shape ellipse/Mode CENTER/metadata.json

@@ -0,0 +1,3 @@
+{
+  "numScreenshots": 1
+}

test/unit/visual/screenshots/Shape Modes/Shape ellipse/Mode CORNER/metadata.json

@@ -0,0 +1,3 @@
+{
+  "numScreenshots": 1
+}

test/unit/visual/screenshots/Shape Modes/Shape ellipse/Mode CORNERS/metadata.json

@@ -0,0 +1,3 @@
+{
+  "numScreenshots": 1
+}

test/unit/visual/screenshots/Shape Modes/Shape ellipse/Mode RADIUS/metadata.json

@@ -0,0 +1,3 @@
+{
+  "numScreenshots": 1
+}

test/unit/visual/screenshots/Shape Modes/Shape rect/Mode CENTER/metadata.json

@@ -0,0 +1,3 @@
+{
+  "numScreenshots": 1
+}

test/unit/visual/screenshots/Shape Modes/Shape rect/Mode CORNER/metadata.json

@@ -0,0 +1,3 @@
+{
+  "numScreenshots": 1
+}

test/unit/visual/screenshots/Shape Modes/Shape rect/Mode CORNERS/metadata.json

@@ -0,0 +1,3 @@
+{
+  "numScreenshots": 1
+}

test/unit/visual/screenshots/Shape Modes/Shape rect/Mode RADIUS/metadata.json

@@ -0,0 +1,3 @@
+{
+  "numScreenshots": 1
+}

test/unit/visual/visualTest.js

@@ -59,6 +59,11 @@ window.checkMatch = function(actual, expected, p5) {
       Math.ceil(img.height * scale)
     );
   }
+  // The below algorithm to calculate differences can produce false negatives in certain cases.
+  // Immediately return true for exact matches.
+  if (toBase64(expected) === toBase64(actual)) {
+    return { ok: true };
+  }
   const diff = p5.createImage(actual.width, actual.height);
   diff.drawingContext.drawImage(actual.canvas, 0, 0);
   diff.drawingContext.globalCompositeOperation = 'difference';

test/visual/visualTestList.js

@@ -1,5 +1,5 @@
 // List all visual test files here that should be manually run
-const visualTestList = ['webgl', 'typography'];
+const visualTestList = ['webgl', 'typography', 'shape_modes'];
 
 for (const file of visualTestList) {
   document.write(