Maniphest T101417

Inverted Curve Trim Output
Needs Information from Developers, NormalDESIGN

Assigned To
Mattias Fredriksson (Osares)
Authored By
Mattias Fredriksson (Osares)
Sep 28 2022, 12:07 AM
Tags
  • Geometry Nodes
Subscribers
Hans Goudey (HooglyBoogly)
Iliya Katueshenock (Moder)
Jacques Lucke (JacquesLucke)
Mattias Fredriksson (Osares)
Paul (genesis2303)
Paul Larson (GeorgiaPacific)

Description

Relevance

Trimming cyclical curves as implemented in D14481 does not give the user control over which part of the trimmed curve to return in the output. This can cause undesired behaviors as in T101379 where the default behavior for cyclical curves is to return a single point on the curve when the trim start and endpoint is equivalent. A solution could be to either allow inputs outside of the interval defined by the curve length or change the behavior for cyclical curves to return the split curve instead of a point and let the user select the 'inner' or 'outer' part of the trimmed curve if opposite behavior is desired.


Alternatives

Behavior for inner/outer output can already be achieved through the use of multiple trim nodes.

D14481 had a solution which used a 'mode' for cyclical curves to resolve the ambiguity startpoint and endpoint were identical. Options included: Split, Copy, Point. While this resolves the ambiguity it introduces a redundant option for non-cyclical curves.


Related Tasks

D14481 T101379


Limitations of the proposed solution(s)

For non-cyclical curves taking the 'outer' curve will no longer map the spline count in the input geometry with the spline count in the output geometry. This may have some performance implications for threading certain tasks.

Mockups


Modulo inputs

Take the modulo of start/end inputs with curve length. Optionally one could add an option checkbox for allow the curve to be looped/repeated if the difference is larger then the length.


Inverted output

A flexible solution would be to provide two geometry outputs for either the 'inner' or the 'outer' part of the trimmed curve.


Alternative

Since providing two geometry outputs is rare, a preference may be to add it as an option instead.


Inverted output example


Trim node


Inner part of the trim curve


Outer part of the trim curve. For non-cyclical curves two disconnected curves would be returned which would include the start and endpoint of the initial curve.

Event Timeline

Mattias Fredriksson (Osares) changed the task status from Needs Triage to Needs Information from Developers.Sep 28 2022, 12:07 AM
Mattias Fredriksson (Osares) created this task.
Mattias Fredriksson (Osares) renamed this task from Inverted Trim Output to Inverted Curve Trim Output.Sep 28 2022, 12:09 AM
Iliya Katueshenock (Moder) added a subscriber: Iliya Katueshenock (Moder).Sep 28 2022, 12:12 AM

About switching curve segment:
I think entering an inversion mask would be more convenient.
But this can be achieved by swapping 2 points, so is it necessary?
About the output of both:
Similar to the geometry split node, most likely this will also be due to the duplication of the function call. Is it necessary?
Although because of the anology, it seems necessary.

Paul Larson (GeorgiaPacific) added a subscriber: Paul Larson (GeorgiaPacific).Sep 28 2022, 8:00 AM
Mattias Fredriksson (Osares) added a comment.EditedSep 28 2022, 9:23 PM

Thought about passing an inversion mask but was unsure what the actual use case would be and providing both selection + inversion mask would be a bit much. It would also be something that could be achieved easily with a few nodes if the behavior would actually be desired (particularly if order is irrelevant).

Not sure why duplication of the function call would be an issue as sockets only need to be evaluated when used? Some computations could be shared between the outputs though.

Yes not sure if this iteration is the best one but it seems to be a reasonable one to me at least.

Iliya Katueshenock (Moder) added a comment.Sep 28 2022, 9:39 PM

I think it's best to make several node layouts (for all possible variations) and their methods of use and interchange. On Wednesday there will be a node module meeting, it will be a big topic.

Mattias Fredriksson (Osares) added a comment.EditedSep 28 2022, 10:39 PM

Yeah the only really good contender to this solution would probably be to allow the user input negative values or values greater then the length and 'loop' the output curve (initially mentioned such an approach in the bug report). It could be a powerful tool for creating say a parametric 'spring' using a circle as starting point, but there are other ways to do it to (just start from a line instead). It would solve the ambiguity for say using 0.0 and 1.0 as inputs since they now do represent different points (relative to the output curve). But such a solution has a downside when the user inputs a very large number which wont show on the display as the curve geometry is overlapped. Such a solution would also be partially incompatible with this one (the curve would no longer be split into two parts in the same sense as it is now).

That is why I like this approach I think it's the one where there are no actual downsides from the user perspective. But I will add a mockup for it tomorrow 💤.

Jacques Lucke (JacquesLucke) added a subscriber: Jacques Lucke (JacquesLucke).Sep 29 2022, 1:53 PM

I think the ambiguity only exists on the code level right now, not on the user level. There is no ambiguity to users when the start is 0.0 and end is 1.0, the full curve is expected in that case. If the user only wants the first point both should be set to 0.0 or both to 1.0 (in the case of cyclic curves). It's not the nodes task to support "rotating" the curve control point order so that e.g. the new beginning and end is at 0.3f and keeping it cyclic. When start and end have the same value, the curve should not be visible anymore. The easiest solution here is probably to just have a special case for start=0 and end=1 and to not modify the curve at all in this case.

Mattias Fredriksson (Osares) added a comment.Sep 29 2022, 7:40 PM

Yes treating 0.0 and 1.0 as an edge case would resolve the issue in the bug report. But to me it has always felt like solving the obvious issue but ignoring the underlying problem as there are no node input which splits the curve at an arbitrary point. I would prefer a solution which would handle all cases not just this edge case but I don't mind providing a simple correction instead if that is the preferred solution.

Jacques Lucke (JacquesLucke) added a comment.Sep 29 2022, 7:46 PM

I don't think it's that task of this node to split curves. We can have a separate node for that.
I do think that start=0 and end=1 is an actual special case, because it's the only case where the numbers are different but they may reference the same point on the curve (or is there any other combination of inputs with this property?). So treating it that way seems ok to me.

Iliya Katueshenock (Moder) added a comment.Sep 29 2022, 7:48 PM

I think the question is whether we want the user to not be limited to a long curve when he wants to sample it cyclically for several revolutions. If 2.4 for a curve with length 1 is normal and = 0.4, then yes, many points, like -0.4 and 0.6

Hans Goudey (HooglyBoogly) added a comment.Sep 29 2022, 7:52 PM

Regarding splitting curves, D13953 should allow that. Though it would need to be changed to allow splitting in between control points. That could be a separate mode (optional for performance reasons ideally).

Mattias Fredriksson (Osares) updated the task description.Sep 29 2022, 10:32 PM
Mattias Fredriksson (Osares) updated the task description.Sep 29 2022, 11:37 PM
Mattias Fredriksson (Osares) added a comment.EditedSep 29 2022, 11:42 PM
In T101417#1425257, @Jacques Lucke (JacquesLucke) wrote:

I don't think it's that task of this node to split curves. We can have a separate node for that.
I do think that start=0 and end=1 is an actual special case, because it's the only case where the numbers are different but they may reference the same point on the curve (or is there any other combination of inputs with this property?). So treating it that way seems ok to me.

You are correct, currently the input is clamped to the length of the curve so there is no other case. It would be useful to allow values outside the [0, curve length] interval though for cyclical curves as the input would be more fluid around the loop point (it is a bit annoying to trim a curve up/down until suddenly one handle stops working because its clamped to 0). As long as the curve is not repeated in the output it would be an improvement.

In T101417#1425277, @Hans Goudey (HooglyBoogly) wrote:

Regarding splitting curves, D13953 should allow that. Though it would need to be changed to allow splitting in between control points. That could be a separate mode (optional for performance reasons ideally).

To me that split node seem to do something entirely different to what we are talking about here. It also works on control point selections which seems to be intended for splitting a curve into multiple curves? Not sure how one would pass an offset into a segment since the number of output curves could vary?

Not sure why the idea of the trimming and splitting (at a single point on the curve) seem to be wildly different things when both is essentially doing the same thing: sampling the curve at and between two points on the curve. So the proposed solutions doesn't really change the behavior in any real way except providing some additional options which could improve user interactions. To me using modulo + negative values would be an improvement that would also resolve the issue without changing the node outward. But if it isn't desired I will not mention it again!

Paul (genesis2303) added a subscriber: Paul (genesis2303).Sep 30 2022, 6:14 PM
Jacques Lucke (JacquesLucke) added a comment.Oct 17 2022, 12:23 PM

Generally, I wouldn't mind a separate mode/node with modulo behavior. Although in this case I'd prefer it if the node could not only trim but also extend (and potentially even duplicate) curves. For example, a range from 0 to 2 would turn a cyclic circle curve into two circles (but not cyclic anymore, kind of like a spiral). For non-cyclic curves the result should probably be two curves. I'm not sure if this functionality could be integrated into the existing behavior of the trim node (also the name "Trim" is not quite correct anymore).

Either way, I think we should just add the 0 to 1 special case to fix the bug now. It makes more sense to the user and also aligns better with potential future plans like the one mentioned above.

Mattias Fredriksson (Osares) added a comment.Oct 17 2022, 10:53 PM
In T101417#1433283, @Jacques Lucke (JacquesLucke) wrote:

Generally, I wouldn't mind a separate mode/node with modulo behavior. Although in this case I'd prefer it if the node could not only trim but also extend (and potentially even duplicate) curves. For example, a range from 0 to 2 would turn a cyclic circle curve into two circles (but not cyclic anymore, kind of like a spiral). For non-cyclic curves the result should probably be two curves. I'm not sure if this functionality could be integrated into the existing behavior of the trim node (also the name "Trim" is not quite correct anymore).

Either way, I think we should just add the 0 to 1 special case to fix the bug now. It makes more sense to the user and also aligns better with potential future plans like the one mentioned above.

Thanks for replying. Yes it seemed like the direction things were heading so I made D16161 which should resolve the primary issue. Regarding extending the design I tested playing around with allowing values outside the [0, 1] range but FP rounding errors made it tricky to achieve certain results (e.g. split) without allowing curve to be 'extended' (extending would be easy with current implementation but it would take things to far). So my conclusion is that if this would be done it would require a properly thought-out design. For returning the 'inverted' trim result it could be a decent improvement for the trim node itself but I don't think it's high enough priority to justify right now since it's neat but can it be achieved by combining nodes.

Not sure if design tasks are 'closed' or just abandoned but it seems the discussion could be resumed in some other place or time if there is relevancy for introducing a further change.

Hans Goudey (HooglyBoogly) moved this task from Backlog/Bugs to Product Backlog on the Geometry Nodes board.Oct 19 2022, 11:49 PM