bezier curve, switching handle mode with curve-point selected freezes one handle #35952

New Issue

David Jeske · 2013-07-02T10:04:50+02:00

David Jeske commented

2013-07-02 10:04:50 +02:00

%%%--- Operating System, Graphics card ---

MacOS X (Macbook Pro Retina)

- Blender version with error, and version that worked ---

Error in 2.67b, 2.68RC1, 2.62, 2.57

- Short description of error ---

If only one bezier curve point is selected when switching the handle-type of the vertex, the switch doesn't work and one curve-point becomes locked and can no longer be rotated (only scaled).

- Steps for others to reproduce the error (preferably based on attached .blend file) ---

add a curve
go into edit mode
select a vertex, drag both bezier handles, witness they both work and are aligned
with only a bezier handle selected, hit "v", toggle it to free..
WITNESS it does NOT switch to free mode, and one bezier handle is rotation locked (shouldn't be)
EXPECTED it to switch the vertex to free mode properly even though only a bezier handle was selected
select the vertex, hit "v", choose free, witness: both handles properly be moved can be moved
with only a bezier handle selected, hit "v", toggle it to "aligned"...
WITNESS: the handles realign, but only one of them is rotatable. the other is rotation locked
EXPECTED: both handles to work ... also expected the UNSELECTED handle to be moved to align (it doesn't take selected handle into account)

(see attached blend where one side of the middle-handle is frozen)

%%%

%%%--- Operating System, Graphics card --- MacOS X (Macbook Pro Retina) - Blender version with error, and version that worked --- Error in 2.67b, 2.68RC1, 2.62, 2.57 - Short description of error --- If *only* one bezier curve point is selected when switching the handle-type of the vertex, the switch doesn't work and one curve-point becomes locked and can no longer be rotated (only scaled). - Steps for others to reproduce the error (preferably based on attached .blend file) --- 1) add a curve 2) go into edit mode 3) select a vertex, drag both bezier handles, witness they both work and are aligned 4) **with only a bezier handle selected**, hit "v", toggle it to free.. 5) WITNESS it does NOT switch to free mode, and one bezier handle is rotation locked (shouldn't be) 6) EXPECTED it to switch the vertex to free mode properly even though only a bezier handle was selected 7) select the vertex, hit "v", choose free, witness: both handles properly be moved can be moved 8) **with only a bezier handle selected**, hit "v", toggle it to "aligned"... 9) WITNESS: the handles realign, but only one of them is rotatable. the other is rotation locked 10) EXPECTED: both handles to work ... also expected the UNSELECTED handle to be moved to align (it doesn't take selected handle into account) (see attached blend where one side of the middle-handle is frozen) %%%

David Jeske commented

2013-07-02 10:04:50 +02:00

Changed status to: 'Open'

David Jeske commented

2013-07-02 21:15:41 +02:00

%%%BTW - I found the source of the problem in BKE_nurbList_handles_set() and I'm testing a patch which I think fixes all the issues. Just trying to figure out how to test the IPO_AUTO_HORIZ / HD_AUTO_ANIM case.

Campbell - random aside, if you look at the current code for this function, you'll see a great example of confugion bugs from non-typesafe code, as there are lots of non-typesafe int/enum/variable munges in there. My patch will remove them and fix the behavior.%%%

%%%BTW - I found the source of the problem in BKE_nurbList_handles_set() and I'm testing a patch which I think fixes all the issues. Just trying to figure out how to test the IPO_AUTO_HORIZ / HD_AUTO_ANIM case. Campbell - random aside, if you look at the current code for this function, you'll see a great example of confugion bugs from non-typesafe code, as there are lots of non-typesafe int/enum/variable munges in there. My patch will remove them and fix the behavior.%%%

David Jeske commented

2013-07-02 23:17:50 +02:00

%%%That turned out a bit more tricky than I thought to get right. I have behavior which seems much "more expected" for edit-mode spline editing.

Before I can call this a "win", I need to better understand these references to HD_AUTO_ANIM and how these spines are used in animation. AFAIK, I have not changed the HD_AUTO_ANIM handle set behavior. However, if animation relies on some of the old odd ALIGN/FREE handle-lockup behavior, I just took that out so I should resolve that for the other users. Who can I talk to about how splines affect animation? (I can also see the IPO code is a copy of this and has some of the same odd behavior)

There are also two remaining handle-type-toggle ambiguities I don't have a strong opinion about how to resolve...

(1) What should "Toggle Free/Align" do to selected Vector handles? Here are the possibilities:

b. Keep current behavior, correct the docs -- treat any selected VECT nodes as if they are aligned and toggle them. > FREE -> ALIGNED... This behavior is simple to understand, but feels a bit odd when there are mixed free and vector handles selected. It's also not what the documentation says, and the user can get this behavior by expliclty setting the handle-type to FREE or ALIGN.

b. Match the docs -- which say it "toggles between Free and Aligned handle types".... by making the toggle ignore handle types which are not free-or-aligned. This seems mor epowerful than "a", matches the docs, and is predictable.. One downside being that with only vector handle(s) selected, toggle does nothing -- which with one handle selected is probably not what the user expected.

c. "Magic" compromise - only if ALL selected nodes are VECT, then toggle VECT to FREE.. but if the selection is mixed, leave the vector nodes alone. This seems like it might be the most powerful option, as mixed-selections will act like "b", while vector-only selections will act like "a".

(2) Should ALIGN/VECT pairs ever be allowed? This is another case of "handle lockup", where the ALIGN handle can't be moved because it's aligned to the VECT. Is this combo relied on anywhere for some useful behavior?

a. Keep current (odd?) behavior - allow the ALIGN/VECT pair... effectively "locking up" the aligned node (since it's stuck against the vector alignment)

b. SImplify the behavior - switch the VECT handle to ALIGN as well, allowing set ALIGN on either handle to turn the spline into an align-spline (removing the VECT side)
%%%

%%%That turned out a bit more tricky than I thought to get right. I have behavior which seems much "more expected" for edit-mode spline editing. Before I can call this a "win", I need to better understand these references to HD_AUTO_ANIM and how these spines are used in animation. AFAIK, I have not changed the HD_AUTO_ANIM handle set behavior. However, if animation relies on some of the old odd ALIGN/FREE handle-lockup behavior, I just took that out so I should resolve that for the other users. Who can I talk to about how splines affect animation? (I can also see the IPO code is a copy of this and has some of the same odd behavior) There are also two remaining handle-type-toggle ambiguities I don't have a strong opinion about how to resolve... (1) What should "Toggle Free/Align" do to selected Vector handles? Here are the possibilities: b. Keep current behavior, correct the docs -- treat any selected VECT nodes as if they are aligned and toggle them. > FREE -> ALIGNED... This behavior is simple to understand, but feels a bit odd when there are mixed free and vector handles selected. It's also not what the documentation says, and the user can get this behavior by expliclty setting the handle-type to FREE or ALIGN. b. Match the docs -- which say it "toggles between Free and Aligned handle types".... by making the toggle ignore handle types which are not free-or-aligned. This seems mor epowerful than "a", matches the docs, and is predictable.. One downside being that with only vector handle(s) selected, toggle does nothing -- which with one handle selected is probably not what the user expected. c. "Magic" compromise - only if ALL selected nodes are VECT, then toggle VECT to FREE.. but if the selection is mixed, leave the vector nodes alone. This seems like it might be the most powerful option, as mixed-selections will act like "b", while vector-only selections will act like "a". (2) Should ALIGN/VECT pairs ever be allowed? This is another case of "handle lockup", where the ALIGN handle can't be moved because it's aligned to the VECT. Is this combo relied on anywhere for some useful behavior? a. Keep current (odd?) behavior - allow the ALIGN/VECT pair... effectively "locking up" the aligned node (since it's stuck against the vector alignment) b. SImplify the behavior - switch the VECT handle to ALIGN as well, allowing set ALIGN on either handle to turn the spline into an align-spline (removing the VECT side) %%%

David Jeske commented

2013-07-03 10:09:56 +02:00

%%%After receiving some artist feedback, I'm working on a slightly more complex "no compromise" win-win-win patch... Here is the plan:

DONE - fix bug preventing "scale" from working on a single selected spline handle
DONE - fix the bug above and prevent unintended "rotation constraint" (“free” -> free/free and “align” -> align/align)
DONE - make sure toggle free/align behavior stays the same (aka, toggle vector to free, etc) -- but ....
DONE - make sure it does not have the bug above, aka, don't create "rotation constrained" handles

todo - add “constrain” menu option (to intentionally create align/free, align/vector constrained handles)
todo - add draw visuals for “constrained” handles ( plan : "T" the handle-end )
todo - test HD_AUTO_ANIM / IPO_AUTO_HORIZ options
todo - review graph-editor spline toggle-behavior (match above changes if necessary)

maybe - “align” should always align the handle you select to the other
maybe - add “mirror length” mode (just like align, but where the handle lengths are kept the same)... how?
maybe - add "constrain/constrain" mode to lock rotation of both handles (align/align allows rotation)

%%%

%%%After receiving some artist feedback, I'm working on a slightly more complex "no compromise" win-win-win patch... Here is the plan: DONE - fix bug preventing "scale" from working on a single selected spline handle DONE - fix the bug above and prevent unintended "rotation constraint" (“free” -> free/free and “align” -> align/align) DONE - make sure toggle free/align behavior stays the same (aka, toggle vector to free, etc) -- but .... DONE - make sure it does not have the bug above, aka, don't create "rotation constrained" handles todo - add “constrain” menu option (to intentionally create align/free, align/vector constrained handles) todo - add draw visuals for “constrained” handles ( plan : "T" the handle-end ) todo - test HD_AUTO_ANIM / IPO_AUTO_HORIZ options todo - review graph-editor spline toggle-behavior (match above changes if necessary) maybe - “align” should always align the handle you select to the other maybe - add “mirror length” mode (just like align, but where the handle lengths are kept the same)... how? maybe - add "constrain/constrain" mode to lock rotation of both handles (align/align allows rotation) %%%

Sergey Sharybin commented

2014-01-02 19:06:40 +01:00

@DavidJeske, sorry for the delay.

From what i could see here now only a possible(?) crappyness with one aligned handle and another free handle. In this case aligned handle is aligned to the free handle and trying to move it wouldn't work (because it stays aligned to free handle obviously :)

I'm thinking about making it so aligned handle will become free when you're transforming them in such way.

Are there any other issues in the latest blender builds? Also not sure, were your patches commited (don't see them apparently)?

@DavidJeske, sorry for the delay. From what i could see here now only a possible(?) crappyness with one aligned handle and another free handle. In this case aligned handle is aligned to the free handle and trying to move it wouldn't work (because it stays aligned to free handle obviously :) I'm thinking about making it so aligned handle will become free when you're transforming them in such way. Are there any other issues in the latest blender builds? Also not sure, were your patches commited (don't see them apparently)?

David Jeske commented

2014-01-10 10:23:59 +01:00

It was too confusing explaining one patch with several changes, so I agreed it would be simpler to split them up. I did get one fix committed, but I ran out of time (and energy) to separate the other pieces.

This is the small change that made it in..

https://developer.blender.org/T36901

My bigger patch fixed the following problems...

resolves wording/behavior ambiguities in handle-type-set when a SINGLE handle is selected (see below)
makes it possible to "Scale" a single-selected spline handle, assuring it's rotation is not changed (previously you could only scale both handles together)
adds a visual indicator that a spline handle is rotation locked ("T" the end of the spline) - http://www.pasteall.org/pic/54987
makes the code safer/clearer by introducing the enum eHandleSetModes, instead of using integer conventions
fixes BUG: where two handles selected without the anchor had incorrect spline handle selection highlighting
fixes BUG: where a single selected handle would not toggle to "Automatic" correctly

I made two versions of this patch, each with slightly different versions of #1...

https://developer.blender.org/T36029

https://developer.blender.org/T36048

It was too confusing explaining one patch with several changes, so I agreed it would be simpler to split them up. I did get one fix committed, but I ran out of time (and energy) to separate the other pieces. This is the small change that made it in.. https://developer.blender.org/T36901 My bigger patch fixed the following problems... 1) resolves wording/behavior ambiguities in handle-type-set when a SINGLE handle is selected (see below) 2) makes it possible to "Scale" a single-selected spline handle, assuring it's rotation is not changed (previously you could only scale both handles together) 3) adds a visual indicator that a spline handle is rotation locked ("T" the end of the spline) - http://www.pasteall.org/pic/54987 4) makes the code safer/clearer by introducing the enum eHandleSetModes, instead of using integer conventions 5) fixes BUG: where two handles selected without the anchor had incorrect spline handle selection highlighting 6) fixes BUG: where a single selected handle would not toggle to "Automatic" correctly I made two versions of this patch, each with slightly different versions of #1... https://developer.blender.org/T36029 https://developer.blender.org/T36048

Sergey Sharybin commented

2014-08-27 12:43:34 +02:00

All this is becoming a bit of difficult to manage. You're referencing to some patches tasks for which are closed. Also, you've started with a bug report and then switched to the patches.

So in order to clarify all the things:

What are the remaining bugs (if any) ?
If you've got bugs to be reported and don't have time for patch -- please report them as a bug.
If you've got patches which solves some bugs, please submit them to the patch review, not to the bug tracker.

Currently i'm being quite confused about what's going on here, what you want us to fix and what you want us to review.

All this is becoming a bit of difficult to manage. You're referencing to some patches tasks for which are closed. Also, you've started with a bug report and then switched to the patches. So in order to clarify all the things: - What are the remaining _bugs_ (if any) ? - If you've got bugs to be reported and don't have time for patch -- please report them as a bug. - If you've got patches which solves some bugs, please submit them to the patch review, not to the bug tracker. Currently i'm being quite confused about what's going on here, what you want us to fix and what you want us to review.

Campbell Barton commented

2014-09-04 07:37:10 +02:00

Added subscriber: @ideasman42

Campbell Barton commented

2014-09-04 07:37:10 +02:00

Confirm that the original bug still happens,

To try simplify this a bit...

When one handle is aligned and another is free, it doesn't make sense to align either handles (both should act as if they're free).

Simple check:
P135: Possible fix for #35952

diff --git a/source/blender/blenkernel/intern/curve.c b/source/blender/blenkernel/intern/curve.c
index 730bffd..2d053dc 100644
--- a/source/blender/blenkernel/intern/curve.c
+++ b/source/blender/blenkernel/intern/curve.c
@@ -3209,7 +3209,11 @@ static void calchandleNurb_intern(BezTriple *bezt, BezTriple *prev, BezTriple *n
 		madd_v3_v3v3fl(p2_h2, p2, dvec_b,  1.0f / 3.0f);
 	}
 
-	if (skip_align || (!ELEM(HD_ALIGN, bezt->h1, bezt->h2) && !ELEM(HD_ALIGN_DOUBLESIDE, bezt->h1, bezt->h2))) {
+	if (skip_align ||
+	    (!ELEM(HD_ALIGN, bezt->h1, bezt->h2) && !ELEM(HD_ALIGN_DOUBLESIDE, bezt->h1, bezt->h2)) ||
+	    (ELEM(HD_FREE, bezt->h1, bezt->h2))
+	    )
+	{
 		/* handles need to be updated during animation and applying stuff like hooks,
 		 * but in such situations it's quite difficult to distinguish in which order
 		 * align handles should be aligned so skip them for now */

Confirm that the original bug still happens, To try simplify this a bit... When one handle is aligned and another is free, it doesn't make sense to align either handles (both should act as if they're free). Simple check: [P135: Possible fix for #35952](https://archive.blender.org/developer/P135.txt) ```diff diff --git a/source/blender/blenkernel/intern/curve.c b/source/blender/blenkernel/intern/curve.c index 730bffd..2d053dc 100644 --- a/source/blender/blenkernel/intern/curve.c +++ b/source/blender/blenkernel/intern/curve.c @@ -3209,7 +3209,11 @@ static void calchandleNurb_intern(BezTriple *bezt, BezTriple *prev, BezTriple *n madd_v3_v3v3fl(p2_h2, p2, dvec_b, 1.0f / 3.0f); } - if (skip_align || (!ELEM(HD_ALIGN, bezt->h1, bezt->h2) && !ELEM(HD_ALIGN_DOUBLESIDE, bezt->h1, bezt->h2))) { + if (skip_align || + (!ELEM(HD_ALIGN, bezt->h1, bezt->h2) && !ELEM(HD_ALIGN_DOUBLESIDE, bezt->h1, bezt->h2)) || + (ELEM(HD_FREE, bezt->h1, bezt->h2)) + ) + { /* handles need to be updated during animation and applying stuff like hooks, * but in such situations it's quite difficult to distinguish in which order * align handles should be aligned so skip them for now */ ```

blender-admin commented

2014-09-04 07:43:21 +02:00

This issue was referenced by 4987eb4dc9

This issue was referenced by 4987eb4dc926fa4dacc2031fbd1aba044e959025

Campbell Barton commented

2014-09-04 07:44:30 +02:00

Changed status from 'Open' to: 'Resolved'

Campbell Barton closed this issue

2014-09-04 07:44:30 +02:00

Campbell Barton commented

2014-09-04 07:44:30 +02:00

Closed by commit 4987eb4dc9.

Closed by commit 4987eb4dc9.

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

bezier curve, switching handle mode with curve-point selected freezes one handle #35952