-
-
Notifications
You must be signed in to change notification settings - Fork 21.8k
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
Add VisualShape2D node for drawing common shapes #99210
base: master
Are you sure you want to change the base?
Conversation
59e98c2
to
a61b4f1
Compare
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.
Oh wow, I was not expecting this to be requested this much.
When adding new nodes like this, my only personal concern is compiled binary size.
a61b4f1
to
3218dce
Compare
Despite the current milestone and popular demand, this feature would require a good chunk of more recent discussion. Merging for Bear in mind this does not mean the PR is abandoned, or undesired. If maintainers approve of it as is, it'll be merged and the milestone changed. |
For reference: I tested this and I like the implementation a lot. It's complete with its own editor and the shapes even work properly with shaders. The rectangle shape is a Node2D equivalent of ColorRect, so I imagine it can be used outside prototyping (previous workaround was a scaled Sprite2D with 1px texture). There was lots of discussion in the proposal and the alternative of simply using CollisionPolygon2D or custom-drawn Shape2D is much inferior to this PR's implementation. Also this can't be easily replicated with a plugin (it's a lot of code); it could work nice as a GDExtension though. I said I like the implementation, but that's not really enough to merge a feature. As a heavy 2D user, I could maybe see myself using it in some cases, but the existing workarounds are good enough. However I'm not against merging this either.
It's pretty much given that a texture will be among the first things requested and someone will eventually add it. Every time engine gets a new thing, it's followed by various follow-up feature requests (something I observed recently). Playing with resolution gives interesting results 🤔 I think the capsule should change orientation when width is bigger than height. |
Yeah, I figured I'd give it another try since the proposal seems wanted and has the implementer wanted tag. For the Texture support, maybe we can add a note in the documentation to head that off? With texture support comes wanting UV offsets, flipping, atlas and animation support, and at that point it would be better to just inherit from Sprite2D. Added horizontal capsules. I used an approach that makes sure the shape will be the same as the vertical one, if rotated. Also fixed issue when changing the rect in 2D editor when offset was not at the center. Offset now moves to be in the same relative position. |
3218dce
to
49fa55c
Compare
Indeed, seems like it was basically accepted: godotengine/godot-proposals#1126 (comment) |
collision_shape_2d_instance->set_shape(shape); | ||
collision_shape_2d_instance->translate(visual_shape_2d->get_offset()); | ||
} break; | ||
case VisualShape2D::SHAPE_CIRCLE: { |
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 might need a special case for ovals. It's the only shape that doesn't get accurate collider.
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 could calculate the eccentricity and make a ConvexPolygonShape2D in some cases, but I didn't want to arbitrarily choose some threshold. I also want to have it use CircleShape2D where possible, since it is better for physics.
I can play around with thresholds and see.
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.
Eccentricity wasn't very helpful, for now I went with a 10% difference in size.
If it still feels off, I could instead do what MeshInstance3DEditor does and have a menu and let the user choose whether to make it a shape or a convex polygon. This would also allow for small resolution shapes (like hexagon) to have a polygon option.
49fa55c
to
e0df676
Compare
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.
The implementation looks good.
e0df676
to
c15cdb1
Compare
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.
LGTM
I feel 2D CSG is best kept separate, as even if you use a single CSG2D node with no children, it'll still have more overhead than a VisualShape2D node. The same applies in 3D already. |
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.
Tested locally, it works as expected.
Some feedback:
- The Circle mode doesn't extend the polygon all the way down with odd side counts (this happens regardless of the node's aspect ratio):
visualshape2d_circle.mp4
This also happens with the Capsule mode with certain side counts such as 5
, albeit in a less extreme manner.
- The Capsule mode has identical looks with some side counts (such as
5
and6
). I'd expect the number of sides specified to be present on each cap of the shape, instead of being the sum of both top and bottom caps. For reference, this is how it's specified in CapsuleMesh in 3D.
visualshape2d_capsule.mp4
-
The Outline Width property disables filled shape drawing whenever its value is greater than
0.0
. This means you can't draw a filled shape with an outline with a single node, which is inconsistent with how many other nodes work (such as Label or Label3D).- Instead, I suggest adding separate Filled (bool), Color and Outline Color properties, and have the Outline Width property control outline thickness drawing without disabling filled shape drawing.
-
Outline Width could allow a value of
-1.0
to draw pixel-perfect lines that don't scale with viewport zoom, although antialiasing wouldn't be supported this way. This behavior would essentially be opposite ofCanvasItem.draw_line()
where-1.0
draws a 1-pixel thick line that scales with viewport zoom. (For a higher-level API, I feel1.0
should scale with viewport zoom instead, as it's more likely to match users' expectations.) -
The antialiasing feather doesn't scale with viewport zoom, and there's no way to override its size (which can be used to create blurred shapes). The CanvasItem drawing functions don't expose a way to do this, so we'll have to leave it as-is for now. This could be worth exploring in its own proposal.
-
There could be an extra mode to draw a rectangle with rounded corners (similar to StyleBoxFlat but with a Node2D base), but it can be done later.
Thanks for the feedback!
These are regular polygons, so the points are an equal distance from the center, so I can't really stretch them to reach the bottom. They won't always fit into a square shape due to the math.
The problem with this is that the capsule's resolution and the circle's resolution are the same, so I wanted it to act similarly. If it controlled the capsule's cap size, then the default value of 64 would have twice as many vertices as the circle does.
I wanted to reduce the amount of properties, but I can do this.
I can add this.
This would require more parameters for each corner radius to match the StyleBoxFlat, and since this isn't resource-based, it would have to be on the base and then hidden on all other shapes (like the resolution). |
I'd put the outline in front of the shape. |
Draw common shapes like Rectangles, Circles, Equilateral Triangles, Right Triangles, Capsules, and Regular Polygons.
Vector-based like Polygon2D, does not rely on textures.
Outline with variable thickness.
Antialiasing.
Solid color (the above image uses 2D lighting on some shapes).
UVs (except on outlines and antialiasing) for shader support.
Regular polygons are under the Circle shape by changing the resolution.
In the editor:
Convert to MeshInstance2D or Polygon2D.
Add CollisionShape2D or ShadowMask2D sibling.
These are similar to the functionality of Sprite.
Implementation notes:
Capsules are vertical only, with some more math it could probably dynamically be vertical or horizontal based on the size.draw_polyline
function has it anddraw_colored_polygon
doesn't. This doesn't have UVs, and half of the line is drawn behind the shape. Alias is also affect by scaling.draw_circle
uses.