CadQuery / cadquery

A python parametric CAD scripting framework based on OCCT
https://cadquery.readthedocs.io
Other
3.19k stars 289 forks source link

Using 2 times pushPoints() offsets positions #202

Closed bragostin closed 3 years ago

bragostin commented 5 years ago

Running the code below produces the results shown in the attached image: there is an offset of 0.6 mm between the holes through the shell and the holes through the cylinders while they should be coincident. In some other cases it goes up to 1.6 mm. Could this be coming from some accuracy setting somewhere?

import numpy as np
import cadquery as cq

y_sh = np.array([14.5,15,15,14.5,29.5,62.3,70,70,119,119,125,125,129.8,131.8,132,132]) - 140./2.
z_sh = np.array([22.5,39.5,89.5,107.5,15.9,122.5,22.5,107.5,22.5,107.5,39.5,89.5,140.,15.9,108,126]) - 145./2.
dsh = np.array([6.5,6.5,6.5,6.5,5.5,5.5,6.5,6.5,6.5,6.5,6.5,6.5,5.5,5.5,5.5,5.5])

cq_object = cq.Workplane('XY').box(5, 140, 145).cut(cq.Workplane("XY" ).box(4, 138, 143))

y_pl = np.linspace(-64, 64, 13)
z_pl = np.linspace(-66.5, 66.5, 13)
y_pl, z_pl = np.meshgrid(y_pl, z_pl)
yz_pl = [(y,z) for y,z in zip(y_pl.flatten(), z_pl.flatten())]
pl = cq.Workplane('YZ').workplane(offset=-2).polygon(4, 10).extrude(4)
def s_pillar(pos):
    return pl.translate(pos).val()
pillars = cq.Workplane('YZ').pushPoints(yz_pl).each(s_pillar)

for dh in np.unique(dsh):
    ysh = y_sh[dsh==dh]
    zsh = z_sh[dsh==dh]
    yzsh = [(y,z) for y,z in zip(ysh.flatten(), zsh.flatten())]
    cq_object = cq_object.faces('<X').workplane().pushPoints(yzsh).polygon(12, dh).cutBlind(-5)
    pillars = pillars.faces('<X').workplane().pushPoints(yzsh).hole(dh)

show_object(cq_object)
show_object(pillars)

holes_pos

adam-urbanczyk commented 4 years ago

@bragostin could you try the following in your loop?

pillars = pillars.faces('<X').workplane(centerOption='ProjectedOrigin')

By default workplane() uses center of mass to calculate the new origin position.

dcowden commented 4 years ago

@bragostin @adam-urbanczyk I think this behavior might be the single largest confusion. Its my fault-- it seemed like a good idea at the time.

Do you guys think that the default behavior of workplane() should be the global origin instead of center of mass? I think MOST of the time people expect global coordinates. The center of mass is handy, but i think you're more likely to have a good experience overall if you first have what you expect work, and then you seek out documentation for how to find the new origin.

The big problem is that it would be a breaking change -- but maybe that's a change we're willing to make on CQ 2 since it wont affect CQ 1?

bragostin commented 4 years ago

@adam-urbanczyk it works if I use it on both cq_object and pillars:

cq_object = cq_object.faces('<X').workplane(centerOption='ProjectedOrigin').pushPoints(yzsh).polygon(12, dh).cutBlind(-5)
pillars = pillars.faces('<X').workplane(centerOption='ProjectedOrigin').pushPoints(yzsh).hole(dh)

so problem solved, thank you very much!

@dcowden I am a relatively new user of Cadquery still and for me the center of mass was not intuitive, I was thinking in terms of global coordinates in this context because I make a compound object with cq_object and pillars after this operation. However there might be other contexts when the center of mass is more useful.

holes_pos_OK
adam-urbanczyk commented 4 years ago

@dcowden @bragostin there are indeed many cases when center of mass approach is desirable. I'm curious what other users think.

michaelgale commented 4 years ago

As a long time CQ user, I must admit the workplane offset defaulting to centre of mass was more problematic since I'm often lofting between shapes with a lot of asymmetry. It is often unpredictable where the CoM will truly reside and therefore a more deterministic/predictable workplane centre would be preferred, e.g. global coordinates (0, 0). I am often inserting "center(0, 0)" after workplane references and it would be nice to unclutter some code without it.

BTW, kudos to @adam-urbanczyk for being a great custodian of this hugely useful package. My company has made many precision injection molded and pressed metal components with high levels of complexity using CQ. The ability to script our CAD pipeline is truly invaluable. The migration to the OCCT kernel to 7.x cannot come soon enough! I am definitely finding that I have to "hack" workarounds when compositing complex assemblies (crashing OCCT) I usually resort to importing intermediate shapes into FreeCAD for final merging/cleaning shapes--it would be awesome to keep everything in the CQ pipeline instead. If you need any help--I would be happy to offer--although I imagine the initial learning curve of building OCCT bindings will be non-trivial ;)

michaelgale commented 4 years ago

As a follow up, I understand how the CoM might be convenient for shape construction from relatively simple shapes with strong symmetry. However, I often build complex shapes from very particular cross-sections with little to no symmetry. The example below shows what we build with CQ. The strength of CQ is maintaining a clean hierarchy of building complex items from progressively simpler primitives.
CQExample

adam-urbanczyk commented 4 years ago

@michaelgale cool model, would you mind sharing some more screenshots for the README?

Regarding the issue: let's wait a little bit more for more feedback (@jmwright do you have any thoughts) and change the default then.

dcowden commented 4 years ago

@michaelgale i agree very cool model. It's also AMAZING to hear you're getting value out of CQ in the real world. The dream of CQ is a world where everyone can share CAD features and models as easily as sharing source code with github. High fidelity, easily changed models, with the difference engine as simple as looking at the source.

tolot27 commented 4 years ago

Often I run into the same issue with the CoM and every time I debug my code until I get the point that CoM was the cause. I have to remember it or have to specify centerOption='ProjectedOrigin' from the beginning. I don't know whether it is caused by less experience or more or less how I build the models in my brain. ;-)

jmwright commented 4 years ago

@adam-urbanczyk We probably get 5 or more questions each year that would be avoided if centerOption defaulted to ProjectedOrigin. I'd be in favor of changing it in CQ 2.0.

dcowden commented 4 years ago

@jmwright @adam-urbanczyk +1

adam-urbanczyk commented 4 years ago

Vox populi, vox Dei. Let's change it.

michaelgale commented 4 years ago

@adam-urbanczyk In addition to screenshots, I've compiled photos and diagrams of our CadQuery workflows and written a few notes. I've put them into this public repo: https://github.com/fx-bricks/fx-cad-notes Feel free to copy and/or link as desired into the CQ Readme, docs, etc. Many thanks again to you and @dcowden @jmwright et. al. for building this awesome tool!

jmwright commented 4 years ago

@michaelgale That's an awesome resource, thanks for sharing!

tolot27 commented 4 years ago

Vox populi, vox Dei. Let's change it.

That will break backward compatibility. Is it possible to detect code from former CQ version and rise a warning?

tolot27 commented 4 years ago

@michaelgale Did you consider cqparts and if yes why did you decide against it?

michaelgale commented 4 years ago

@tolot27 I guess the short answer is that I discovered cqparts too long after I had dug deep into our own equivalent implementation. Having said that, I think we would have quickly outgrown cqparts; in particular, needing "mating" functionality beyond simple adjacency and offsets. We often need to compute relative positioning dynamically and using rules which involve tangents, spline intersections, intersections with other solids, etc. etc. I really like the cqparts concept and it is a natural layer to use on top of CadQuery. I'm sure it works great for a lot of applications--just not ours.

adam-urbanczyk commented 4 years ago

@tolot27 I don't think it is possible. I'd like to change this behaviour in the 2.0 release and add a warning. In 2.1 we would remove the warning.

Maybe we could support a configuration module, but I'd personally rather not do it.

jmwright commented 3 years ago

Closing since this change to the centerOption was deprecated for awhile and ProjectedOrigin was then made the default in the 2.1 stable release.