-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Merged by Bors] - Add RegularPolygon and Circle meshes #3730
Conversation
Another thing to note is that this is generating meshes like the pentagon on the right side: (edit: fixed this) Doing it like what is shown on the left would result in fewer triangles, but I'm not sure if there are any graphical implications with "tiny thin triangles" when using this to draw circles or with UVs or how much we should be concerned about that otherwise. |
Now generating the "optimized" mesh like the left side in the diagram above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small nits, but otherwise this looks great. I'd missed this one!
Personally I'd prefer if the choice of which triangulation method to wasn't hard coded, but could be selected at runtime |
@bevyengine/rendering-team; do y'all have opinions on the triangulation strategy? |
I personally prefer using something like SDF's for circles as they can scale much better and you don't have to be concerned with sides, but otherwise this seems fine to me. |
I think if someone wants a different triangulation method they can just build their own mesh? |
Agreed, but this is a solid stop-gap. |
I only had a vague notion of "long triangle thin triangle bad" to go on, and I couldn't really dig up any concrete information and the (limited) testing I did wasn't revealing. So I just went with fewest triangles / fastest mesh generation. I'm assuming you had a pretty good reason to implement that recursive strategy in your own app? Seems worthwhile to me if it actually solves some sort of problem. |
The recursive algorithm is much easier to extend to arbitrary polygons. That's probably a minor bit of useful future-proofing here, but not essential. I'm unsure if there are other benefits though. |
I did this while I was working on UV mapping. I can't remember if I saw artifacts, but I was using the fan method (on the left of the diagram above) and circles were ugly when looking at them in Blender, so I fixed it. I like the recursive approach because I love recursion, but also because if you draw the triangles from largest to smallest with this method, you get an increasingly accurate approximation of the polygon. So the lowest quality triangles contribute the least to the shape. It's probably best to stick with the simple fan until it's at least an annoyance, though. |
0373e67
to
610185c
Compare
610185c
to
ffe364b
Compare
ffe364b
to
8dc3c72
Compare
18e68b6
to
689c074
Compare
sides: 6, | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is a hexagon the default? Does it make sense to provide a default?
uvs.push([0.5 * (c + 1.0), 1.0 - 0.5 * (s + 1.0)]); | ||
} | ||
|
||
let mut indices = Vec::with_capacity((sides - 1) * 3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will crash for 0 sides.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think is the most appropriate way to handle that situation?
if sides < 3 {
panic!(/* ... */)
}
Or clamping the value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now protected by a debug_assert
. Still a panic, but it doesn't really make any sense to try to build a polygon with fewer than 3 sides, and now there's a friendly error message.
"Bevy users often want to create circles and other simple shapes." who would have thought?))) I mean the engine is like 1.5 years old and still doesn't have a beginner-approachable drawing API. I love Bevy but can't use it because of that... |
@adsick You can use |
I know that, looked at the examples, but it seems to me that immediate mode API is more intuitive. |
bors r+ |
# Objective Bevy users often want to create circles and other simple shapes. All the machinery is in place to accomplish this, and there are external crates that help. But when writing code for e.g. a new bevy example, it's not really possible to draw a circle without bringing in a new asset, writing a bunch of scary looking mesh code, or adding a dependency. In particular, this PR was inspired by this interaction in another PR: #3721 (comment) ## Solution This PR adds `shape::RegularPolygon` and `shape::Circle` (which is just a `RegularPolygon` that defaults to a large number of sides) ## Discussion There's a lot of ongoing discussion about shapes in <bevyengine/rfcs#12> and at least one other lingering shape PR (although it seems incomplete). That RFC currently includes `RegularPolygon` and `Circle` shapes, so I don't think that having working mesh generation code in the engine for those shapes would add much burden to an author of an implementation. But if we'd prefer not to add additional shapes until after that's sorted out, I'm happy to close this for now. ## Alternatives for users For any users stumbling on this issue, here are some plugins that will help if you need more shapes. https://github.com/Nilirad/bevy_prototype_lyon https://github.com/johanhelsing/bevy_smud https://github.com/Weasy666/bevy_svg https://github.com/redpandamonium/bevy_more_shapes https://github.com/ForesightMiningSoftwareCorporation/bevy_polyline
# Objective Bevy users often want to create circles and other simple shapes. All the machinery is in place to accomplish this, and there are external crates that help. But when writing code for e.g. a new bevy example, it's not really possible to draw a circle without bringing in a new asset, writing a bunch of scary looking mesh code, or adding a dependency. In particular, this PR was inspired by this interaction in another PR: bevyengine#3721 (comment) ## Solution This PR adds `shape::RegularPolygon` and `shape::Circle` (which is just a `RegularPolygon` that defaults to a large number of sides) ## Discussion There's a lot of ongoing discussion about shapes in <bevyengine/rfcs#12> and at least one other lingering shape PR (although it seems incomplete). That RFC currently includes `RegularPolygon` and `Circle` shapes, so I don't think that having working mesh generation code in the engine for those shapes would add much burden to an author of an implementation. But if we'd prefer not to add additional shapes until after that's sorted out, I'm happy to close this for now. ## Alternatives for users For any users stumbling on this issue, here are some plugins that will help if you need more shapes. https://github.com/Nilirad/bevy_prototype_lyon https://github.com/johanhelsing/bevy_smud https://github.com/Weasy666/bevy_svg https://github.com/redpandamonium/bevy_more_shapes https://github.com/ForesightMiningSoftwareCorporation/bevy_polyline
# Objective Bevy users often want to create circles and other simple shapes. All the machinery is in place to accomplish this, and there are external crates that help. But when writing code for e.g. a new bevy example, it's not really possible to draw a circle without bringing in a new asset, writing a bunch of scary looking mesh code, or adding a dependency. In particular, this PR was inspired by this interaction in another PR: bevyengine#3721 (comment) ## Solution This PR adds `shape::RegularPolygon` and `shape::Circle` (which is just a `RegularPolygon` that defaults to a large number of sides) ## Discussion There's a lot of ongoing discussion about shapes in <bevyengine/rfcs#12> and at least one other lingering shape PR (although it seems incomplete). That RFC currently includes `RegularPolygon` and `Circle` shapes, so I don't think that having working mesh generation code in the engine for those shapes would add much burden to an author of an implementation. But if we'd prefer not to add additional shapes until after that's sorted out, I'm happy to close this for now. ## Alternatives for users For any users stumbling on this issue, here are some plugins that will help if you need more shapes. https://github.com/Nilirad/bevy_prototype_lyon https://github.com/johanhelsing/bevy_smud https://github.com/Weasy666/bevy_svg https://github.com/redpandamonium/bevy_more_shapes https://github.com/ForesightMiningSoftwareCorporation/bevy_polyline
# Objective Bevy users often want to create circles and other simple shapes. All the machinery is in place to accomplish this, and there are external crates that help. But when writing code for e.g. a new bevy example, it's not really possible to draw a circle without bringing in a new asset, writing a bunch of scary looking mesh code, or adding a dependency. In particular, this PR was inspired by this interaction in another PR: bevyengine#3721 (comment) ## Solution This PR adds `shape::RegularPolygon` and `shape::Circle` (which is just a `RegularPolygon` that defaults to a large number of sides) ## Discussion There's a lot of ongoing discussion about shapes in <bevyengine/rfcs#12> and at least one other lingering shape PR (although it seems incomplete). That RFC currently includes `RegularPolygon` and `Circle` shapes, so I don't think that having working mesh generation code in the engine for those shapes would add much burden to an author of an implementation. But if we'd prefer not to add additional shapes until after that's sorted out, I'm happy to close this for now. ## Alternatives for users For any users stumbling on this issue, here are some plugins that will help if you need more shapes. https://github.com/Nilirad/bevy_prototype_lyon https://github.com/johanhelsing/bevy_smud https://github.com/Weasy666/bevy_svg https://github.com/redpandamonium/bevy_more_shapes https://github.com/ForesightMiningSoftwareCorporation/bevy_polyline
Objective
Bevy users often want to create circles and other simple shapes.
All the machinery is in place to accomplish this, and there are external crates that help. But when writing code for e.g. a new bevy example, it's not really possible to draw a circle without bringing in a new asset, writing a bunch of scary looking mesh code, or adding a dependency.
In particular, this PR was inspired by this interaction in another PR: #3721 (comment)
Solution
This PR adds
shape::RegularPolygon
andshape::Circle
(which is just aRegularPolygon
that defaults to a large number of sides)Discussion
There's a lot of ongoing discussion about shapes in bevyengine/rfcs#12 and at least one other lingering shape PR (although it seems incomplete).
That RFC currently includes
RegularPolygon
andCircle
shapes, so I don't think that having working mesh generation code in the engine for those shapes would add much burden to an author of an implementation.But if we'd prefer not to add additional shapes until after that's sorted out, I'm happy to close this for now.
Alternatives for users
For any users stumbling on this issue, here are some plugins that will help if you need more shapes.
https://github.com/Nilirad/bevy_prototype_lyon
https://github.com/johanhelsing/bevy_smud
https://github.com/Weasy666/bevy_svg
https://github.com/redpandamonium/bevy_more_shapes
https://github.com/ForesightMiningSoftwareCorporation/bevy_polyline