jmwright / cadquery-freecad-module

A module-workbench combo that adds a CadQuery editor to FreeCAD
GNU Lesser General Public License v3.0
120 stars 36 forks source link

Asymmetric chamfer problem #85

Closed ghost closed 7 years ago

ghost commented 7 years ago

I am trying to apply an asymmetric chamfer to the top edges of this object:

image

Symmetric chamfer works OK:

image

import cadquery as cq
from Helpers import show
if "module" in __name__:
    box1 = cq.Workplane("XY").box(10, 5, 1)\
        .faces(">X").edges("|Z").chamfer(1)\
        .faces(">Z").edges(">Z").chamfer(0.5)
    show(box1)

When I try different values for length and length2, they get swapped round on one of the edges. Most of the edges have a "wide and shallow" chamfer, but one edge has "narrow and deep".

image

import cadquery as cq
from Helpers import show
if "module" in __name__:
    box2 = cq.Workplane("XY").box(10, 5, 1)\
        .faces(">X").edges("|Z").chamfer(1)\
        .faces(">Z").edges(">Z").chamfer(0.5, 0.2)
    show(box2)

Am I doing something wrong?

Thanks

jmwright commented 7 years ago

If you add the asymmetric chamfers manually through the FreeCAD GUI do they work right?

Faces have a concept of a normal direction that can effect asymmetric operations, but I don't think there's anything like that for edges. I'm a little puzzled at the moment. I can sit down with it later and try a few things.

ghost commented 7 years ago

Results from applying chamfer manually in Part workbench ...

image

Same results from 0.16 and latest daily.

So it looks like it's a FreeCAD thing?

jmwright commented 7 years ago

Unfortunately yes, it looks like a FreeCAD problem. However, I would try selecting that top face, select all its edges, and then iterate over them. I think that calling each on the selected edges should work because it will apply the chamfers​ one at a time.

http://dcowden.github.io/cadquery/classreference.html?highlight=each#cadquery.Workplane.each

ghost commented 7 years ago

Need help getting the callback to work :disappointed:

Error is 'Edge' object has no attribute 'chamfer' with the code below.

import cadquery as cq
from Helpers import show

def my_chamfer(e):
    return e.chamfer(0.5, 0.2)

if "module" in __name__:

    box = cq.Workplane("XY").box(10, 5, 1)\
        .faces(">X").edges("|Z").chamfer(1)\
        .faces(">Z").edges(">Z").each(my_chamfer)

    show(box)
jmwright commented 7 years ago

I'm struggling to find an example using each for you. I don't think you want a selector string in the last edges() call since you already have just the face. Try that, and if that doesn't work I'll have to experiment with it a little bit when I'm back at my computer.

@dcowden Any ideas?

dcowden commented 7 years ago

I will dig up an example or two...

On Apr 29, 2017 9:14 AM, "Jeremy Wright" notifications@github.com wrote:

I'm struggling to find an example using each for you. I don't think you want a selector string in the last edges() call since you already have just the face. Try that, and if that doesn't work I'll have to experiment with it a little bit when I'm back at my computer.

@dcowden https://github.com/dcowden Any ideas?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jmwright/cadquery-freecad-module/issues/85#issuecomment-298168363, or mute the thread https://github.com/notifications/unsubscribe-auth/ABPOA4IuuFbj6ee2DrcbD6KLyKt4KmMVks5r0zfBgaJpZM4NMOk8 .

jmwright commented 7 years ago

@dcowden Awesome, thanks.

jmwright commented 7 years ago

@hackscribble While we're finding time to look for a decent example, you can try wrapping chamfer in a function and pass that to each, using this code as an example.

https://github.com/dcowden/cadquery/blob/e3d8bacf9519fc575f9b00daa99e75c62b7d0db4/cadquery/cq.py#L727

ghost commented 7 years ago

I couldn't get each() to work, but I have found a workaround ...

import cadquery as cq
from Helpers import show

l1 = 0.5
l2 = 0.2
if "module" in __name__:
    box = cq.Workplane("XY").box(10, 5, 1)\
        .faces(">X").edges("|Z").chamfer(1)
    box = box.faces(">Z").edges("not(<X or >X or <Y or >Y)").chamfer(l1, l2)
    box = box.faces(">Y").edges(">Z").chamfer(l1, l2)
    box = box.faces("<Y").edges(">Z").chamfer(l1, l2)
    box = box.faces("<X").edges(">Z").chamfer(l1, l2)
    box = box.faces(">X").edges(">Z").chamfer(l1, l2)
    show(box)

image

Using "not(<X or >X or <Y or >Y)" as the selector was a bit of a fluke - I guessed that maybe edges are ranked based on their centre points, so the two 45-degree sides aren't max or min X or Y.

Then I had to switch to selecting X and Y faces followed by ">Z" edges, otherwise I still got the same chamfer problem.

Since this is a FreeCAD quirk anyway, and I have a workaround, I'm happy to close the issue, unless you want to add anything @jmwright and @dcowden. Thank you both :smile:

jmwright commented 7 years ago

@hackscribble Glad you've got it working. It seems like there should be a more intuitive way to do that though. I think @dcowden is looking at it now.

It looks like the chamfer function expects a solid because it pulls the edges from the solid. That's why it doesn't work when called on an edge object (can't pull edges from an edge). It seems intuitive to me that chamfer and fillet could be called on a selection of edges, but I'm not sure how easy that is to implement.

dcowden commented 7 years ago

Hi, everyone:

Sorry for the delay on this. This situation is more complex than i thought.

@hackscribble, your guess at using each() was reasonable, and would work in many cases. In this case, though, the chamfer function needs edges AND a solid. each won't work here due to that issue.

To do it idiomatically with cq, you use the context solid, and then select edges on the stack. Your workaround is probably the best way.

There is another way i was thinking would work, but doesnt for a reason I cannot figure out:

import cadquery as cq
from Helpers import show

if "module" in __name__:

    box = cq.Workplane("XY").box(10, 5, 1).faces(">X").edges("|Z").chamfer(1)
    edge_query = box.faces(">Z").edges(">Z")

    for i in range(edge_query.size()):
        edge_query.item(i).chamfer(0.8,0.2)

    show(box)

another possible solution would be to use the all() method, which returns the edges.

jmwright commented 7 years ago

@dcowden Thanks! Does your new solution throw an error, or does it not do the chamfers correctly?

@hackscribble Sorry for steering you wrong on each(). As Dave mentioned, this case ended up being more complex than expected.

dcowden commented 7 years ago

@jmwright it throws an error:

TCollection_IndexedDataMap::FindFromKey

The problem here is that for chamfer we need a reference to both a solid and a valid edge on THAT solid. When we select 6 edges, the first chamfer we do works. But then, the original solid no longer exists-- it is re-created. Now, the other 5 'original' edges are now invalid in the context of the new solid.

Considering that fact, something like this is a nice try:

import cadquery as cq
from Helpers import show

if "module" in __name__:

    box = cq.Workplane("XY").box(10, 5, 1).faces(">X").edges("|Z").chamfer(1)
    edge_query = box.faces(">Z").edges(">Z")

    for i in range(edge_query.size()):
        box = box.faces(">Z").edges(">Z").item(i).chamfer(0.8,0.2)

    show(box)

Here we re-execute the edge query each loop through, so that each query result is a valid edge on the most recently selected solid. But this doesnt work either, because the edges for the newly chamfered surfaces are getting thrown into the mix.

This is another case where the CQ 2.0 'created by' and 'modified by' selectors are what is needed. To solve this problem without resorting to selecting each edge manually, we need to combine the >Z selector, which will select all edges on the top face, with a 'created by' selector, so that we can filter out the edges that are the result of a chamfer that's already been applied:

box = cq.Workplane("XY").box(10, 5, 1).faces(">X").edges("|Z").chamfer(1,id="first_chamfer")

while True:
     q = box.faces(">Z").edges(created_by="first_chamfer")
     if q.size() > 0:
          box = q.item(0).chamfer(0.8,0.2)
     else:
         break;
jmwright commented 7 years ago

Ok, thanks for the explanation. I'm closing this for now since @hackscribble is up and running, and we'll roll the lessons learned into the argument for CQ 2.0.