Skip to content

Commit

Permalink
Merge pull request #7363 from martinleopold/ellipse-fix
Browse files Browse the repository at this point in the history
Negative dimensions will mirror rect() again
  • Loading branch information
limzykenneth authored Nov 22, 2024
2 parents 33883e5 + ebd00c2 commit 4180fd1
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 19 deletions.
41 changes: 31 additions & 10 deletions src/core/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,35 +15,56 @@ function modeAdjust(a, b, c, d, mode) {

if (mode === constants.CORNER) {

// CORNER mode already corresponds to a bounding box (top-left corner, width, height)
bbox = { x: a, y: b, w: c, h: d };
// CORNER mode already corresponds to a bounding box (top-left corner, width, height).
// For negative widhts or heights, the absolute value is used.
bbox = {
x: a,
y: b,
w: Math.abs(c),
h: Math.abs(d)
};

} else if (mode === constants.CORNERS) {

// 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 };
bbox = {
x: Math.min(a, c),
y: Math.min(b, d),
w: Math.abs(c - a),
h: Math.abs(d - b)
};

} else if (mode === constants.RADIUS) {

// 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 };
c = Math.abs(c);
d = Math.abs(d);
bbox = {
x: a - c,
y: b - d,
w: 2 * c,
h: 2 * d
};

} else if (mode === constants.CENTER) {

// 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 };
// in calculating the top-left corner (x, y).
c = Math.abs(c);
d = Math.abs(d);
bbox = {
x: a - (c * 0.5),
y: b - (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;
}

Expand Down
9 changes: 9 additions & 0 deletions src/core/shape/2d_primitives.js
Original file line number Diff line number Diff line change
Expand Up @@ -1339,6 +1339,15 @@ p5.prototype._renderRect = function() {
this._renderer._rectMode
);

// For the default rectMode (CORNER), restore a possible negative width/height
// removed by modeAdjust(). This results in flipped/mirrored rendering,
// which is especially noticable when using WEGBL rendering and texture().
// Note that this behavior only applies to rect(), NOT to ellipse() and arc().
if (this._renderer._rectMode === constants.CORNER) {
vals.w = arguments[2];
vals.h = arguments[3];
}

const args = [vals.x, vals.y, vals.w, vals.h];
// append the additional arguments (either cornder radii, or
// segment details) to the argument list
Expand Down
79 changes: 70 additions & 9 deletions test/unit/visual/cases/shape_modes.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ function shapeCorners(p5, shape, mode, x1, y1, x2, y2) {
// Don't use abs(), so we get negative values as well
let w = x2 - x1; // w
let h = y2 - y1; // h
// With mode CORNER, rects with negative widths/heights result in mirrored/flipped rendering
// In this case, adjust position so the rect is in line with the other cases
if (shape === 'rect') {
if (w < 0) { x += (-w); } // Move right
if (h < 0) { y += (-h); } // Move down
}
x1 = x; y1 = y; x2 = w; y2 = h;
} else if (mode === p5.CENTER) {
// Find center
Expand Down Expand Up @@ -56,14 +62,15 @@ function shapeCorners(p5, shape, mode, 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) {
/*
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 tests.
Each test renders the shape in 16 different coordinate configurations,
testing combinations of positive and negative coordinate values.
*/

// Shapes to test
const SHAPES = [ 'ellipse', 'arc', 'rect' ];

Expand Down Expand Up @@ -113,6 +120,60 @@ visualSuite('Shape Modes', function(...args) {
}); // End of: visualTest
} // End of: MODES loop

}); // End of: Inner visualSuite
}); // End of: visualSuite
} // End of: SHAPES loop
}); // End of: Outer visualSuite


/*
An extra test suite specific to shape mode CORNER and negative dimensions.
For rect, negative widths/heights result in flipped rendering (horizontally/vertically)
For ellipse and arc, using negative widths/heights has no effect (the absolute value is used)
*/
visualSuite('Negative dimensions', function() {
// Negative widths/height result in flipped rects.
visualTest('rect', function(p5, screenshot) {
p5.createCanvas(50, 50);
p5.translate(p5.width/2, p5.height/2);
p5.rectMode(p5.CORNER);
p5.rect(0, 0, 20, 10);
p5.fill('red');
p5.rect(0, 0, -20, 10);
p5.fill('green');
p5.rect(0, 0, 20, -10);
p5.fill('blue');
p5.rect(0, 0, -20, -10);
screenshot();
});
// Since negative widths/heights are used with their absolute value,
// ellipses are drawn on top of each other, blue one last
visualTest('ellipse', function(p5, screenshot) {
p5.createCanvas(50, 50);
p5.translate(p5.width/2, p5.height/2);
p5.ellipseMode(p5.CORNER);
p5.ellipse(0, 0, 20, 10);
p5.fill('red');
p5.ellipse(0, 0, -20, 10);
p5.fill('green');
p5.ellipse(0, 0, 20, -10);
p5.fill('blue');
p5.ellipse(0, 0, -20, -10);
screenshot();
});
// Since negative widths/heights are used with their absolute value,
// arcs are drawn on top of each other, blue one last.
visualTest('arc', function(p5, screenshot) {
p5.createCanvas(50, 50);
p5.translate(p5.width/2, p5.height/2);
p5.ellipseMode(p5.CORNER);
p5.arc(0, 0, 20, 10, 0, p5.PI + p5.HALF_PI);
p5.fill('red');
p5.arc(0, 0, -20, 10, 0, p5.PI + p5.HALF_PI);
p5.fill('green');
p5.arc(0, 0, 20, -10, 0, p5.PI + p5.HALF_PI);
p5.fill('blue');
p5.arc(0, 0, -20, -10, 0, p5.PI + p5.HALF_PI);
screenshot();
});
});

});
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.

0 comments on commit 4180fd1

Please sign in to comment.