WIP: Fix #105578: Fix the tilt of the curve if realize instance makes it impossible to calculate the normal correctly #105634

Iliya Katushenock · 2023-03-10T13:06:07+01:00

Iliya Katushenock commented

2023-03-10 13:06:07 +01:00

If, when realize instances, the curve gets into a state where its normal cannot be calculated (the tangent is vertical), then can take into account the state of this normal in order to rotate the tilt of the curve.

Iliya Katushenock added 1 commit 2023-03-10 13:06:17 +01:00

3f58e340e2 init commit

Iliya Katushenock changed title from ~~WIP: Fix 105578: Fix the tilt of the curve if realize instance makes it impossible to calculate the normal correctly~~ to WIP: Fix #105578: Fix the tilt of the curve if realize instance makes it impossible to calculate the normal correctly

2023-03-10 13:06:31 +01:00

Iliya Katushenock added this to the Nodes & Physics project 2023-03-10 13:06:38 +01:00

Iliya Katushenock commented

2023-03-10 17:26:31 +01:00

Fine.
It seems to be easier to take and save the normals of the curve. Keep them separate when realizing. Apply a transformation matrix to them.
After that, take the new normals from the realized curve, based on the old normals and tangent, calculate the angle of rotation for each point on the curve, and add that to the tilt attribute.
I see this overhead for all other cases instead of reported one...

Fine. It seems to be easier to take and save the normals of the curve. Keep them separate when realizing. Apply a transformation matrix to them. After that, take the new normals from the realized curve, based on the old normals and tangent, calculate the angle of rotation for each point on the curve, and add that to the tilt attribute. I see this overhead for all other cases instead of reported one...

Martijn Versteegh commented

2023-03-14 17:02:34 +01:00

Seems like a workable approach to me.

Iliya Katushenock commented

2023-03-14 17:06:24 +01:00

So far the problem is:

The particular case, when each spline is processed individually, due to a simplified formula for calculating the rotation offset, is complicated.
The general variant, with the preservation of the old normals, is very costly in memory and time.

So far the problem is: - The particular case, when each spline is processed individually, due to a simplified formula for calculating the rotation offset, is complicated. - The general variant, with the preservation of the old normals, is very costly in memory and time.

Martijn Versteegh commented

2023-03-17 12:55:06 +01:00

I took a (somewhat) longer look at the code, and you're completely right: it is kinda complicated.

Maybe it's better after all to just document this as a 'known limitation' and better document that the curve normals are always parametric and calculated based on the evaluated curve? After all a workaround is quite straightforward using a 'capture attribute', which does more or less the same as the proposed fix but only when you need it.

The main problem is that it's rather confusing , so maybe better docs is the way to go?

I took a (somewhat) longer look at the code, and you're completely right: it is kinda complicated. Maybe it's better after all to just document this as a 'known limitation' and better document that the curve normals are always parametric and calculated based on the evaluated curve? After all a workaround is quite straightforward using a 'capture attribute', which does more or less the same as the proposed fix but only when you need it. The main problem is that it's rather confusing , so maybe better docs is the way to go?

Iliya Katushenock commented

2023-03-17 13:04:12 +01:00

If during implementation we could get the transformation matrix attribute, then yes, it would be very simple. For now, a little more movement is needed. But it's still possible.

Since this case is very unlikely, unless you try to achieve it on purpose, I think it's easier to expect the user to be able to bypass it himself if necessary.

If during implementation we could get the transformation matrix attribute, then yes, it would be very simple. For now, a little more movement is needed. But it's still possible. Since this case is very unlikely, unless you try to achieve it on purpose, I think it's easier to expect the user to be able to bypass it himself if necessary.

Iliya Katushenock closed this pull request

2023-03-17 13:04:13 +01:00

Iliya Katushenock deleted branch fix_tilt_of_realized_curve

2023-04-08 12:18:43 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

WIP: Fix #105578: Fix the tilt of the curve if realize instance makes it impossible to calculate the normal correctly #105634

Pull request closed