Implement Meshable for some 3D primitives#11688
Merged
alice-i-cecile merged 7 commits intobevyengine:mainfrom Feb 6, 2024
Merged
Implement Meshable for some 3D primitives#11688alice-i-cecile merged 7 commits intobevyengine:mainfrom
Meshable for some 3D primitives#11688alice-i-cecile merged 7 commits intobevyengine:mainfrom
Conversation
41 tasks
NthTensor
reviewed
Feb 6, 2024
NthTensor
reviewed
Feb 6, 2024
Contributor
There was a problem hiding this comment.
Some thoughts:
- I like that the default primitives are more uniformly scaled. Getting any more specific than that would be super bikeshedy.
- I like the api in general. Good use of the builder pattern.
- I would prefer if
cuboid,plane,cylinderandtorusgot their own modules instead of living indim3.
Looks pretty good all in all.
NthTensor
approved these changes
Feb 6, 2024
Contributor
|
I did a review ass, the biggest thing is I don't understand why you're duplicating all this code instead of making use of existing code where it exists. Duplicated code is fragile; it would be much more preferable to build a |
Contributor
|
The plan is to remove the render shapes in a follow up. We are temporally copying rather than replacing to make the patch smaller and more manageable. |
alice-i-cecile
approved these changes
Feb 6, 2024
Member
alice-i-cecile
left a comment
There was a problem hiding this comment.
I'm content with this, but I'd like to remove the duplicated code ASAP.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 8, 2024
# Objective #11431 and #11688 implemented meshing support for Bevy's new geometric primitives. The next step is to deprecate the shapes in `bevy_render::mesh::shape` and to later remove them completely for 0.14. ## Solution Deprecate the shapes and reduce code duplication by utilizing the primitive meshing API for the old shapes where possible. Note that some shapes have behavior that can't be exactly reproduced with the new primitives yet: - `Box` is more of an AABB with min/max extents - `Plane` supports a subdivision count - `Quad` has a `flipped` property These types have not been changed to utilize the new primitives yet. --- ## Changelog - Deprecated all shapes in `bevy_render::mesh::shape` - Changed all examples to use new primitives for meshing ## Migration Guide Bevy has previously used rendering-specific types like `UVSphere` and `Quad` for primitive mesh shapes. These have now been deprecated to use the geometric primitives newly introduced in version 0.13. Some examples: ```rust let before = meshes.add(shape::Box::new(5.0, 0.15, 5.0)); let after = meshes.add(Cuboid::new(5.0, 0.15, 5.0)); let before = meshes.add(shape::Quad::default()); let after = meshes.add(Rectangle::default()); let before = meshes.add(shape::Plane::from_size(5.0)); // The surface normal can now also be specified when using `new` let after = meshes.add(Plane3d::default().mesh().size(5.0, 5.0)); let before = meshes.add( Mesh::try_from(shape::Icosphere { radius: 0.5, subdivisions: 5, }) .unwrap(), ); let after = meshes.add(Sphere::new(0.5).mesh().ico(5).unwrap()); ```
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
Split up from #11007, fixing most of the remaining work for #10569.
Implement
MeshableforCuboid,Sphere,Cylinder,Capsule,Torus, andPlane3d. This covers all shapes that Bevy has mesh structs for inbevy_render::mesh::shapes.ConeandConicalFrustumare 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
MeshableforCuboid,Sphere,Cylinder,Capsule,Torus, andPlane3d.The logic is mostly just a copy of the the existing
bevy_rendershapes, butPlane3dhas 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:
Cuboidis 1x1x1 by default unlike the dreadedBoxwhich is 2x1x1.Before, with "old" shapes:
Now, with primitive meshing:
I only changed the
3d_shapesexample 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
IcosphereandUVSpherestructs, but with primitives we only have oneSphere.We need to handle this with builders:
We could add methods on
Spheredirectly to skip calling.mesh().I also added a
SphereKindenum that can be used with thekindmethod:The default mesh for a
Sphereis an icosphere with 5 subdivisions (like the defaultIcosphere).Changelog
MeshableandDefaultforCuboid,Sphere,Cylinder,Capsule,Torus, andPlane3d3d_shapesexample