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

Meshes from primitives #10569

Closed
Jondolf opened this issue Nov 15, 2023 · 5 comments
Closed

Meshes from primitives #10569

Jondolf opened this issue Nov 15, 2023 · 5 comments
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible

Comments

@Jondolf
Copy link
Contributor

Jondolf commented Nov 15, 2023

What problem does this solve or what need does it fill?

Meshes can currently be created from the shapes in bevy_render::mesh::shape. These are shapes that are specific to rendering and contain information about things like how a sphere should be subdivided.

Now that primitive shapes have been added in #10466, it should be possible to have a more unified shape API for meshes, gizmos, colliders and so on. It should be possible to create meshes from the primitive shapes in bevy_math::primitives.

What solution would you like?

Implement mesh generation for primitive shapes, and maybe remove duplicate shapes from bevy_render::mesh::shape.

For some shapes, it's as straightforward as this:

let cube = Mesh::from(Cuboid::new(1.0, 1.0, 1.0)); // or just call .mesh()?

However, for many other shapes it's not as straightforward. For example, the current Capsule shape used for rendering has properties such as rings, latitudes, longitudes and uv_profile. This would not work with a simple impl From<Capsule> for Mesh.

Some options include:

  1. Keep separate types for rendering specifics, and create meshes from them like Mesh::from(UvSphere::new(Sphere::new(1.0), longitudes, latitudes)).
    • Similar to what we have now, pretty explicit and also familiar
    • Quite verbose and repeats Sphere
    • Annoying to have separate rendering shapes when one of the main ideas of shape primitives is to unify things
    • Rendering configuration is stored in a separate struct (good or bad depending on the use case)
  2. Have methods like mesh that are different for each primitive, like Sphere::new(1.0).uv_mesh(longitudes, latitudes)
    • Can be nice and concise
    • Shape primitives can be used directly instead of intermediary types
    • No shared trait for all types of meshes because of different requirements
    • No information about the rendering configuration is stored (good or bad depending on the use case)
  3. Hard-code a global default configuration for mesh quality / subdivision count
    • Very easy API, just Mesh::from(Sphere::new(1.0))
    • Very inflexible; there would still need to be a way to specify it manually as needed
    • Shapes don't always have a single subdivision property, but can have multiple like longitudes/latitudes

We don't want 3 by itself, but I'm personally leaning on a combination of 2 and 3. We could have hard-coded default implementations for From and maybe a Meshable trait that has a mesh method with some default mesh configuration implemented, as suggested by the original RFC, but also give primitives custom mesh construction methods like .uv_mesh(...) where necessary. This would provide a nice API with good defaults, without having many different shape types for the same core shapes (like spheres), while still allowing full control for subdivisions and other configuration as needed.

Ideas and suggestions are welcome!

@Jondolf Jondolf added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Nov 15, 2023
@nicopap nicopap added A-Rendering Drawing game state to the screen and removed S-Needs-Triage This issue needs to be labelled labels Nov 16, 2023
@Kanabenki
Copy link
Contributor

  1. Have methods like mesh that are different for each primitive, like Sphere::new(1.0).uv_mesh(longitudes, latitudes)

Wouldn't that make bevy_math depends on bevy_render for the Mesh definition, which I guess isn't desirable ? (unless this is done through extension traits in bevy_render)

@Jondolf
Copy link
Contributor Author

Jondolf commented Nov 21, 2023

Yep it'd be traits in bevy_render. Same concept applies to using primitives for gizmos, colliders and so on. bevy_math should only contain the primitives themselves, and the various things using them should be in their respective crates.

@IQuick143
Copy link
Contributor

Could we simplify the syntax of option 1. by having constructor methods on the primite shapes, which would return the desired rendering shape, ie:

// instead of
let mesh: Mesh = UvSphere::new(Sphere::new(radius), longitudes, latitudes).into();
let mesh: Mesh = IcoSphere::new(Sphere::new(radius), subdivisions).into();
// one would use
let mesh: Mesh = Sphere::new(radius).uv(longitudes, latitudes).into();
let mesh: Mesh = Sphere::new(radius).ico(subdivisions).into();

This would simplify the syntax for people who ultimately aim for a mesh (like option 2.), but it would still expose the UvSphere and similar types, if they were needed (are they needed?).

@Jondolf
Copy link
Contributor Author

Jondolf commented Dec 4, 2023

I'm attempting to implement mesh construction for primitives now, and I think I have found quite a nice approach that provides a unified API with both ergonomics and configurability. It's fairly similar to the suggestion by @IQuick143 above.

We can have a single Meshable trait like the original RFC suggests, but have the mesh output be an associated type:

pub trait Meshable {
    type Output;

    fn mesh(&self) -> Self::Output;
}

This way, something like Cuboid::mesh can directly return a mesh:

impl Meshable for Cuboid {
    type Output = Mesh;

    fn mesh(&self) -> Mesh {
        // ...
    }
}

But for e.g. a sphere, we can instead return a SphereBuilder:

pub enum SphereKind {
    Ico {
        subdivisions: usize,
    },
    Uv {
        sectors: usize,
        stacks: usize,
    },
}

pub struct SphereBuilder {
    pub sphere: Sphere,
    pub kind: SphereKind,
}

impl SphereBuilder {
    pub fn build(&self) -> Mesh { ... } // uses default configuration
    pub fn ico(&self, subdivisions: usize) -> Result<Mesh, IcosphereError> { ... }
    pub fn uv(&self, sectors: usize, stacks: usize) -> Mesh { ... }
}

impl Meshable for Sphere {
    type Output = SphereBuilder;

    fn mesh(&self) -> Self::Output {
        SphereBuilder {
            sphere: *self,
            kind: SphereKind::Ico { subdivisions: 5 }, // default config
        }
    }
}

If desired, you could even impl From<MyShape> for Mesh for the primitives and their builders so that you can still do Mesh::from(shape) or shape.into().

Using it would look like this:

let cube = Cuboid::new(1.0, 1.0, 1.0).mesh(); // or .into() if we implement that
let icosphere = Sphere { radius: 0.5 }.mesh().ico(10);
let uv_sphere = Sphere { radius: 0.5 }.mesh().uv(6, 8);
let circle = Circle { radius: 50.0 }.mesh().vertices(32);
let flipped_rectangle = Rectangle::new(100.0, 100.0).mesh().flipped();

I still need to implement this for some more shapes, but I should have a PR ready relatively soon.

github-merge-queue bot pushed a commit that referenced this issue Jan 29, 2024
# Objective

The first part of #10569, split up from #11007.

The goal is to implement meshing support for Bevy's new geometric
primitives, starting with 2D primitives. 3D meshing will be added in a
follow-up, and we can consider removing the old mesh shapes completely.

## Solution

Add a `Meshable` trait that primitives need to implement to support
meshing, as suggested by the
[RFC](https://github.com/bevyengine/rfcs/blob/main/rfcs/12-primitive-shapes.md#meshing).

```rust
/// A trait for shapes that can be turned into a [`Mesh`].
pub trait Meshable {
    /// The output of [`Self::mesh`]. This can either be a [`Mesh`]
    /// or a builder used for creating a [`Mesh`].
    type Output;

    /// Creates a [`Mesh`] for a shape.
    fn mesh(&self) -> Self::Output;
}
```

This PR implements it for the following primitives:

- `Circle`
- `Ellipse`
- `Rectangle`
- `RegularPolygon`
- `Triangle2d`

The `mesh` method typically returns a builder-like struct such as
`CircleMeshBuilder`. This is needed to support shape-specific
configuration for things like mesh resolution or UV configuration:

```rust
meshes.add(Circle { radius: 0.5 }.mesh().resolution(64));
```

Note that if no configuration is needed, you can even skip calling
`mesh` because `From<MyPrimitive>` is implemented for `Mesh`:

```rust
meshes.add(Circle { radius: 0.5 });
```

I also updated the `2d_shapes` example to use primitives, and tweaked
the colors to have better contrast against the dark background.

Before:

![Old 2D
shapes](https://github.com/bevyengine/bevy/assets/57632562/f1d8c2d5-55be-495f-8ed4-5890154b81ca)

After:

![New 2D
shapes](https://github.com/bevyengine/bevy/assets/57632562/f166c013-34b8-4752-800a-5517b284d978)

Here you can see the UVs and different facing directions: (taken from
#11007, so excuse the 3D primitives at the bottom left)

![UVs and facing
directions](https://github.com/bevyengine/bevy/assets/57632562/eaf0be4e-187d-4b6d-8fb8-c996ba295a8a)

---

## Changelog

- Added `bevy_render::mesh::primitives` module
- Added `Meshable` trait and implemented it for:
  - `Circle`
  - `Ellipse`
  - `Rectangle`
  - `RegularPolygon`
  - `Triangle2d`
- Implemented `Default` and `Copy` for several 2D primitives
- Updated `2d_shapes` example to use primitives
- Tweaked colors in `2d_shapes` example to have better contrast against
the (new-ish) dark background

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
tjamaan pushed a commit to tjamaan/bevy that referenced this issue Feb 6, 2024
…ine#11431)

# Objective

The first part of bevyengine#10569, split up from bevyengine#11007.

The goal is to implement meshing support for Bevy's new geometric
primitives, starting with 2D primitives. 3D meshing will be added in a
follow-up, and we can consider removing the old mesh shapes completely.

## Solution

Add a `Meshable` trait that primitives need to implement to support
meshing, as suggested by the
[RFC](https://github.com/bevyengine/rfcs/blob/main/rfcs/12-primitive-shapes.md#meshing).

```rust
/// A trait for shapes that can be turned into a [`Mesh`].
pub trait Meshable {
    /// The output of [`Self::mesh`]. This can either be a [`Mesh`]
    /// or a builder used for creating a [`Mesh`].
    type Output;

    /// Creates a [`Mesh`] for a shape.
    fn mesh(&self) -> Self::Output;
}
```

This PR implements it for the following primitives:

- `Circle`
- `Ellipse`
- `Rectangle`
- `RegularPolygon`
- `Triangle2d`

The `mesh` method typically returns a builder-like struct such as
`CircleMeshBuilder`. This is needed to support shape-specific
configuration for things like mesh resolution or UV configuration:

```rust
meshes.add(Circle { radius: 0.5 }.mesh().resolution(64));
```

Note that if no configuration is needed, you can even skip calling
`mesh` because `From<MyPrimitive>` is implemented for `Mesh`:

```rust
meshes.add(Circle { radius: 0.5 });
```

I also updated the `2d_shapes` example to use primitives, and tweaked
the colors to have better contrast against the dark background.

Before:

![Old 2D
shapes](https://github.com/bevyengine/bevy/assets/57632562/f1d8c2d5-55be-495f-8ed4-5890154b81ca)

After:

![New 2D
shapes](https://github.com/bevyengine/bevy/assets/57632562/f166c013-34b8-4752-800a-5517b284d978)

Here you can see the UVs and different facing directions: (taken from
bevyengine#11007, so excuse the 3D primitives at the bottom left)

![UVs and facing
directions](https://github.com/bevyengine/bevy/assets/57632562/eaf0be4e-187d-4b6d-8fb8-c996ba295a8a)

---

## Changelog

- Added `bevy_render::mesh::primitives` module
- Added `Meshable` trait and implemented it for:
  - `Circle`
  - `Ellipse`
  - `Rectangle`
  - `RegularPolygon`
  - `Triangle2d`
- Implemented `Default` and `Copy` for several 2D primitives
- Updated `2d_shapes` example to use primitives
- Tweaked colors in `2d_shapes` example to have better contrast against
the (new-ish) dark background

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
github-merge-queue bot pushed a commit that referenced this issue Feb 6, 2024
# Objective

Split up from #11007, fixing most of the remaining work for #10569.

Implement `Meshable` for `Cuboid`, `Sphere`, `Cylinder`, `Capsule`,
`Torus`, and `Plane3d`. This covers all shapes that Bevy has mesh
structs for in `bevy_render::mesh::shapes`.

`Cone` and `ConicalFrustum` are new shapes, so I can add them in a
follow-up, or I could just add them here directly if that's preferrable.

## Solution

Implement `Meshable` for `Cuboid`, `Sphere`, `Cylinder`, `Capsule`,
`Torus`, and `Plane3d`.

The logic is mostly just a copy of the the existing `bevy_render`
shapes, but `Plane3d` has a configurable surface normal that affects the
orientation. Some property names have also been changed to be more
consistent.

The default values differ from the old shapes to make them a bit more
logical:

- Spheres now have a radius of 0.5 instead of 1.0. The default capsule
is equivalent to the default cylinder with the sphere's halves glued on.
- The inner and outer radius of the torus are now 0.5 and 1.0 instead of
0.5 and 1.5 (i.e. the new minor and major radii are 0.25 and 0.75). It's
double the width of the default cuboid, half of its height, and the
default sphere matches the size of the hole.
- `Cuboid` is 1x1x1 by default unlike the dreaded `Box` which is 2x1x1.

Before, with "old" shapes:


![old](https://github.com/bevyengine/bevy/assets/57632562/733f3dda-258c-4491-8152-9829e056a1a3)

Now, with primitive meshing:


![new](https://github.com/bevyengine/bevy/assets/57632562/5a1af14f-bb98-401d-82cf-de8072fea4ec)

I only changed the `3d_shapes` example to use primitives for now. I can
change them all in this PR or a follow-up though, whichever way is
preferrable.

### Sphere API

Spheres have had separate `Icosphere` and `UVSphere` structs, but with
primitives we only have one `Sphere`.

We need to handle this with builders:

```rust
// Existing structs
let ico = Mesh::try_from(Icophere::default()).unwrap();
let uv = Mesh::from(UVSphere::default());

// Primitives
let ico = Sphere::default().mesh().ico(5).unwrap();
let uv = Sphere::default().mesh().uv(32, 18);
```

We could add methods on `Sphere` directly to skip calling `.mesh()`.

I also added a `SphereKind` enum that can be used with the `kind`
method:

```rust
let ico = Sphere::default()
    .mesh()
    .kind(SphereKind::Ico { subdivisions: 8 })
    .build();
```

The default mesh for a `Sphere` is an icosphere with 5 subdivisions
(like the default `Icosphere`).

---

## Changelog

- Implement `Meshable` and `Default` for `Cuboid`, `Sphere`, `Cylinder`,
`Capsule`, `Torus`, and `Plane3d`
- Use primitives in `3d_shapes` example

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
@james7132
Copy link
Member

This seems to be implemented as of 0.13. If there's still missing primitives, we can handle them in new PRs or issues, but this overall looks resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants