Page MenuHome

Curves: Implement Handles for selected points only
ClosedPublic

Authored by Antonio Vazquez (antoniov) on Sat, May 16, 4:09 PM.

Details

Summary

When editing a complex curve is very annoying to have all handles at a time. Also, this is a requirement for the current GSoC Edit Grease Pencil using curves.
I have seen that this improvement can be used in any other area of blender, so I have decided to publish the option in the overlay panel..

Diff Detail

Repository
rB Blender

Event Timeline

Antonio Vazquez (antoniov) requested review of this revision.Sat, May 16, 4:09 PM
This revision is now accepted and ready to land.Sat, May 16, 6:10 PM
  • Remove unused code and change default value

In this current version, box select acts even on the invisible handles. So if you drag a box over an invisible handle, it becomes selected. That differs from what we do in the Graph Editor, where only visible handles can become box-selected.
There's also the Include Handles keymap option for when the handles are already visible, but I don't think we need to add that for curve objects.

DRW code is fine. Note I did not review the UX.

source/blender/draw/engines/overlay/shaders/edit_curve_point_vert.glsl
9

Style: curveHandleType

source/blender/makesdna/DNA_view3d_defaults.h
68

I'm not sure we want this to be the default.

  • Fixed selection problems.
  • Cleanup: Rename variable
  • Change default mode to ALL

    To solve selection a new flag is required to set if the handle is visible or not. Whitout this flag is impossible make it run because the previous selection is removed.

@Julian Eisel (Severin) Look at the selection code to see if I miss something. I had to add a new flag because if the selection operator tries to determine if the handle is visible with f1, f2 or f3does not work because for a lot of operators, before select anything, the variables are cleared, so we cannot know what is visible and what not.

The flag reuse part of _padvariable, so no memory added.

After any operation of selection I run a function to determine what is visible and what not. A handle is visible if any of the three points are selected.

Also, I cahnged the default to ALLas it was by default in Blender.

Julian Eisel (Severin) requested changes to this revision.Tue, May 19, 12:49 PM

I don't see a need for an explicit visibility flag, since visibility depends on selection. But I also don't really understand what you say re f1, f2 and f3 being cleared. That doesn't seem to be an issue from what I can see.

This gives me just the behavior I'd expect for click, box, circle and lasso-select:

diff --git a/source/blender/draw/intern/draw_cache_impl_curve.c b/source/blender/draw/intern/draw_cache_impl_curve.c
index ab683be7999..5578e647ec4 100644
--- a/source/blender/draw/intern/draw_cache_impl_curve.c
+++ b/source/blender/draw/intern/draw_cache_impl_curve.c
@@ -766,7 +766,7 @@ static void curve_create_edit_data_and_handles(CurveRenderData *rdata,
         if (bezt->hide == true) {
           continue;
         }
-        const bool handle_selected = (bezt->flag & BEZT_HANDLER_VISIBLE);
+        const bool handle_selected = BEZT_ISSEL_ANY(bezt);
 
         if (elbp_verts) {
           GPU_indexbuf_add_point_vert(elbp_verts, vbo_len_used + 0);
diff --git a/source/blender/editors/curve/editcurve_query.c b/source/blender/editors/curve/editcurve_query.c
index 0b15d9e55b9..e0f161ca0cf 100644
--- a/source/blender/editors/curve/editcurve_query.c
+++ b/source/blender/editors/curve/editcurve_query.c
@@ -44,8 +44,13 @@
 /** \name Cursor Picking API
  * \{ */
 
-static void ED_curve_pick_vert__do_closest(
-    void *userData, Nurb *nu, BPoint *bp, BezTriple *bezt, int beztindex, const float screen_co[2])
+static void ED_curve_pick_vert__do_closest(void *userData,
+                                           Nurb *nu,
+                                           BPoint *bp,
+                                           BezTriple *bezt,
+                                           int beztindex,
+                                           bool handles_visible,
+                                           const float screen_co[2])
 {
   struct {
     BPoint *bp;
@@ -64,6 +69,8 @@ static void ED_curve_pick_vert__do_closest(
     flag = bp->f1;
   }
   else {
+    BLI_assert(handles_visible || beztindex == 1);
+
     if (beztindex == 0) {
       flag = bezt->f1;
     }
diff --git a/source/blender/editors/include/ED_view3d.h b/source/blender/editors/include/ED_view3d.h
index 668ca3c6437..be07a01f7f9 100644
--- a/source/blender/editors/include/ED_view3d.h
+++ b/source/blender/editors/include/ED_view3d.h
@@ -246,6 +246,7 @@ void nurbs_foreachScreenVert(struct ViewContext *vc,
                                           struct BPoint *bp,
                                           struct BezTriple *bezt,
                                           int beztindex,
+                                          bool handle_visible,
                                           const float screen_co[2]),
                              void *userData,
                              const eV3DProjTest clip_flag);
diff --git a/source/blender/editors/space_view3d/view3d_iterators.c b/source/blender/editors/space_view3d/view3d_iterators.c
index 08e68c9174e..c609b6d80e4 100644
--- a/source/blender/editors/space_view3d/view3d_iterators.c
+++ b/source/blender/editors/space_view3d/view3d_iterators.c
@@ -406,6 +406,7 @@ void nurbs_foreachScreenVert(ViewContext *vc,
                                           BPoint *bp,
                                           BezTriple *bezt,
                                           int beztindex,
+                                          bool handles_visible,
                                           const float screen_co_b[2]),
                              void *userData,
                              const eV3DProjTest clip_flag)
@@ -414,6 +415,8 @@ void nurbs_foreachScreenVert(ViewContext *vc,
   Nurb *nu;
   int i;
   ListBase *nurbs = BKE_curve_editNurbs_get(cu);
+  /* If no point in the triple is selected, the handles are invisible. */
+  const bool only_selected = (vc->v3d->overlay.handle_type == CURVE_HANDLE_SELECTED);
 
   ED_view3d_check_mats_rv3d(vc->rv3d);
 
@@ -427,15 +430,18 @@ void nurbs_foreachScreenVert(ViewContext *vc,
         BezTriple *bezt = &nu->bezt[i];
 
         if (bezt->hide == 0) {
+          const bool handles_visible = (vc->v3d->overlay.edit_flag &
+                                        V3D_OVERLAY_EDIT_CU_HANDLES) &&
+                                       (!only_selected || BEZT_ISSEL_ANY(bezt));
           float screen_co[2];
 
-          if ((vc->v3d->overlay.edit_flag & V3D_OVERLAY_EDIT_CU_HANDLES) == 0) {
+          if (!handles_visible) {
             if (ED_view3d_project_float_object(vc->region,
                                                bezt->vec[1],
                                                screen_co,
                                                V3D_PROJ_RET_CLIP_BB | V3D_PROJ_RET_CLIP_WIN) ==
                 V3D_PROJ_RET_OK) {
-              func(userData, nu, NULL, bezt, 1, screen_co);
+              func(userData, nu, NULL, bezt, 1, false, screen_co);
             }
           }
           else {
@@ -444,21 +450,21 @@ void nurbs_foreachScreenVert(ViewContext *vc,
                                                screen_co,
                                                V3D_PROJ_RET_CLIP_BB | V3D_PROJ_RET_CLIP_WIN) ==
                 V3D_PROJ_RET_OK) {
-              func(userData, nu, NULL, bezt, 0, screen_co);
+              func(userData, nu, NULL, bezt, 0, true, screen_co);
             }
             if (ED_view3d_project_float_object(vc->region,
                                                bezt->vec[1],
                                                screen_co,
                                                V3D_PROJ_RET_CLIP_BB | V3D_PROJ_RET_CLIP_WIN) ==
                 V3D_PROJ_RET_OK) {
-              func(userData, nu, NULL, bezt, 1, screen_co);
+              func(userData, nu, NULL, bezt, 1, true, screen_co);
             }
             if (ED_view3d_project_float_object(vc->region,
                                                bezt->vec[2],
                                                screen_co,
                                                V3D_PROJ_RET_CLIP_BB | V3D_PROJ_RET_CLIP_WIN) ==
                 V3D_PROJ_RET_OK) {
-              func(userData, nu, NULL, bezt, 2, screen_co);
+              func(userData, nu, NULL, bezt, 2, true, screen_co);
             }
           }
         }
@@ -473,7 +479,7 @@ void nurbs_foreachScreenVert(ViewContext *vc,
           if (ED_view3d_project_float_object(
                   vc->region, bp->vec, screen_co, V3D_PROJ_RET_CLIP_BB | V3D_PROJ_RET_CLIP_WIN) ==
               V3D_PROJ_RET_OK) {
-            func(userData, nu, bp, NULL, -1, screen_co);
+            func(userData, nu, bp, NULL, -1, false, screen_co);
           }
         }
       }
diff --git a/source/blender/editors/space_view3d/view3d_select.c b/source/blender/editors/space_view3d/view3d_select.c
index 2350604d690..c2631cf0159 100644
--- a/source/blender/editors/space_view3d/view3d_select.c
+++ b/source/blender/editors/space_view3d/view3d_select.c
@@ -903,6 +903,7 @@ static void do_lasso_select_curve__doSelect(void *userData,
                                             BPoint *bp,
                                             BezTriple *bezt,
                                             int beztindex,
+                                            bool handles_visible,
                                             const float screen_co[2])
 {
   LassoSelectUserData *data = userData;
@@ -918,8 +919,8 @@ static void do_lasso_select_curve__doSelect(void *userData,
     }
   }
   else {
-    if ((data->vc->v3d->overlay.edit_flag & V3D_OVERLAY_EDIT_CU_HANDLES) == 0) {
-      /* can only be (beztindex == 0) here since handles are hidden */
+    if (!handles_visible) {
+      /* can only be (beztindex == 1) here since handles are hidden */
       const bool is_select = bezt->f2 & SELECT;
       const int sel_op_result = ED_select_op_action_deselected(data->sel_op, is_select, is_inside);
       if (sel_op_result != -1) {
@@ -2710,6 +2711,7 @@ static void do_nurbs_box_select__doSelect(void *userData,
                                           BPoint *bp,
                                           BezTriple *bezt,
                                           int beztindex,
+                                          bool handles_visible,
                                           const float screen_co[2])
 {
   BoxSelectUserData *data = userData;
@@ -2724,8 +2726,8 @@ static void do_nurbs_box_select__doSelect(void *userData,
     }
   }
   else {
-    if ((data->vc->v3d->overlay.edit_flag & V3D_OVERLAY_EDIT_CU_HANDLES) == 0) {
-      /* can only be (beztindex == 0) here since handles are hidden */
+    if (!handles_visible) {
+      /* can only be (beztindex == 1) here since handles are hidden */
       const bool is_select = bezt->f2 & SELECT;
       const int sel_op_result = ED_select_op_action_deselected(data->sel_op, is_select, is_inside);
       if (sel_op_result != -1) {
@@ -3653,6 +3655,7 @@ static void nurbscurve_circle_doSelect(void *userData,
                                        BPoint *bp,
                                        BezTriple *bezt,
                                        int beztindex,
+                                       bool handles_visible,
                                        const float screen_co[2])
 {
   CircleSelectUserData *data = userData;
@@ -3662,8 +3665,8 @@ static void nurbscurve_circle_doSelect(void *userData,
       bp->f1 = data->select ? (bp->f1 | SELECT) : (bp->f1 & ~SELECT);
     }
     else {
-      if ((data->vc->v3d->overlay.edit_flag & V3D_OVERLAY_EDIT_CU_HANDLES) == 0) {
-        /* can only be (beztindex == 0) here since handles are hidden */
+      if (!handles_visible) {
+        /* can only be (beztindex == 1) here since handles are hidden */
         bezt->f1 = bezt->f2 = bezt->f3 = data->select ? (bezt->f2 | SELECT) : (bezt->f2 & ~SELECT);
       }
       else {

(This patch probably won't apply, so here's the full diff with your changes: P1404)

Another issue in the current patch:

  • The handle-lines (not the handle points themselves) still show up if selected, even though the Handles checkbox is disabled. If this is disabled, I guess the handles should never show up.
source/blender/makesdna/DNA_view3d_defaults.h
68

I'd also vote for Selected by default. That's consistent to the graph editor then.

source/blender/makesrna/intern/rna_space.c
486–490

Getting a warning on GCC:

In file included from source/blender/makesrna/intern/rna_space_gen.c:33:
/home/guest/blender/software/git/blender/src/source/blender/makesrna/intern/rna_space.c:486:25: warning: ‘rna_enum_curve_handles_items’ defined but not used [-Wunused-variable]
  486 | static EnumPropertyItem rna_enum_curve_handles_items[] = {
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Can be fixed by wrapping into #ifndef RNA_RUNTIME (like done for other enums in this file).

3834–3838

I don't think Type is a descriptive term here. Maybe Limit to, but better ideas welcome.

This revision now requires changes to proceed.Tue, May 19, 12:49 PM
  • Patch simplified.
  • SELECTED as default.
  • Changed to Display Handles for UI.
Julian Eisel (Severin) updated this revision to Diff 24935.EditedTue, May 19, 5:32 PM
  • Fix issue when box/circle/lasso selecting handles only

@Antonio Vazquez (antoniov) made a GIF showing the issue:

One design note: With this change, circle select does not select the handles
when drawing over the center point to select it and unhide the handles.
Otherwise it would be tricky to allow painting the selection properly.

  • Merge Handles and Display type in only one property.

  • Fixed error handles were not hidden when disable handles.
  • Renamed internal variables.

Just a small comment on the tooltip. I also don't see any other colons in the overlays, it might be better to remove it for consistency,

Otherwise I like this! It's nice to see curves getting some love.

source/blender/makesrna/intern/rna_space.c
3830

I would say "Limit the display of curve handles in edit mode"

Each handle is the same type whether it is displayed or not.

source/blender/makesrna/intern/rna_space.c
3830

I will change the tooltip.

  • Keep handles always visible for Nurbs.
This revision is now accepted and ready to land.Mon, May 25, 5:44 PM