-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Gizmos for primitive shapes #10571
Comments
This could also partially fix some items for #9400 because of the new supported shapes. |
What do you think about // I'm using empty types for everything just for demonstrating purposes
struct Gizmo;
struct Circle;
struct CircleDetails;
struct Rect;
struct RectDetails;
pub trait GizmoPrimitive2D<P, D> {
fn primitive_2d(&mut self, primitive: P, detail: D);
}
impl GizmoPrimitive2D<Rect, RectDetails> for Gizmo {
fn primitive_2d(&mut self, primitive: Rect, detail: RectDetails) {
todo!()
}
}
impl GizmoPrimitive2D<Circle, CircleDetails> for Gizmo {
fn primitive_2d(&mut self, primitive: Circle, detail: CircleDetails) {
todo!()
}
}
fn main() {
let g = Gizmo;
g.primitive_2d(Rect, RectDetails);
g.primitive_2d(Circle, CircleDetails);
// doesn't compile
//g.primitive_2d(Rect, CircleDetails);
} We could optionally create a trait
|
Re: #11072 So, the gizmos.primitive_3d(
Cuboid::new(2.0, 1.0, 4.0),
Cuboid3dDetails {
center: Vec3::Y,
rotation: Quat::from_rotation_z(1.2),
color: Color::CYAN,
..default()
},
); To me, this feels a bit verbose for what it does, and it's also very different from the approach used by existing gizmo methods. If we look at all of the I'm wondering if we could make the API more builder-like, similar to the approach used by the existing gizmos and my (somewhat WIP) meshing implementation #11007. This would have the following benefits:
Essentially, /// A trait for rendering 2D geometric primitives (`P`) with [`Gizmos`].
pub trait GizmoPrimitive2d<P: Primitive2d> {
/// The output of `primitive_2d`. For most primitives, this is just `()`,
/// but some primitives return a builder for specifying details.
type Output;
/// Renders a 2D primitive.
fn primitive_2d(&mut self, primitive: P, position: Vec2, rotation: f32, color: Color) -> Self::Output;
}
/// A trait for rendering 3D geometric primitives (`P`) with [`Gizmos`].
pub trait GizmoPrimitive3d<P: Primitive3d> {
/// The output of `primitive_3d`. For most primitives, this is just `()`,
/// but some primitives return a builder for specifying details.
type Output;
/// Renders a 3D primitive.
fn primitive_3d(&mut self, primitive: P, position: Vec3, rotation: Quat, color: Color) -> Self::Output;
} For a 2D circle, it would just return the existing Primitives like impl GizmoPrimitive3d<Cuboid> {
type Output = ();
fn primitive_3d(&mut self, primitive: Cuboid, position: Vec3, rotation: Quat, color: Color) -> () {
// Render cuboid, returning nothing
}
} Let's compare this API with the // Before
gizmos.primitive_3d(
Cuboid::new(2.0, 1.0, 4.0),
Cuboid3dDetails {
center: Vec3::Y,
rotation: Quat::from_rotation_z(1.2),
color: Color::CYAN,
..default()
},
);
// After
gizmos.primitive_3d(
Cuboid::new(2.0, 1.0, 4.0),
Vec3::Y,
Quat::from_rotation_z(1.2),
Color::CYAN,
); let position = Vec2::new(10.0, 20.0);
// Before
gizmos.primitive_2d(
Circle { radius: 0.5 },
Circle2dDetails {
center: position,
color: Color::PINK,
segments: 16,
},
);
// After
gizmos
.primitive_2d(Circle { radius: 0.5 }, position, 0.0, Color::PINK)
.segments(16);
// Compare with existing gizmos.circle_2d
gizmos.circle_2d(position, 0.5, Color::PINK).segments(16); To me, the builder API definitely seems more ergonomic for most cases, and it's much closer to the API used by existing gizmo methods like Would this API make sense, or am I perhaps missing something important? |
Sorry for the short answer in before. Yeah that makes total sense. So this is closer to the API we already have for circles & arcs. I started to like that while implementing the first draft of the PR actually. It's pretty clean from the users perspective although it's a bit verbose to implement. I'm going to refactor to this API 👍 |
I went for something like this here /// A trait for rendering 3D geometric primitives (`P`) with [`Gizmos`].
pub trait GizmoPrimitive3d<'s, P: Primitive3d> {
/// The output of `primitive_3d`. This is a builder to set non-default values.
type Output<'a>
where
Self: 's,
's: 'a;
/// Renders a 3D primitive with its associated details.
fn primitive_3d<'a>(&'s mut self, primitive: P) -> Self::Output<'a>;
} with this example impl /// Builder for configuring the drawing options of [`Sphere`].
pub struct SphereBuilder<'a, 's> {
gizmos: &'a mut Gizmos<'s>,
radius: f32, // Radius of the sphere
center: Vec3, // Center position of the sphere in 3D space
rotation: Quat, // Rotation of the sphere around the origin in 3D space
color: Color, // Color of the sphere
segments: usize, // Number of segments used to approximate the sphere geometry
}
impl SphereBuilder<'_, '_> {
/// Set the center position of the sphere in 3D space.
pub fn center(mut self, center: Vec3) -> Self {
self.center = center;
self
}
/// Set the rotation of the sphere around the origin in 3D space.
pub fn rotation(mut self, rotation: Quat) -> Self {
self.rotation = rotation;
self
}
/// Set the color of the sphere.
pub fn color(mut self, color: Color) -> Self {
self.color = color;
self
}
/// Set the number of segments used to approximate the sphere geometry.
pub fn segments(mut self, segments: usize) -> Self {
self.segments = segments;
self
}
}
impl<'s> GizmoPrimitive3d<'s, Sphere> for Gizmos<'s> {
type Output<'a> = SphereBuilder<'a, 's> where Self: 's, 's: 'a;
fn primitive_3d<'a>(&'s mut self, primitive: Sphere) -> Self::Output<'a> {
SphereBuilder {
gizmos: self,
radius: primitive.radius,
center: Default::default(),
rotation: Default::default(),
color: Default::default(),
segments: Default::default(),
}
}
}
impl<'s> Drop for SphereBuilder<'_, 's> {
fn drop(&mut self) {
let SphereBuilder {
radius,
center,
rotation,
color,
segments,
..
} = self;
// draw two caps, one for the "upper half" and one for the "lower" half of the sphere
[-1.0, 1.0].into_iter().for_each(|sign| {
draw_cap(
self.gizmos,
*radius,
*segments,
*rotation,
*center,
sign,
*color,
);
});
draw_circle(self.gizmos, *radius, *segments, *rotation, *center, *color);
}
} and this user facing api gizmos
.primitive_3d(Sphere { radius: 1.0 })
.center(center)
.rotation(rotation)
.segments(segments); But lifetimes seem to make this approach impossible due to borrowing issue with the associated type |
Any ideas on how we can proceed here @Jondolf ? |
The PR is in a reviewable state now in the sense that the basic implementations are there. There are still some ToDos that I'm aware of: - [x] docs for all the new structs and traits - [x] implement `Default` and derive other useful traits for the new structs - [x] Take a look at the notes again (Do this after a first round of reviews) - [x] Take care of the repetition in the circle drawing functions --- # Objective - TLDR: This PR enables us to quickly draw all the newly added primitives from `bevy_math` in immediate mode with gizmos - Addresses #10571 ## Solution - This implements the first design idea I had that covered everything that was mentioned in the Issue #10571 (comment) --- ## Caveats - I added the `Primitive(2/3)d` impls for `Direction(2/3)d` to make them work with the current solution. We could impose less strict requirements for the gizmoable objects and remove the impls afterwards if the community doesn't like the current approach. --- ## Changelog - implement capabilities to draw ellipses on the gizmo in general (this was required to have some code which is able to draw the ellipse primitive) - refactored circle drawing code to use the more general ellipse drawing code to keep code duplication low - implement `Primitive2d` for `Direction2d` and impl `Primitive3d` for `Direction3d` - implement trait to draw primitives with specialized details with gizmos - `GizmoPrimitive2d` for all the 2D primitives - `GizmoPrimitive3d` for all the 3D primitives - (question while writing this: Does it actually matter if we split this in 2D and 3D? I guess it could be useful in the future if we do something based on the main rendering mode even though atm it's kinda useless) --- --------- Co-authored-by: nothendev <[email protected]>
Closing, as #11072 has been merged :) |
The PR is in a reviewable state now in the sense that the basic implementations are there. There are still some ToDos that I'm aware of: - [x] docs for all the new structs and traits - [x] implement `Default` and derive other useful traits for the new structs - [x] Take a look at the notes again (Do this after a first round of reviews) - [x] Take care of the repetition in the circle drawing functions --- # Objective - TLDR: This PR enables us to quickly draw all the newly added primitives from `bevy_math` in immediate mode with gizmos - Addresses bevyengine#10571 ## Solution - This implements the first design idea I had that covered everything that was mentioned in the Issue bevyengine#10571 (comment) --- ## Caveats - I added the `Primitive(2/3)d` impls for `Direction(2/3)d` to make them work with the current solution. We could impose less strict requirements for the gizmoable objects and remove the impls afterwards if the community doesn't like the current approach. --- ## Changelog - implement capabilities to draw ellipses on the gizmo in general (this was required to have some code which is able to draw the ellipse primitive) - refactored circle drawing code to use the more general ellipse drawing code to keep code duplication low - implement `Primitive2d` for `Direction2d` and impl `Primitive3d` for `Direction3d` - implement trait to draw primitives with specialized details with gizmos - `GizmoPrimitive2d` for all the 2D primitives - `GizmoPrimitive3d` for all the 3D primitives - (question while writing this: Does it actually matter if we split this in 2D and 3D? I guess it could be useful in the future if we do something based on the main rendering mode even though atm it's kinda useless) --- --------- Co-authored-by: nothendev <[email protected]>
What problem does this solve or what need does it fill?
Primitive shapes were added in #10466. It should be possible to render them using Bevy's gizmos. This would help unify APIs while also adding support for more gizmo shapes.
What solution would you like?
There's a few potential APIs I have in mind. One would be to have
primitive_2d
/primitive_3d
methods forGizmos
:Here, each shape primitive should implement a trait like
GizmoPrimitive2d
/GizmoPrimitive3d
so that the gizmos know how to draw them.One issue with this approach is that you can't really control the detail. I'm not sure how to fix it nicely, but one approach could be to have the trait be given an associated type so that the methods can return different builder types for the different shapes, like the current
CircleBuilder
. It feels a bit verbose, but maybe it's necessary with this approach?Another alternative is to have each shape primitive have its own gizmo method. This approach could support detail where applicable and support more configuration. Something like this:
If desired,
Gizmos
could still haveprimitive_2d
/primitive_3d
with default implementations.Ideas and suggestions are welcome!
The text was updated successfully, but these errors were encountered: