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

Refactor primitive meshing #12793

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d430554
Add files via upload
gibletfeets Mar 29, 2024
c421e29
Add files via upload
gibletfeets Mar 29, 2024
90963cd
Added a benchmark for the torus mesh generation function.
gibletfeets Mar 29, 2024
86e6de4
Added torus generation benchmark.
gibletfeets Mar 29, 2024
512084e
Merge branch 'main' of https://github.com/gibletfeets/bevy
gibletfeets Mar 29, 2024
da14a71
Update torus.rs
gibletfeets Mar 29, 2024
19017ba
Update torus.rs
gibletfeets Mar 29, 2024
6e356cd
Refactored circle generation logic in primitive meshes.
gibletfeets Mar 30, 2024
5a31d31
Update crates/bevy_render/src/mesh/primitives/circle_iterator.rs
gibletfeets Mar 30, 2024
1bd3e48
Update crates/bevy_render/src/mesh/primitives/circle_iterator.rs
gibletfeets Mar 30, 2024
d08fae3
Update crates/bevy_render/src/mesh/primitives/circle_iterator.rs
gibletfeets Mar 30, 2024
dd0725e
Merge branch 'main' into refactor-primitive-meshing
gibletfeets Mar 30, 2024
8049680
Merge branch 'main' into refactor-primitive-meshing
gibletfeets Mar 30, 2024
8f8c980
Merge branch 'refactor-primitive-meshing' of https://github.com/gible…
gibletfeets Mar 30, 2024
c5d4341
Merge branch 'refactor-primitive-meshing' of https://github.com/gible…
gibletfeets Mar 30, 2024
9ba2b9e
Merge branch 'refactor-primitive-meshing' of https://github.com/gible…
gibletfeets Mar 30, 2024
cfbb22a
amogus
gibletfeets Mar 30, 2024
8e7ca84
Merge branch 'bevyengine:main' into refactor-primitive-meshing
gibletfeets Apr 1, 2024
2b8f50b
Changed usage of circle iterator, made benchmarks actually functional.
gibletfeets Apr 1, 2024
025591d
Merge branch 'refactor-primitive-meshing' of https://github.com/gible…
gibletfeets Apr 1, 2024
188d654
Changed iterator usage, made benchmark actually functional
gibletfeets Apr 1, 2024
57176e8
Merge branch 'bevyengine:main' into refactor-primitive-meshing
gibletfeets Apr 3, 2024
f4bafb9
Refactored capsule, optimized circle_iterator.
gibletfeets Apr 5, 2024
fcbb23b
Merge branch 'refactor-primitive-meshing' of https://github.com/gible…
gibletfeets Apr 5, 2024
de58e4f
Merge branch 'main' into refactor-primitive-meshing
gibletfeets Apr 5, 2024
5792f15
Forgot to rename setter functions.
gibletfeets Apr 5, 2024
980cbdc
Merge branch 'refactor-primitive-meshing' of https://github.com/gible…
gibletfeets Apr 5, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions benches/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ name = "torus"
path = "benches/bevy_render/torus.rs"
harness = false

[[bench]]
name = "capsule"
path = "benches/bevy_render/capsule.rs"
Copy link
Contributor

Choose a reason for hiding this comment

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

Torus and capsule could probably live together in primitives.rs

harness = false


[[bench]]
name = "entity_hash"
path = "benches/bevy_ecs/world/entity_hash.rs"
Expand Down
52 changes: 52 additions & 0 deletions benches/benches/bevy_render/capsule.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
use criterion::{black_box, criterion_group, criterion_main, Criterion};

use bevy_render::mesh::Capsule3dMeshBuilder;

criterion_group!(benches, capsule_low_res, capsule_default, capsule_high_res);
fn capsule_low_res(c: &mut Criterion) {
c.bench_function("build_capsule_low_res", |b| {
b.iter(|| {
black_box(
Capsule3dMeshBuilder::new(
black_box(0.5),
black_box(1.0),
black_box(16),
black_box(4),
)
.build(),
)
});
});
}
fn capsule_default(c: &mut Criterion) {
c.bench_function("build_capsule_default", |b| {
b.iter(|| {
black_box(
Capsule3dMeshBuilder::new(
black_box(0.5),
black_box(1.0),
black_box(32),
black_box(8),
)
.build(),
)
});
});
}

fn capsule_high_res(c: &mut Criterion) {
c.bench_function("build_capsule_high_res", |b| {
b.iter(|| {
black_box(
Capsule3dMeshBuilder::new(
black_box(0.5),
black_box(1.0),
black_box(64),
black_box(16),
)
.build(),
)
});
});
}
criterion_main!(benches);
38 changes: 31 additions & 7 deletions benches/benches/bevy_render/torus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,38 @@ use criterion::{black_box, criterion_group, criterion_main, Criterion};

use bevy_render::mesh::TorusMeshBuilder;

fn torus(c: &mut Criterion) {
c.bench_function("build_torus", |b| {
b.iter(|| black_box(TorusMeshBuilder::new(black_box(0.5),black_box(1.0))));
criterion_group!(benches, torus_low_res, torus_default, torus_high_res,);

fn torus_low_res(c: &mut Criterion) {
c.bench_function("build_torus_low_res", |b| {
b.iter(|| {
black_box(
TorusMeshBuilder::new(black_box(0.5), black_box(1.0))
.minor_resolution(black_box(12))
.major_resolution(black_box(16))
Copy link
Contributor

Choose a reason for hiding this comment

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

Its interesting that torus needs the builder-style api to set the resolution while Capsule (and Sphere?) have them set from new but then also have builder methods. Would be nice to make them all consistent

.build(),
)
});
});
}

fn torus_default(c: &mut Criterion) {
c.bench_function("build_torus_default", |b| {
b.iter(|| black_box(TorusMeshBuilder::new(black_box(0.5), black_box(1.0)).build()));
});
}

fn torus_high_res(c: &mut Criterion) {
c.bench_function("build_torus_high_res", |b| {
b.iter(|| {
black_box(
TorusMeshBuilder::new(black_box(0.5), black_box(1.0))
.minor_resolution(black_box(48))
.major_resolution(black_box(64))
.build(),
)
});
});
}

criterion_group!(
benches,
torus,
);
criterion_main!(benches);
83 changes: 83 additions & 0 deletions crates/bevy_render/src/mesh/primitives/circle_iterator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
use bevy_math::DMat2;
use bevy_math::DVec2;
use bevy_math::Vec2;
use std::f64::consts::TAU;

//iterator for generating a set of points around a unit circle, beginning at 0 radians
#[derive(Debug, Clone)]
pub(crate) struct CircleIterator {
count: usize,
rot: DMat2,
pos: DVec2,
}

impl CircleIterator {
//produces an iterator over (count) equidistant points that starts at (1.0, 0.0) on the unit circle
pub(crate) fn new(count: usize) -> CircleIterator {
Self {
count,
rot: DMat2::from_angle(TAU / (count as f64)),
pos: DVec2::new(1.0, 0.0),
}
}

//produces an iterator over (count) equidistant points that starts at (1.0, 0.0) on the unit circle, with an additional end point of (1.0, 0.0)
pub(crate) fn wrapping(count: usize) -> impl Iterator<Item = Vec2> {
Self::new(count).chain(std::iter::once(Vec2::new(1.0, 0.0)))
}

//semicircle with points ranging from 0 radians to pi radians, with (count) regions between points.
pub(crate) fn semicircle(count: usize) -> impl Iterator<Item = Vec2> {
Self::new(count * 2)
.take(count)
.chain(std::iter::once(Vec2::new(-1.0, 0.0)))
}

//quarter circle with points ranging from 0 radians to pi/2 radians, with (count) regions between points.
pub(crate) fn quarter_circle(count: usize) -> impl Iterator<Item = Vec2> {
Self::new(count * 4)
.take(count)
.chain(std::iter::once(Vec2::new(0.0, 1.0)))
}
}
impl Iterator for CircleIterator {
type Item = Vec2;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just opening a discussion here:

Thoughts on using Vec2 here? Having the resulting values accessed by .x and .y seems sort of confusing.

Maybe (f32, f32) or perhaps SinCos { sin: f32, cos: f32 }?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Vec2 was chosen because the first alternative to sin/cos calls that I wanted to investigate was application of a Mat2 for rotation, which would get generated on the initialization of the iterator - which could then be subsequently cloned for creation of identical sets of unit circle points.

If this method does not yield favorable results, I agree that something else should be used (and if it does, it would still make sense to have it return something with labels that are more intuitive than .x, .y).

fn next(&mut self) -> Option<Self::Item> {
if self.count != 0 {
let prev = self.pos.as_vec2();
self.pos = self.rot * self.pos;
self.count -= 1;
Some(prev)
} else {
None
}
}
}

#[cfg(test)]
mod tests {
gibletfeets marked this conversation as resolved.
Show resolved Hide resolved

use super::CircleIterator;
use bevy_math::Vec2;
#[test]
fn circle_iterator_has_correct_length() {
let vertices: Vec<Vec2> = CircleIterator::new(6).collect();
assert_eq!(vertices.len(), 6);
}

#[test]
fn circle_iterator_vertices_are_equidistant() {
let epsilon = 0.00001;
let vertices: Vec<Vec2> = CircleIterator::new(6).collect();
let center = Vec2::new(0.0, 0.0);
let distances_center: Vec<f32> = vertices.iter().map(|x| center.distance(*x)).collect();
assert!(distances_center
.windows(2)
.all(|w| (w[0] - w[1]).abs() < epsilon));
let distances_neighbors: Vec<f32> =
vertices.windows(2).map(|w| w[0].distance(w[1])).collect();
assert!(distances_neighbors
.windows(2)
.all(|w| (w[0] - w[1]).abs() < epsilon));
}
}