Page MenuHome

Bevel Tool: Add functionality for width-method Width and Depth, when 'Only Vertices' is selected.
ClosedPublic

Authored by George Vogiatzis (Gvgeo) on Apr 22 2019, 10:49 AM.

Details

Summary

Bevel Tool:

  1. Add functionality for width-method Width and Depth, when 'Only Vertices' is selected.
  2. Move vertex_only's weight calculations, to accommodate the necessary order for vert_axis calculations.
  3. remove unused (weight <= 0.0f) check.
  4. remove unused line
v2 = BM_edge_other_vert(e->e, bv->v);

Diff Detail

Repository
rB Blender

Event Timeline

Howard Trickey (howardt) requested changes to this revision.Apr 26 2019, 2:48 PM

This looks good. Thanks for doing this. The solution for what to do about 'width' seems like the best approximate solution to a somewhat undefined problem.

Please make the following minor changes:

  1. change the comment before your changes to "Transfer to e->offset_[lr]_spec according to offset_type"
  2. remove the '(float)M_PI - " from the two computations for z. They aren't needed by the properties of sin and cos and the fact that you take fabs afterwards.
  3. run this code through clang_format
This revision now requires changes to proceed.Apr 26 2019, 2:48 PM
George Vogiatzis (Gvgeo) updated this revision to Diff 14953.

Changes made.

  1. changed comment.
  2. fixed basic trigonometry.
  3. Added brackets for cases. Now clang format looks fine. But need to mention that, style guide wants opening bracket in next line.
  4. Take as center axis(vert_axis), the average of the vertex's edges direction, instead of the normal.
  5. Move vertex_only's weight calculations, to accommodate the necessary order for vert_axis calculations. (Avoid checking twice for vertex_only)

Glad to hear.

This patch was a mean, to pass my idea to you primarily. I did not double check it.
Now that I looked closer, using vertex normal can create unwanted shapes and problems.
Here is a file with examples (need previous diff).


Made change 4 to fix it.

Normal could be added as an option, too. Could give better vertex bevel, or even edge bevel.
But I question who would really benefit from it.

Why did you move the weight transfer stuff for vertex_only down? It seems that it now moves it past the point where the BM_ELEM tags were needed and set/unset.

George Vogiatzis (Gvgeo) set the repository for this revision to rB Blender.
George Vogiatzis (Gvgeo) updated this revision to Diff 14985.
  1. remove unused (weight <= 0.0f) check.

I moved them, to avoid double if (vertex_only). But preferred to find center axis after the edges have completely calculated.
Although, it should possible to find it after the find_bevel_edge_order, if needed (have not checked, to confirm it).

But forgot this part.
If I'm not wrong, (weight <= 0.0f) is never true.
Maybe you prefer to keep it, to avoid problems in future.
Here are parts from the code, with the 2 cases.

    if (bp->dvert != NULL && bp->vertex_group != -1) {
      weight = defvert_find_weight(bp->dvert + BM_elem_index_get(v), bp->vertex_group);
      if (weight <= 0.0f) {     <---------------- never true
        BM_elem_flag_disable(v, BM_ELEM_TAG);
        return NULL;
	  }
      bv->offset *= weight;
    }

Bevel modifier call.

static Mesh *applyModifier(ModifierData *md, const ModifierEvalContext *ctx, Mesh *mesh)
{
  .
  .
  .
  if (vertex_only) {
    BM_ITER_MESH (v, &iter, bm, BM_VERTS_OF_MESH) {
      if (!BM_vert_is_manifold(v)) {
        continue;
      }
      if (bmd->lim_flags & MOD_BEVEL_WEIGHT) {
        weight = BM_elem_float_data_get(&bm->vdata, v, CD_BWEIGHT);
        if (weight == 0.0f) {
          continue;
        }
      }
      else if (vgroup != -1) {
        weight = defvert_array_find_weight_safe(dvert, BM_elem_index_get(v), vgroup);
        /* Check is against 0.5 rather than != 0.0 because cascaded bevel modifiers will
         * interpolate weights for newly created vertices, and may cause unexpected "selection" */
        if (weight < 0.5f) {      <---------------- Will skip BM_elem_flag_enable. Without flag will not even reach bevel_vert_construct function.
          continue;
        }
      }
      BM_elem_flag_enable(v, BM_ELEM_TAG);
    }
  }
  .
  .
  .
  BM_mesh_bevel(   );
  .
  .
  .
  }

Bevel edit mode call

void bmo_bevel_exec(BMesh *bm, BMOperator *op)
{
	.
	.
	.
	BM_mesh_bevel(bm,
                  offset,
                  offset_type,
                  seg,
                  profile,
                  vonly,
                  false,
                  clamp_overlap,
                  NULL,				<----- Sends *dvert NULL
                  -1,                              <- and vertex_group -1
                  material,
                  loop_slide,
                  mark_seam,
                  mark_sharp,
                  harden_normals,
                  face_strength_mode,
                  miter_outer,
                  miter_inner,
                  spread,
                  smoothresh);

    BMO_slot_buffer_from_enabled_hflag(bm, op, op->slots_out, "faces.out", BM_FACE, BM_ELEM_TAG);
    BMO_slot_buffer_from_enabled_hflag(bm, op, op->slots_out, "edges.out", BM_EDGE, BM_ELEM_TAG);
    BMO_slot_buffer_from_enabled_hflag(bm, op, op->slots_out, "verts.out", BM_VERT, BM_ELEM_TAG);
  }
}

This looks good now. I agree with you that the weight <=0 case cannot occur at that point in the code.
I've submitted these changes now, with rB3e780507bd6 .
Thanks for your help on this!

This revision is now accepted and ready to land.Apr 30 2019, 1:17 PM