-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Implement segmenting of Line2D for better fidelity when using width curves #10452
Comments
My concern with this is that it'll significantly increase the number of curve points without the user realizing it, which has a performance impact. This is particularly the case on integrated graphics and mobile. If this is to be implemented, it should be something you manually opt into, so that you only pay the performance cost when you need it. |
Also note there's the same issue with Line2D's gradient, see: godotengine/godot#50076. It might make sense to tackle both issues at once. |
I'm aware of and agree with you about the potential performance implications of this change. The main reasons for choosing this approach are correctness and ease-of-use which I posit supersedes performance implications that can come from the excessive use of Line2Ds with curve widths. I would argue that using the curve width in its current incarnation is broken - if the points on your line don't match up with the points on your curve, it's not apparent why your line doesn't render according to the visual representation of your curve. Adding more properties further complicates the number of dials you have to tune to get your line rendering in a way that a matches user's natural intuition (and the gradient that @kleonc brought up also illustrates this complication). But to play devil's advocate, let's breakdown the potential performance implications of this: Is a default 200 triangles for a single curve-width-line really that much of a performance concern even on integrated graphics and mobile? Even in the very bespoke use-case of using a 100+ of these curve-width-lines yields triangles in the range of ~20000, which is pretty standard for a single 3d mesh these days, even on integrated cards and mobile devices. To me, a complete, easy-to-use, working feature is more important than the theoretical performance loss in very specific use-cases of games that need to render 100s or even 1000s of a Line2D. I would assume that developers leveraging Line2D to that extent would already need to be aware that a performance would need to be addressed on their end. To address your legitimate concerns, I propose that the solution for this is user education via additional tooltip information on the curve_width property (e.g. increasing the bake_resolution affects the number of segments which can blow up your tri-count/performance). EDIT: |
I can work on an implementation for this as well. |
Yes, but the cost is mostly in line generation, not rendering. The 200 triangles need to be regenerated every frame if the Line2D is animated or is otherwise updated in real-time. This is where most of the performance cost comes from - it's all on the CPU, so it can be a source of unavoidable stuttering too. Updating Line2Ds in real-time is a rather common scenario, and it can happpen with multiple Line2Ds needing regeneration every frame. Most users don't tweak a curve's bake resolution as they don't need to, so I doubt most users will notice the increased triangle count as a result of using a width curve when upgrading their project. It could also be argued that implementing this without a toggle will change visuals in some projects in an unexpected manner too, so there should be a way to avoid that. In other words, some projects may rely on the current behavior for a specific visual effect, even if it's technically incorrect. |
Thanks for the feedback. 100 segments, 1000 animating lines: 16 segments, 1000 animating lines: I've added a new I've also gone ahead and implemented a fix for issue godotengine/godot#50076. line2d-working-gradients.mp4In order to fix the gradients, points have to be inserted separate from the usage of the curve segments feature, so if a gradient is defined, the number of base line segments goes up. The minimum number of segments will always be at least the number of points on the (optionally) gradient-point-inclusive line. Here's a video to show the fidelity increase as the segments property goes up: line2d-working-curve-and-gradients-with-segments.mp4I've verified that the line works with all the different capping methods as well. |
I second the goal of the proposal to offer options to make the line draw as intended by the user, and having a property to control the amount of subdivisions sounds great. We have similar controls on 3D mesh tools like CSGPolygon3D when drawing along a path. CSG meshes can be much costlier performance-wise due to the third dimension plus boolean operations, but the point is offering tools to prototype and iterate fast. I see the same value in Line2D. Being an all-purpose drawing tool for lines and curves, it's never going to be super efficient. In practice, currently, for prototyping we tend to generate points in GDScript, which results in lower performance than a native option would, and then usually ditch Line2D because of its limitations and use our own shaders and/or drawing code as needed to e.g. pan a texture along a a mesh designed and unwrapped in Blender, curving it at runtime instead of generating points. |
I feel like having subdivisions sounds great, but the offset option I'm a little more hesitant with. This is something that can be done in a shader, even if with more difficulty. I'm not opposed to it, but I'm wondering at what point we stop to say "ok no, this feature should be a shader feature not a line feature". |
It's probably up to the maintainers in charge of this area to join the discussion and decide on the purpose and scope of the feature and what may go in or not. |
Describe the project you are working on
Top-down game
Describe the problem or limitation you are having in your project
Currently, Line2D doesn't render the correct curved geometry that would intuitively expect from how define you width curve - it doesn't accommodate well for when the number of points on your line are less than the number of points on your curve.
Here's an example of the problem described:
And this is what I expect it to look like:
Describe the feature / enhancement and how it helps to overcome the problem or limitation
I have a pull request here that fixes it by generating additional points on a line based on the number of segments derived from the curve bake resolution: godotengine/godot#95541
Along with the problem described above, my pull request also adds an extra
curve_offset
property which allows you to do some neat Line2D animations with the curve width (see example videos below).Segment produce better 'wavey' lines when more curve control points exist than line points:
https://github.com/user-attachments/assets/35a1fe1a-bd1f-4313-bb29-d7d7adb75b57
Curve offset adjusts the sampling position of the curve:
https://github.com/user-attachments/assets/d66792df-39c9-4945-93ab-47e96d49cfcd
You can also animate the curve offset to produce some neat line effects:
https://github.com/user-attachments/assets/e1e2d108-ad7d-4fdd-83b9-3629abf76da2
Other Notes:
Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
See video above.
If this enhancement will not be used often, can it be worked around with a few lines of script?
No. This has to be implemented as part of Line2D before the line is built and sent to the rendering server.
Is there a reason why this should be core and not an add-on in the asset library?
It's a pretty simple change that gives you the visual result that you generally expect when using the curve width feature.
The curve offset also allows for some neat freedom of expression that should really just come standard with Line2D.
The text was updated successfully, but these errors were encountered: