Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ellipseMode(CORNERS) and rectMode(CORNER) #7290

Merged
merged 7 commits into from
Oct 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
</tbody>
</table>
Expand Down
39 changes: 35 additions & 4 deletions src/core/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
13 changes: 1 addition & 12 deletions src/core/shape/2d_primitives.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion test/node/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion test/unit/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
118 changes: 118 additions & 0 deletions test/unit/visual/cases/shape_modes.js
Original file line number Diff line number Diff line change
@@ -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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
5 changes: 5 additions & 0 deletions test/unit/visual/visualTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
2 changes: 1 addition & 1 deletion test/visual/visualTestList.js
Original file line number Diff line number Diff line change
@@ -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(
Expand Down
Loading