Page MenuHome

Bevel amount less than 1 mm and profile at 1 doesn't give expected result
Confirmed, NormalPublicBUG

Description

Blender Version
Broken: version: 2.91.0 Alpha, branch: master, commit date: 2020-08-03 05:14, hash: rB144f780c71ed, 2.79

Short description of error
When the bevel amount is smaller than 1mm., the bevel profile shape at 1 doesn't give the expected result.
The 'Depth' width type starts having the issue from 0.65 mm and below. All other width types suffer from 0.9 mm and below.

Exact steps for others to reproduce the error


Try changing the bevel amount and the width type in the above file.

Event Timeline

Hans Goudey (HooglyBoogly) changed the task status from Needs Triage to Confirmed.EditedAug 3 2020, 4:29 PM
Hans Goudey (HooglyBoogly) changed the subtype of this task from "Report" to "Bug".

I'm guessing this is related to precision issues. Although I think it only shows up in the weld case (two beveled edges connecting to each other).



Pasang Bomjan (irex124) updated the task description. (Show Details)

Small correction: I actually meant to say 'width type' not 'limit method'

Although I think it only shows up in the weld case (two beveled edges connecting to each other).

Only happens when two connecting edges are beveled. It doesn't happen with three edges.

Okay, I found the issue, or at least where it comes from.

Normally when the profile parameters are built for the weld case the profile on each side is moved so that it is coplanar with the unbeveled edges on each side of it.
But in this case, the bevel amount isn't large enough for this to happen.

In move_weld_profile_planes:

sub_v3_v3v3(d1, bv->v->co, bndv1->nv.co);
sub_v3_v3v3(d2, bv->v->co, bndv2->nv.co);
cross_v3_v3v3(no, d1, d2);
l1 = normalize_v3(no);
...
if (l1 > BEVEL_EPSILON && (l2 > BEVEL_EPSILON || l3 > BEVEL_EPSILON)) {
  /* Move the profile planes. */
}

Just tweaking the epsilons to ...l1 > BEVEL_EPSILON_SQ... fixes my particular test case. But what I'm not understanding is why we need these checks at all.

If I remove all the checks from this function like in the following code the tests still pass and I haven't noticed any different behavior.

sub_v3_v3v3(d1, bv->v->co, bndv1->nv.co);
sub_v3_v3v3(d2, bv->v->co, bndv2->nv.co);
cross_v3_v3v3(no, d1, d2);
l1 = normalize_v3(no);
/* "no" is new normal projection plane, but don't move if it is coplanar with both of the 
 * projection dirs. */
cross_v3_v3v3(no2, d1, bndv1->profile.proj_dir);
l2 = normalize_v3(no2);
cross_v3_v3v3(no3, d2, bndv2->profile.proj_dir);
l3 = normalize_v3(no3);
// if (l1 > BEVEL_EPSILON && (l2 > BEVEL_EPSILON || l3 > BEVEL_EPSILON)) {
dot1 = fabsf(dot_v3v3(no, no2));
dot2 = fabsf(dot_v3v3(no, no3));
// if (fabsf(dot1 - 1.0f) > BEVEL_EPSILON) {
copy_v3_v3(bndv1->profile.plane_no, no);
// }
// if (fabsf(dot2 - 1.0f) > BEVEL_EPSILON) {
copy_v3_v3(bndv2->profile.plane_no, no);
// }
// }

Theoretically, I'm also not sure why we would only want to use these new normals if they're not coplanar.

@Howard Trickey (howardt) Maybe you remember the reasoning behind those checks or it's clear to you that I'm missing something?

Here's a video for some extra clarity:

Sorry, I can't remember what I had in mind when I put that restriction. I would be a bit hesitant to remove it because I probably did have a particular case in mind. I wasn't as comprehensive as I should have been in having tests for all possible cases, sorry, so the fact that the tests pass without those conditions doesn't give me a large amount of reassurance.