Regression: deleting a point in a (cyclic) nurbs curve permanently converts to poly, subdividing as well #101160

New Issue

jorge esparza · 2022-09-18T06:43:24+02:00

jorge esparza commented

2022-09-18 06:43:24 +02:00

System Information
Operating system: Windows-10-10.0.22000-SP0 64 Bits
Graphics card: NVIDIA GeForce GTX 1650/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 516.94

Blender Version
Broken: version: 3.3.0, branch: master, commit date: 2022-09-06 15:39, hash: 0759f671ce

Caused by 0602852860 and 45d038181a

Short description of error
deleting a point in a nurbs curve permanently converts to poly.

Exact steps for others to reproduce the error
1- add a nurbs circle
2- enter edit mode
3- delete any point
curve is now poly and can´t be changed back to nurbs.
operator does nothing after this point.

While this particular issue reported here is caused by 0602852860, there is another change in behavior (caused by a previous commit):
repro case:

add a nurbs circle
enter editmode
subdivide
is now also poly
(that one is caused by 45d038181a)

**System Information** Operating system: Windows-10-10.0.22000-SP0 64 Bits Graphics card: NVIDIA GeForce GTX 1650/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 516.94 **Blender Version** Broken: version: 3.3.0, branch: master, commit date: 2022-09-06 15:39, hash: `0759f671ce` Caused by 0602852860 and 45d038181a **Short description of error** deleting a point in a nurbs curve permanently converts to poly. **Exact steps for others to reproduce the error** 1- add a nurbs circle 2- enter edit mode 3- delete any point curve is now poly and can´t be changed back to nurbs. operator does nothing after this point. While this particular issue reported here is caused by 0602852860, there is another change in behavior (caused by a previous commit): repro case: - add a nurbs circle - enter editmode - subdivide - is now also poly (that one is caused by 45d038181a)

jorge esparza commented

2022-09-18 06:43:24 +02:00

Added subscriber: @Zombie

Pratik Borhade commented

2022-09-19 06:40:44 +02:00

Added subscriber: @PratikPB2123

Pratik Borhade commented

2022-09-19 06:40:44 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Pratik Borhade commented

2022-09-19 06:40:44 +02:00

Thanks for the report. I can confirm. Something has changed between 3.1-3.2. Checking...

Pratik Borhade commented

2022-09-19 07:41:44 +02:00

and removes the "active spline" section

Panel reappears after making any control point active (with LMB)

> and removes the "active spline" section Panel reappears after making any control point active (with LMB)

jorge esparza commented

2022-09-20 05:01:10 +02:00

strangely enough, after converting to nurbs, it also returns the nurbs type curves, but said curves don´t appear until that panel section is active, maybe a redrawing/refresh issue?

maybe conversion needs to have an "active" point to work, as you can see in the video, i used the operator but no nurbs curve is drawn (maybe because i didn´t had an active vertex selected with lmb first).

i still think there´s a bug there

strangely enough, after converting to nurbs, it also returns the nurbs type curves, but said curves don´t appear until that panel section is active, maybe a redrawing/refresh issue? maybe conversion needs to have an "active" point to work, as you can see in the video, i used the operator but no nurbs curve is drawn (maybe because i didn´t had an active vertex selected with lmb first). i still think there´s a bug there

Philipp Oeser commented

2022-09-23 11:38:29 +02:00

Added subscribers: @laurynas, @pmakal, @HooglyBoogly, @lichtwerk

Philipp Oeser commented

2022-09-23 11:38:29 +02:00

While this particular issue reported here is caused by 0602852860, there is another change in behavior (caused by a previous commit):
repro case:

add a nurbs circle
enter editmode
subdivide
is now also poly
(that one is caused by 45d038181a)

I think we should remove the issue about the Active Spline panel though -- when deleting stuff, we are loosing the active, this is expected behavior I think and has always been the case...

CC @laurynas
CC @HooglyBoogly
CC @pmakal

While this particular issue reported here is caused by 0602852860, there is another change in behavior (caused by a previous commit): repro case: - add a nurbs circle - enter editmode - subdivide - is now also poly (that one is caused by 45d038181a) I think we should remove the issue about the `Active Spline` panel though -- when deleting stuff, we are loosing the `active`, this is expected behavior I think and has always been the case... CC @laurynas CC @HooglyBoogly CC @pmakal

Philipp Oeser changed title from ~~deleting a point in a nurbs curve permanently converts to poly and removes the "active spline" section, also permanently~~ to Regression: deleting a point in a (cyclic) nurbs curve permanently converts to poly, subdividing as well

2022-09-23 11:42:18 +02:00

Laurynas Duburas commented

2022-09-23 13:29:22 +02:00

Active Spline issue is old one. In editcurve.c curve_delete_exec always deselects active curve after control point deletion:
cu->actnu = cu->actvert = CU_ACT_NONE;
Though cu->actnu = CU_ACT_NONE; should be set only after all curve deletion.

Circle point deletion causes problems, because it brings invalid degree and control point count setup for Bezier. After selecting any cp again info message could be seen in the Active Spline panel. The idea was to not to show geometry at all in such cases and with Nurbs Surface Circe behaviour is such. In Nurbs Curves acts differently, for some unknown reason geometry replicating control point leg is shown.

Subdivision results in a sequence 3 point Bezier curves. Those curves start at the corners and their middle cp's lay on the same line, this leads us to the square.
Result maybe not intuitive for user but from math point everything is fine. For Nurbs subdivision like tool should be done by refining knot vector not by subdividing control point leg.

`Active Spline` issue is old one. In editcurve.c `curve_delete_exec` always deselects active curve after control point deletion: `cu->actnu = cu->actvert = CU_ACT_NONE;` Though `cu->actnu = CU_ACT_NONE;` should be set only after all curve deletion. Circle point deletion causes problems, because it brings invalid degree and control point count setup for Bezier. After selecting any cp again info message could be seen in the `Active Spline` panel. The idea was to not to show geometry at all in such cases and with `Nurbs Surface Circe` behaviour is such. In `Nurbs Curves` acts differently, for some unknown reason geometry replicating control point leg is shown. Subdivision results in a sequence 3 point Bezier curves. Those curves start at the corners and their middle cp's lay on the same line, this leads us to the square. Result maybe not intuitive for user but from math point everything is fine. For Nurbs subdivision like tool should be done by refining knot vector not by subdividing control point leg.

Laurynas Duburas commented

2022-09-23 20:04:48 +02:00

Regarding circle shown as a poly after point deletion. It appears invalid NURBS are treated that way:
https://developer.blender.org/rB2d80f814cc249d26b01fe1a306d4f7d1c2ffed90

Regarding circle shown as a poly after point deletion. It appears invalid NURBS are treated that way: [[ https://developer.blender.org/rB2d80f814cc249d26b01fe1a306d4f7d1c2ffed90]]

Philipp Oeser commented

2022-09-23 21:00:24 +02:00

Also: could there be something wrong with versioning code?
Opening a plain Nurbs Circle saved pre 0602852860 in master looks like this now:
{F13561099 size=full}

Also: could there be something wrong with versioning code? Opening a plain Nurbs Circle saved pre 0602852860 in master looks like this now: {[F13561099](https://archive.blender.org/developer/F13561099/image.png) size=full}

Philipp Oeser commented

2022-09-23 21:37:55 +02:00

In #101160#1421560, @laurynas wrote:
Circle point deletion causes problems, because it brings invalid degree and control point count setup for Bezier. After selecting any cp again info message could be seen in the Active Spline panel. The idea was to not to show geometry at all in such cases and with Nurbs Surface Circe behaviour is such. In Nurbs Curves acts differently, for some unknown reason geometry replicating control point leg is shown.

Subdivision results in a sequence 3 point Bezier curves. Those curves start at the corners and their middle cp's lay on the same line, this leads us to the square.
Result maybe not intuitive for user but from math point everything is fine. For Nurbs subdivision like tool should be done by refining knot vector not by subdividing control point leg.

Think get it now (at least understand it much better).
Not quite sure how this was working prior to those commits (because from the user standpoint it did make it intuitive modeling wise -- while probably on other levels this was worsebecause you never knew which options worked together and which were ignored)

In #101160#1421834, @lichtwerk wrote:
Also: could there be something wrong with versioning code?
Opening a plain Nurbs Circle saved pre 0602852860 in master looks like this now:
{F13561099 size=full}

And this is because for the default Nurbs Circle, you had to have Cyclic & Bezier turned on prior to said commit, now it is Cyclic & Bezier & Endpoint, is this not taken care of in versioning?

> In #101160#1421560, @laurynas wrote: > Circle point deletion causes problems, because it brings invalid degree and control point count setup for Bezier. After selecting any cp again info message could be seen in the `Active Spline` panel. The idea was to not to show geometry at all in such cases and with `Nurbs Surface Circe` behaviour is such. In `Nurbs Curves` acts differently, for some unknown reason geometry replicating control point leg is shown. > > Subdivision results in a sequence 3 point Bezier curves. Those curves start at the corners and their middle cp's lay on the same line, this leads us to the square. > Result maybe not intuitive for user but from math point everything is fine. For Nurbs subdivision like tool should be done by refining knot vector not by subdividing control point leg. Think get it now (at least understand it much better). Not quite sure how this was working prior to those commits (because from the user standpoint it did make it intuitive modeling wise -- while probably on other levels this was worsebecause you never knew which options worked together and which were ignored) > In #101160#1421834, @lichtwerk wrote: > Also: could there be something wrong with versioning code? > Opening a plain Nurbs Circle saved pre 0602852860 in master looks like this now: > {[F13561099](https://archive.blender.org/developer/F13561099/image.png) size=full} And this is because for the default Nurbs Circle, you had to have `Cyclic` & `Bezier` turned on prior to said commit, now it is `Cyclic` & `Bezier` & `Endpoint`, is this not taken care of in versioning?

Laurynas Duburas commented

2022-09-23 23:06:26 +02:00

Not quite sure how this was working prior to those commits (because from the user standpoint it did make it intuitive modeling wise -- while probably on other levels this was worsebecause you never knew which options worked together and which were ignored)

Until D11664 it circle was 4 order cyclic Nurbs with appropriate cps and weights, but it wasn't accurate. Differences where seen when compared to mesh circle. Same scheme with same problems was used for sphere, cylinder. I agree that accuracy came at a cost, but Blender's approach to NURBS is to blame at least at some points. For ex. point deletion could give more user friendly result if knot vector was generated only at the creation of primitive.

And this is because for the default Nurbs Circle, you had to have Cyclic & Bezier turned on prior to said commit, now it is Cyclic & Bezier & Endpoint, is this not taken care of in versioning?

Migration code in 0602852860 assumes that files were saved with version before 45d038181a . 45d038181a should have had it's own migration code.

> Not quite sure how this was working prior to those commits (because from the user standpoint it did make it intuitive modeling wise -- while probably on other levels this was worsebecause you never knew which options worked together and which were ignored) Until [D11664](https://archive.blender.org/developer/D11664) it circle was 4 order cyclic Nurbs with appropriate cps and weights, but it wasn't accurate. Differences where seen when compared to mesh circle. Same scheme with same problems was used for sphere, cylinder. I agree that accuracy came at a cost, but Blender's approach to NURBS is to blame at least at some points. For ex. point deletion could give more user friendly result if knot vector was generated only at the creation of primitive. >And this is because for the default Nurbs Circle, you had to have Cyclic & Bezier turned on prior to said commit, now it is Cyclic & Bezier & Endpoint, is this not taken care of in versioning? Migration code in 0602852860 assumes that files were saved with version before 45d038181ae2 . 45d038181ae2 should have had it's own migration code.

blender-admin commented

2022-10-07 23:33:50 +02:00

This issue was referenced by f3f0ea8dc2

This issue was referenced by f3f0ea8dc27e457f61667eb24c476646c46ae0a9

Laurynas Duburas commented

2022-10-11 15:21:52 +02:00

Point deletion. After point deletion a poly line is shown instead of a curve as the situation is mathematically invalid. Previously deletion of one point for a circle was possible because knots for a circle were generated with Bezier mode unchecked. Without Bezier accurate circle or sphere is not possible, more on this in D11664. Pollyline instead of "no geometry at all" appeared with 2d80f814cc.
Active spline. Fixed in D16050. This should ease 1. problem if user models with Active Spline visible, he will see the message "1 more point(s) needed for Bezier" after point deletion.
Subdivision. In this case it is not a poly, but a curve that matches the poly. Try moving control points. Math for Bezier works that way. It's partially price for accurate circle partially naive implementation of Subdivision.
Versioning To fix versioning .blend files saved before 45d038181a should get the modification:

lang=c++
LISTBASE_FOREACH (Curve *, curve, &bmain->curves) {
    LISTBASE_FOREACH (Nurb *, nurb, &curve->nurb) {
        /* Previously other flags were ignored if CU_NURB_CYCLIC is set. */
        if (nurb->flagu & CU_NURB_CYCLIC) {
          nurb->flagu = CU_NURB_CYCLIC;
        }
    }
}

and versioning code in block if (!MAIN_VERSION_ATLEAST(bmain, 302, 6)) { should be fixed to:

lang=c++

...
    LISTBASE_FOREACH (Nurb *, nurb, &curve->nurb) {
        /* Previously other flags were ignored if CU_NURB_CYCLIC is set. */
        if (nurb->flagu & CU_NURB_CYCLIC) {
          nurb->flagu = CU_NURB_CYCLIC | (nurb->flagu & CU_NURB_BEZIER);
        }
...

but I don't know to which MAIN_VERSION_ATLEAST block to put the first chunk. The one for files before 45d038181a.

- **Point deletion.** After point deletion a poly line is shown instead of a curve as the situation is mathematically invalid. Previously deletion of one point for a circle was possible because knots for a circle were generated with `Bezier` mode unchecked. Without `Bezier` accurate circle or sphere is not possible, more on this in [D11664](https://archive.blender.org/developer/D11664). Pollyline instead of "no geometry at all" appeared with 2d80f814cc24. - **Active spline.** Fixed in [D16050](https://archive.blender.org/developer/D16050). This should ease 1. problem if user models with `Active Spline` visible, he will see the message *"1 more point(s) needed for Bezier"* after point deletion. - **Subdivision.** In this case it is not a poly, but a curve that matches the poly. Try moving control points. Math for Bezier works that way. It's partially price for accurate circle partially naive implementation of `Subdivision`. - **Versioning** To fix versioning `.blend` files saved before 45d038181ae2 should get the modification: ``` lang=c++ LISTBASE_FOREACH (Curve *, curve, &bmain->curves) { LISTBASE_FOREACH (Nurb *, nurb, &curve->nurb) { /* Previously other flags were ignored if CU_NURB_CYCLIC is set. */ if (nurb->flagu & CU_NURB_CYCLIC) { nurb->flagu = CU_NURB_CYCLIC; } } } ``` and versioning code in block ` if (!MAIN_VERSION_ATLEAST(bmain, 302, 6)) {` should be fixed to: ``` lang=c++ ... LISTBASE_FOREACH (Nurb *, nurb, &curve->nurb) { /* Previously other flags were ignored if CU_NURB_CYCLIC is set. */ if (nurb->flagu & CU_NURB_CYCLIC) { nurb->flagu = CU_NURB_CYCLIC | (nurb->flagu & CU_NURB_BEZIER); } ... ``` but I don't know to which `MAIN_VERSION_ATLEAST` block to put the first chunk. The one for files before 45d038181ae2.

Hans Goudey commented

2022-11-09 16:40:36 +01:00

Thanks for the investigation @laurynas. It looks like the only remaining thing is fixing the versioning code.

For the first block of versioning, what do you think about using the bump from f756dc4812f3852bba54ee46c21288540f066672?
That commit was on January 28th rather than the 21st, but it seems close enough in the scheme of things.
If that sounds good, could you make a patch with those changes to versioning? Thanks.

Thanks for the investigation @laurynas. It looks like the only remaining thing is fixing the versioning code. For the first block of versioning, what do you think about using the bump from f756dc4812f3852bba54ee46c21288540f066672? That commit was on January 28th rather than the 21st, but it seems close enough in the scheme of things. If that sounds good, could you make a patch with those changes to versioning? Thanks.

blender-admin commented

2022-11-15 01:04:27 +01:00

This issue was referenced by 18af9da572

This issue was referenced by 18af9da572933af85371dddfefeb3cbafab968be

Hans Goudey commented

2022-11-21 19:09:49 +01:00

Changed status from 'Confirmed' to: 'Archived'

Hans Goudey closed this issue

2022-11-21 19:09:49 +01:00

Hans Goudey commented

2022-11-21 19:09:49 +01:00

I'm going to close this report, since various "sub-issues" have been addressed, and the main thing where NURBS curves with invalid degrees/point counts aren't evaluated is the intended behavior.

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Regression: deleting a point in a (cyclic) nurbs curve permanently converts to poly, subdividing as well #101160