Broken example (?)

[Peque]

I spotted a weird/unexpected behavior when displaying an object I was making with CadQuery. "Debugging" this issue led me to one of the CadQuery examples:

import cadquery
from Helpers import show

result = cadquery.Workplane("front").circle(2.0).rect(0.5, 0.75).extrude(0.5)

show(result)

Which results in:

If I change the .circle().rect() to .rect().circle(), then I get:

Is this expected? Can you reproduce this issue?

Using cadquery-freecad-module v1.0.0.1 and FreeCAD 0.16 (unknown revision, installed it from the official Fedora 26 repositories).

[jmwright]

@Peque I get the intended result (cylinder with rectangular hole through it) in FreeCAD 0.16 revision 6710.

In the top menu if you go to Help->About FreeCAD it doesn't show a revision number?

[Peque]

@jmwright It does not show a revision number (says "Unknown").

I will try to compile from source instead of installing from the official Fedora repositories and see what happens.

Thanks and feel free to close this issue. 😊

[jmwright]

Will the AppImage in this link work in Fedora?

https://www.freecadweb.org/wiki/Install_on_Unix

[Peque]

@jmwright I am afraid not.

I will try to compile from source when I have some time and report back if things are still broken.

[jmwright]

Ok. I'll close this issue for now. Feel free to reopen if needed.

[Peque]

@jmwright I just compiled it from source. Tried with the 0.16 and 0.16.6712 tags in their Git repository (there is no 0.16.6710 tag 😕). They both yield the same result (i.e.: bad result, as shown in the screenshots above).

With 0.16.6712 I can see in the "About" section that FreeCAD reports a "6698 +14 (Git)" revision number. So I guess I am not compiling the same exact version you have available in the Ubuntu repositories.

Do you have any idea about why could this be happening? Do you know how could I try the same version you are using in Ubuntu?

PS: using cadquery-freecad-module version v1.0.0.1 (from Git).

[jmwright]

@Peque What version of Fedora are you running, and is FreeCAD in the standard repo? I'll set up a VM to see if I can replicate your issue.

[Peque]

@jmwright Latest (Fedora 26), fully upgraded (sudo dnf upgrade && reboot). FreeCAD is in the standard repository, yes (sudo dnf install freecad).

If you want to compile FreeCAD from the Git repository, you can install dependencies with:

sudo dnf install cmake swig gettext doxygen gcc-c++ gcc dos2unix libXmu-devel desktop-file-utils freeimage-devel mesa-libGLU-devel OCE-devel python-devel python-pyside-devel pyside-tools boost-devel tbb-devel eigen3-devel qt-devel qt-webkit-devel ode-devel xerces-c-devel opencv-devel smesh-devel Coin3-devel SoQt-devel freetype-devel libspnav-devel python-pivy vtk-devel med-devel

And then compile with (I guess you already know this better than me): 😂

mkdir build && cd build
cmake ../
make -j8

Thanks a lot for your help and tell me if I can be of any further assistance. 😊

[jmwright]

@Peque I have a Fedora VM set up and can replicate your results now.

I'm not sure why this is happening. When I create your object manually it works fine. It's probably got something to do with a wire not being closed properly.

One work-around you can use to keep moving forward would be the following.

import cadquery
from Helpers import show

# result = cadquery.Workplane("front").circle(2.0).rect(0.5, 0.75).extrude(0.5)

result = cadquery.Workplane("front").circle(2.0).extrude(0.5) \
                 .faces(">Z").rect(0.5, 0.75).cutThruAll()

show(result)

Depending on how you want the object to change as you alter the parameters, that may or may not work. I think it will work for most cases though.

I know it's not a full solution, but does this at least make it so you can finish your script and get what you need?

Have you tried FreeCAD 0.17 in Fedora? I'm curious whether that would work or not.

[Peque]

@jmwright Yeah, do not worry, I already used a workaround which works well (code does not look so good though 😂). Anyway, happy to see that you can at least reproduce the same issue.

I tried with FreeCAD 0.17 as well (master branch) with the same results.

Thanks for your help. 😊

[jmwright]

When I set FreeCAD up so that I could do Part -> Check Geometry, I got the following.

Face6 is the bottom face of the extruded solid, and Face7 is the top. I checked the 2D wires separately, and they don't show any errors. There's something with how we're using FreeCAD's extrude operation that's introducing the problem. I tried changing the options to our extrude operation and it didn't help. The problem must be closer to the FreeCAD object level.

[adam-james-v]

@jmwright , is this bug related to #111 ?

[jmwright]

@RustyVermeer Looks like it. Thanks for catching it.

@Peque - Rusty recently implemented a fix for this. I know it's been a long time since you've reported it so you may have moved on, but if you grab the latest master (install/update through the 0.17 add-on manager) you should be all set.

[Peque]

@jmwright It seems with the latest changes this works fine:

import cadquery
from Helpers import show

result = cadquery.Workplane("front").circle(2.0).rect(0.5, 0.75).extrude(0.5)

show(result)

However this (changing .circle().rect() to .rect().circle) seems to be broken still:

import cadquery
from Helpers import show

result = cadquery.Workplane("front").rect(0.5, 0.75).circle(2.0).extrude(0.5)

show(result)

Note that I am using FreeCAD 0.16, not 0.17. The stock Fedora 27 version provided with the official repositories. However, when I last tried with 0.17 I was able to reproduce the same results.

[adam-james-v]

I can replicate this behaviour as well. As a matter of fact, this has been true for some time. When I first encountered it, I didn't think it was a bug, and that's still a question on my mind. @jmwright, is this behaviour as expected or not?

When I make the .rect larger than the circle, things behave as we'd expect:

So, what seems to be happening is that the first wire added to the cq object is considered as a face. Then, subsequent wires are checked.

If they are smaller than the first wire, they are correctly combined with the face and result in a hole when extruded.

If they are bigger than the first wire, they are treated independently, and will be extruded as their own face. This is what we see in your sample, @Peque .

So, in short:

• with current behaviour, it is the user's responsibility to draw outermost lines first and work inward to achieve their desired result.
• I am unclear on whether or not that is the intended behaviour.

[dcowden]

We definitely need to fix this. It's not the intent to require the user to draw things in the correct order.

Does this problem persist even after the recent validate() addition?

[adam-james-v]

Yes, this persists.

There is an assumption stated in the sortWiresByBuildOrder function stating:

Outer wires are listed ahead of inner wires

Which implies that we think the wires are already sorted before we call that sort function. I'm a bit confused by that, so if there's something I'm not aware of, let me know!

This function is used in the _extrude function, so I think that's a good place to start poking around in.

I'll see if I can find anything out...

[dcowden]

@RustyVermeer yeah I remember that now, you're right.

We need to move towards just letting all of the edges generated by the user accumulate, and then organize them into wires at the end, right before the extrude.

I say this because when we import geometry from DXF/SVG, we have to do that anyway, so why not use the same code path for manually generated geometry as well?

What will still be missing is a way to sort from outer to inner-- but again, we'll face that with imported geometry anyhow-- there wont be a reliable way to force creation order when we're reading geometry from a file...

[adam-james-v]

First thought for sorting wires is this:

1. compute bounding box per wire
2. compute diagonal
3. sort on absolute length of diagonal, longest to shortest
4. compute diagonal center points
5. add checks according to positioning for situations where a wire may sit way outside of any other. That would result in a compound.

Anyway, that's just rough thinking. As for this particular problem, I have a proposal to solve it:

# This example is meant to be used from within the CadQuery module of FreeCAD.
import cadquery as cq
import Part
from Helpers import show

from cadquery import Solid
from cadquery.freecad_impl.shapes import Face, Shape, Compound
from cadquery.freecad_impl.geom import sortWiresByBuildOrder

# The dimensions of the model. These can be modified rather than changing the
# object's code directly.
circle_radius = 30.0
rectangle_width = 13.0
rectangle_length = 19.0
thickness = 13.0



def _extrude2(self, distance, both=False):

    # wireSets = sortWiresByBuildOrder(list(self.ctx.pendingWires), self.plane, []) # this is the existing line
    wireSets = [list(self.ctx.pendingWires)] # setting it this way bypasses the sort, but works.
    self.ctx.pendingWires = []  # now all of the wires have been used to create an extrusion

    #compute extrusion vector and extrude
    eDir = self.plane.zDir.multiply(distance)

    toFuse = []
    print len(wireSets)
    for ws in wireSets:
        thisObj = Solid.extrudeLinear(ws[0], ws[1:], eDir)
        toFuse.append(thisObj)

        if both:
            thisObj = Solid.extrudeLinear(ws[0], ws[1:], eDir.multiply(-1.))
            toFuse.append(thisObj)

    return Compound.makeCompound(toFuse)


cq.Workplane.extr = _extrude2


result = (cq.Workplane("front")
                                    .rect(rectangle_width, rectangle_length)
                                    .circle(circle_radius)
                                    # .rect(rectangle_width, rectangle_length)
                                    .extr(thickness)
         )

show(cq.CQ(Compound.makeCompound([result])))

If you run that in the CQ module in FreeCAD, it should work, even if you swap the .rect and .circle around.

What I've done is copy exactly the code from _extrude and the only change I made was turning this:

wireSets = sortWiresByBuildOrder(list(self.ctx.pendingWires), self.plane, [])

into this:

wireSets = [list(self.ctx.pendingWires)]

What are the thoughts on this? I don't think it will interfere with anything else to just bypass the sort, especially because it appears that the extrudeLinear function will handle this anyway (when it turns wires into faces)

[jmwright]

@RustyVermeer

...especially because it appears that the extrudeLinear function will handle this anyway (when it turns wires into faces)

So are you saying that our current extrude function is basically double sorting the wires? Is that the source of this problem?

[adam-james-v]

Well, sort of.

Our sort function has a check if plane.isWireInside(outerWire, w): that will always fail if the outerWire is smaller than the next wire being checked.

That failure then results in the function returning a list of lists, where each inner list has only 1 wire.
Then, that ultimately results in each wire being extruded independently.

incorrect extrude behaviour arises from:
[[<cadquery.freecad_impl.shapes.Wire object at 0x0000020AA9D2F470>], [<cadquery.freecad_impl.shapes.Wire object at 0x0000020AACC5DF98>]]

correct behaviour arises from:
[[<cadquery.freecad_impl.shapes.Wire object at 0x0000020A987E6748>, <cadquery.freecad_impl.shapes.Wire object at 0x0000020AA8793A58>]]

Now, bypassing the sort function will leave all wires in one list. Then, when we do a linear extrude, we convert a group of wires into a face. It appears that f = FreeCADPart.Face(freeCADWires) will take care of things for us.

So, we could either change the logic in the sort OR rely on the FreeCAD .Face() function to create the face from a list of wires for us. We already do rely on the .Face() function for this anyway, if I'm understanding all the code correctly.

[Peque]

As I think it is a separate issue, I created #113 just to track this.