Implement segmenting of Line2D for better fidelity when using width curves

chryan commented 4 weeks ago

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: line2d-not-showing-ridges

And this is what I expect it to look like: line2d-proper-curve-width

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: https://github.com/godotengine/godot/pull/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:

It's possible for the logic to move into the LineBuilder if it's deemed that that's the better place to implement this feature. I'm just not currently familiar with the math behind how that works so I opted for the simpler solution
The number of segments corresponds with the curve bake resolution instead of a being separate property - I didn't see it being very useful to keep those values independent

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.

Calinou commented 4 weeks ago

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.

kleonc commented 4 weeks ago

Also note there's the same issue with Line2D's gradient, see: https://github.com/godotengine/godot/issues/50076. It might make sense to tackle both issues at once.

chryan commented 4 weeks ago

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.

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. In fact, it wasn't apparent to me why my line wasn't rendering the way the feature presented itself until I started to randomly throwing in points on the curve, and subsequently digging into the C++ engine code to understand why.

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: The default bake resolution for curves is 100, yielding approximately 100 segments on your line. With each segment being made up of 2 triangles, you get about 200 triangles per curve-width-line (my algorithm doesn't result in exact number of segments equal to the bake resolution).

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 do also want to add that there's the also a hidden performance implication if your line segment and bake resolution count being separate. For example: If your bake resolution is 100 and your line segments is 10, you've wasted 720 bytes of memory for points (90 * sizeof(Vector2)) on your curve you don't need. If your line segments is 100, and your bake resolution is 10, you're rendering an additional 180 triangles completely hidden to users which I would argue is worse.

chryan commented 4 weeks ago

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 can work on an implementation for this as well.

Calinou commented 3 weeks ago

Is a default 200 triangles for a single curve-width-line really that much of a performance concern even on integrated graphics and mobile?

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.

chryan commented 3 weeks ago

Thanks for the feedback. I did some profiling to verify the performance claims.

100 segments, 1000 animating lines: line2d-lots-of-segments

16 segments, 1000 animating lines: line2d-less-line-segments

I've added a new _curve_line_segments property that defaults to 1, but the resulting segments used will be always be higher (details below).

I've also gone ahead and implemented a fix for issue https://github.com/godotengine/godot/issues/50076. Each gradient point is inserted between points relative to where they would live on a normalized 0-1 scale of the total line.

https://github.com/user-attachments/assets/ff759192-9c7b-4fc9-9e42-043427ea8e46

In 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:

https://github.com/user-attachments/assets/bb60220d-9879-4981-badd-0311765a8961

I've verified that the line works with all the different capping methods as well.

NathanLovato commented 1 week ago

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.

godotengine / godot-proposals