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

Solves issue #6787 #6809

Merged
merged 12 commits into from
Mar 13, 2024
40 changes: 40 additions & 0 deletions src/webgl/p5.Geometry.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ p5.Geometry = class Geometry {
//@type [p5.Vector]
this.vertices = [];

this.boundingBoxCache = null;


//an array containing every vertex for stroke drawing
this.lineVertices = new p5.DataArray();

Expand Down Expand Up @@ -74,6 +77,43 @@ p5.Geometry = class Geometry {
}
return this; // TODO: is this a constructor?
}
// Custom bounding box calculation based on the object's vertices
calculateBoundingBox() {
if (this.boundingBoxCache) {
return this.boundingBoxCache; // Return cached result if available
}

let minVertex = createVector(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, createVector called like this assumes p5 is in global mode, but it needs to also work in instance mode (where you have a p5 object, and all the previously global functions are referenced via p5.createVector.) Rather than trying to detect which mode we're in, maybe instead we can use the constructor syntax, which should work regardless of mode:

let minVertex = new p5.Vector(Number.MAX_VALUE, Number.MAX_VALUE, Number.MAX_VALUE);

Number.MAX_VALUE, Number.MAX_VALUE, Number.MAX_VALUE);
let maxVertex = createVector(
Number.MIN_VALUE, Number.MIN_VALUE, Number.MIN_VALUE);

for (let i = 0; i < this.vertices.length; i++) {
let vertex = this.vertices[i];
minVertex.x = min(minVertex.x, vertex.x);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see tests are still failing, min and max are also p5 globals and won't be defined in instance mode. I think Math.min and Math.max will fix it though

minVertex.y = min(minVertex.y, vertex.y);
minVertex.z = min(minVertex.z, vertex.z);

maxVertex.x = max(maxVertex.x, vertex.x);
maxVertex.y = max(maxVertex.y, vertex.y);
maxVertex.z = max(maxVertex.z, vertex.z);
}
// Calculate size and offset properties
let size = createVector(maxVertex.x - minVertex.x,
maxVertex.y - minVertex.y, maxVertex.z - minVertex.z);
let offset = createVector((minVertex.x + maxVertex.x) / 2,
(minVertex.y + maxVertex.y) / 2, (minVertex.z + maxVertex.z) / 2);

// Cache the result for future access
this.boundingBoxCache = {
min: minVertex,
max: maxVertex,
size: size,
offset: offset
};

return this.boundingBoxCache;
}

reset() {
this._hasFillTransparency = undefined;
Expand Down
14 changes: 14 additions & 0 deletions test/unit/webgl/p5.Geometry.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,20 @@ suite('p5.Geometry', function() {
assert.equal(geom._addJoin.callCount, 4);
});

test('calculateBoundingBox()', function() {
geom.vertices.push(
myp5.createVector(0, 0, 0),
myp5.createVector(10, 20, 30),
myp5.createVector(-5, 15, 25)
);
geom.calculateBoundingBox();
assert.deepEqual(geom.boundingBox.min.array(), [-5, 0, 0]);
assert.deepEqual(geom.boundingBox.max.array(), [10, 20, 30]);
assert.deepEqual(geom.boundingBox.size.array(), [15, 20, 30]);
assert.deepEqual(geom.boundingBox.offset.array(), [2.5, 10, 15]);
});


test('degenerate edge in the middle', function() {
geom.vertices.push(
myp5.createVector(0, 0),
Expand Down
Loading