bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Add `Meshable` trait and implement meshing for 2D primitives

Open Jondolf opened this issue 5 months ago • 13 comments

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.

/// 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:

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:

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

After:

New 2D shapes

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


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

Jondolf avatar Jan 20 '24 11:01 Jondolf

I think we should also consider re-exporting the primitives so that you don't need primitives::foo everywhere. They're meant to be used all over the place, for e.g. meshes, bounding volumes, and eventually gizmos and colliders, so I think having them in the prelude is justified.

I'll probably make a separate PR for this now

Jondolf avatar Jan 20 '24 11:01 Jondolf

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

github-actions[bot] avatar Jan 20 '24 11:01 github-actions[bot]

@ManevilleF has done similar things in hexx for hexagonal meshes: could you give us a review?

alice-i-cecile avatar Jan 20 '24 15:01 alice-i-cecile

I like it ! it also could allow for user custom primitives.

Some concerns:

  • I'm not a fan of the Facing enum, why not use a Vec3 directly ? like mesh.facing(Vec3::Z), it would allow for more rotation options
  • I think the builders should not take in account the shape agnostic options like facing, offset, etc which all work the same, changing the facing means rotating the vertex positions and normals, changing offset just offsets the vertex positions, etc. those methods could be added to Mesh instead and applied by the builders

ManevilleF avatar Jan 21 '24 10:01 ManevilleF

I'm not a fan of the Facing enum, why not use a Vec3 directly ? like mesh.facing(Vec3::Z), it would allow for more rotation options

Yeah, I could maybe do this instead. It would just be more expensive since you'd need to rotate everything afterwards instead of having the vertex positions "predefined" as rotated. But this cost probably isn't that substantial in practise.

I'll try this and see if there are any larger issues that would make this unviable

I think the builders should not take in account the shape agnostic options like facing, offset, etc which all work the same, changing the facing means rotating the vertex positions and normals, changing offset just offsets the vertex positions, etc. those methods could be added to Mesh instead and applied by the builders

I believe EllipseMeshBuilder::build_mesh_data is currently the only method that has an offset, and it's not public. The reason it has it is to allow reusing the same implementation for e.g. bases of 3D primitives like cylinders, as I briefly mentioned earlier.

My code for adding bases to cylinders currently looks like this:

// Top and bottom
let base = CircleMesh::new(self.cylinder.radius, self.resolution as usize);
base.facing_y().build_mesh_data(
    Vec3::Y * self.cylinder.half_height,
    &mut indices,
    &mut positions,
    &mut normals,
    &mut uvs,
);
base.facing_neg_y().build_mesh_data(
    Vec3::NEG_Y * self.cylinder.half_height,
    &mut indices,
    &mut positions,
    &mut normals,
    &mut uvs,
);

I need the offset so that the top and bottom are at the correct positions, and the facing direction is needed so that they're oriented correctly. I don't think it's possible to efficiently pass in a Mesh and just extend its positions, normals, and so on, which is why they're passed separately and the Mesh is only created at the end. This is somewhat similar to what bevy_more_shapes does with its MeshData struct, although it doesn't reuse code as much for e.g. the cylinder top and bottom.

Edit: I think I could actually just pass a Mesh to build_mesh_data... Or maybe even add a merge function to Mesh for combining meshes... I'll play around with this a bit

Jondolf avatar Jan 21 '24 10:01 Jondolf

Another reason for not having facing on Mesh is that you'd need to first build the mesh, which makes the API slightly less convenient:

// Now
meshes.add(Circle::default().mesh().facing_y())

// With facing on Mesh
meshes.add(Circle::default().mesh().build().facing_y())

Probably not a huge deal though. I think a transform_by on Mesh would be valuable either way.

Jondolf avatar Jan 21 '24 10:01 Jondolf

Another reason for not having facing on Mesh is that you'd need to first build the mesh, which makes the API slightly less convenient:

// Now
meshes.add(Circle::default().mesh().facing_y())

// With facing on Mesh
meshes.add(Circle::default().mesh().build().facing_y())

Probably not a huge deal though. I think a transform_by on Mesh would be valuable either way.

It's not incompatible, the facing method can be on the builder but be applied to Mesh. That's what I do in hexx mesh

ManevilleF avatar Jan 21 '24 14:01 ManevilleF

Basically my suggestion is that those primitive mesh builders output a Mesh, which has generic methods like facing, rotated, offsetted, scaled, or whatever.

So the builder only has to think about building a Mesh, and not care about transformation specifics, as it will simply call the appropriate Mesh methods.

For example, all hexx mesh builders output a MeshInfo and their facing() or offset() method directly call the MeshInfo methods

ManevilleF avatar Jan 21 '24 14:01 ManevilleF

Gotcha, makes sense. I already made PRs to add transformation methods (#11454) and merging support (#11456) for Mesh directly since they're useful to have anyway. It might be a bit expensive compared to a separate MeshInfo/MeshData struct though, so I'll probably add that too

Jondolf avatar Jan 21 '24 14:01 Jondolf

I don't know if an extra struct is needed, I have one in hexx because bevy is not a dependency of the lib

ManevilleF avatar Jan 21 '24 15:01 ManevilleF

I think it would actually make sense to just remove the stuff related to facing for now and to split it up into a future PR. It's a feature that doesn't exist for the old/current shapes, so it's probably not needed for initial primitive meshing support either.

Moving facing things to a follow-up PR might make the changes in this PR more trivial and easier to review, which would help us move forward with meshing support quicker. Some 3D shapes might end up havIng a bit more code duplication (because e.g. Circle can't be reused), but that's also the way the current shapes work, so it wouldn't be a regression in that sense.

Jondolf avatar Jan 22 '24 16:01 Jondolf

Yep, let's split that out!

alice-i-cecile avatar Jan 22 '24 17:01 alice-i-cecile

Alright, I've made some changes:

  • Removed Facing and other code related to transformations.
  • Removed builder structs from RegularPolygon, Rectangle, and Triangle2d; without facing they only store the shape, so it makes more sense to just return a Mesh directly.
  • Moved all 2D meshing to a single dim2 file for now. This is more consistent with primitive code in other crates (e.g. bevy_math::primitives::dim2) and some files were only about 30 lines of code when the impls were separate. We can always split this up as needed later on.

One extra thing to note is that Bevy's "old" Quad mesh has a flip property for flipping the UVs horizontally. I didn't add this to Rectangle yet because (1) I think you could just flip the texture in some other way, and (2) I'm not sure why only quads would support this and not other meshes. I could add it if it's actually valuable and I'm missing something though

Jondolf avatar Jan 22 '24 20:01 Jondolf