Improve bend offset calculations when angle isn't 90deg.

[karliss]

TODO: will prepare test project and screenshots tomorrow.

Previously when doing bend using "Make wall" command "Material Inside" and "Material Outside" modes didn't take into account the angle.

Test plan:

• Using Outer sharp or Inner sharp modes
  • Material inside, 90 > angle > 0
  • Material inside, angle > 90
  • Material inside, -90 < angle < 0
  • Material inside, angle < -90
  • Material outside, 90 > angle > 0
  • Material outside, angle > 90
  • Material outside, -90 < angle < 0
  • Material outside, angle < -90

In each of the cases above changing radius should not affect endpoint

• Using inner sharp mode change angle from -145 to 145
• Using outer sharp mode change angle from -145 to 145
• Using tangential mode change angle from -145 to 145

During the 3 cases described above the shape should change smoothly, negative angles should look the same as positive angle reversed.

[shaise]

Thanks!
Remarks:

1. Can you attach an image of what do think should be the new dimensions if the angle is not 90 degrees?
2. We need to take into account that changing the results of the measurements might break old files that are counting on the current behavior. So we might need to add new modes instead of replacing the current ones.
3. Please stand by with this PR until the bend wall new features are implemented. See Issue Making folded walls on planes or surfaces #307

[karliss]

I feel like might need a few more testcases to convince myself of the new behavior being more useful. Here is what I did so far.
Let's start with the "good" case.

The shape is created by making flat 30mm wall, 140 degree corner 20mm wall, 40 degree corner with 30mm length, -140 degree corner selecting outside with 20mm lenght. First row is using "Material Inside"+"Outer sharp", second row is "Thickness outside"+"Inside sharp". Of course in practice this specific part would be easier to create by using base bend feature, but the same problem would apply for more complex 3d shapes which you couldn't create using base bend from sketch. Only difference between left column and right column is radius.
Note how before patch, the shape changes based on radius, which should not unless "Material Outside" or "Leg" modes are used. After changes it's easy to get desired dimensions and changing radius only affects the radius and nothing else. Also before patch, length for negative angle is completely wrong.
Upstream

Patched:

Now here comes the bad case.

First row "Material inside", second row "Thickness outside". 1st column base radius 1mm side radius 1mm, 2nd column base radius 1mm, side radius 2mm, 3rd column base radius 2mm side radius 2mm, 4th column base radius 1mm side radius mix of 1mm and 2mm.

In this case before patching when all radius are identical due to shape of parallelogram errors partially cancel out so that the two sides at least align, positioning is still off and varies based on radius, negative angle length completely wrong.
In the patched version changing radius doesn't affect position of walls, but they don't align due offset introduced when creating walls from the base,

I have a feeling that if the base shape was different polygon the seeming advantage of upstream behavior might disappear.

Upstream

Patched:

Source of misalignment in patched version

What to do next -> compare the behavior in similar setups with all 90deg angles. More complex polygon. Come up with other cases and test how the new and old behavior makes sense. I am slightly worried that the offset which happens in base bend, might significantly reduce usefulness of the changes I am proposing.

[karliss]

One more observation reopening the project with modified sheetmetal WB and running recompute doesn't cause the sheetmetal part to change. Only if I modify some property of base shape like the material thickness it actually recomputes and updates the shape. Saying this only for the purpose of testing, I am not suggesting that relying on this for project compatibility.

[luzpaz]

This looks like a complex effort. Thank you for all the work you've done so far 🙏

[karliss]

Now that face reference changes are done I guess this is unblocked. Next steps would be:

• rebase on top latest changes and look into whether any of wall changes affect this PR
• project backwards compatibility

Like suggested i am thinking of adding two new modes and renaming the old modes since any new project should probably prefer new modes. I hope that the mode names in project and UI strings are separate. If the list of modes gets's too long/messy then the legacy modes can be hidden by default showing only for objects which had chosen them in older workbench versions.

[shaise]

Yes. Hope rebasing will not make many conflicts

[karliss]

Rebased and added the fixed calculations as new modes. Within task GUI old modes are labelled as "Thickness Outside (legacy)". Unfortunately the property view directly displays the internal names used by code, so it's a bit more confusing there. But the property view also doesn't use localized strings.

After playing with the feature more noticed that there is one aspect of non 90° that would be good to discuss early, as attempting to fix later will introduce more breaking changes in behavior.

If you compare to face reference modes, setting angle to something like 45° essentially establishes a plane. The non obvious part is which of the two planes (going through left corner, or right corner of wall before adding the new bend).

I can think of at least 3 if not more somewhat logical ways it could behave:
a) assume previous wall is on same side -> if you choose material inside assume that previous wall was already inside
b) define plane based on the corner user picked before running bend command
c) top hit -> imagine a plane moving towards unbent wall, which ever corner was hit first stop there. Might be a bit non-intuitive when applied to thickness outside. Maybe "top hit" for material inside and "bottom hit" for thickness outside.

For 90° bends there is no choice since there is single plane going through both corners.

For a) and c) has nice property that it matters less how exactly bend was created.
b) is slightly more powerfull as it allows user to choose which of the 2 planes the wall should be inside or outside of.

Maybe I should try implementing b) and c) to see how useful they are in practice when you try to more complex shapes.

[shaise]

IMO the most important thing is that it is consistent and simple to explain. I does not have to be powerful or make assumptions, or relay on user selecting edges.

[karliss]

I does not have to be powerful or make assumptions

You can't hand wave away inherent choice. There are 2 corners and math needs to pick one. Either the code makes the choice for user, picking the one which we assume will be more useful in most situatuions. Or the user needs to pick themselves implicitly (like the edge which gets selected to start the bend) or explicitly with additional UI element.

And if the choice is done badly, the tool becomes useless.

After a bit of thinking I am somewhat sure that the approach c) doesn't work. Depending on angle it will pick the wrong corner.

For the approach a) how about tweaking icons to something like this:

As for the possibility of explicit choice that could be merged with the offset reference face activation, thus avoiding introduction of more dropdowns than we already have.

Currently there are 2 dropdowns containing nearly identical choices:

• Wall pos
  • Material Outside
  • Material Inside
  • Thickness outside
  • Offset
• Offset position (shown only when offset face is selected)
  • Material Outside
  • Material inside
  • Thickness outside
  • Offset

The alternative d) could be having :

• Bend reference
  • Inside corner
  • Outside corner
  • Face (similar effect as currently selecting offset+reference face)
• Wall pos
  • Material Outside
  • Material Inside
  • Thickness outside
  • Offset

[karliss]

Currently I am leaning more towards a).

Seems like each CAD uses slightly different naming and each provides different subset of choices:

Freecad naming+	Fusion 360	Solidworks	Onshape
Material Outside	Adjacent	Bend Outside	Hold line
Material Inside (old mode)	Tangent	Tangent to bend	-
Thickness Outside (old mode)	-	-	-
[new version] Thickness outside (Inside corner)	Outside	Material Outside	(align) Inside [???]
[not implemented] Thickness Outside (Outside corner)	-	-
[not implemented] Material inside (Inside corner)	-	Material Inside
[new version] Material Inside (Outside corner)	Inside	Bend from Virtual Sharp	(align) Outside [???]
-	-	-	Middle

??? - it's unclear whether Onshape Outside behaves like "Material Inside (Outside corner)", "Material Inside (Inside corner)" or something else since the docs only showed 90° flange, same for "Inside".

Autodesk inventor -> 4 choices corresponding to main modes + hidden checkbox called "Old method" which based on illustration probably changes the inside corner/outside corner choice.

I had noticed before that the old "Material Inside" behavior for sharp angles made at least some sense. Should probably rename it from "Material Inside (legecy)" to something else, might be worth keeping it not just for backwards compatibility.