dcowden / cadquery

CadQuery-- a parametric cad script framework
Other
432 stars 56 forks source link

Workplane orientation breaks when "Locating workplane on a vertex". #289

Closed sander76 closed 5 years ago

sander76 commented 6 years ago

Comparable to the example displayed at http://dcowden.github.io/cadquery/examples.html#locating-a-workplane-on-a-vertex I am trying to draw a cilinder on a cube with the center located at one of the cube's vertices. However when I do that the workplane changes orientation unexpectedly.

Two examples below.

  1. with cilinder on cube's face at the center
  2. identical to above, but now trying to put it at one of the vertices of the cube.

Example 1

import cadquery as cq

result = cq.Workplane("XY").rect(10.0,10.0).extrude(10)

# Draws a cilinder on the center of the XZ face of the cube
result = result.faces("<Y").workplane().circle(2.0).extrude(10)

show_object(result)

image

Example Two

import cadquery as cq

result = cq.Workplane("XY").rect(10.0,10.0).extrude(10)

# Should draw the cilinder on the same face as above but with its center placed
# at the lower XZ coordinate of that face
# But instead the workplane orientation changes too.
result = result.faces("<Y").vertices("<XZ").workplane().circle(2.0).extrude(10)

show_object(result)

image

dcowden commented 6 years ago

Thanks for reporting this. I can see how you would expect the behavior to be as you described, but it isn't currently designed to work this way.

When you select a face, you have not implicitly selected a workplane. The workplane method is choosing an indeterminate direction because it is not coded to look backwards in the stack for face selections made earlier.

Right now the way to do this is to first establish a workplane on a face, and then shift its origin. That does not allow you to utilize the vertices, though, so I can see why you're wanting it to work this way.

There are two solutions we could consider. The simplest is to allow the syntax toy expected, by having the workplane method look backwards in the stack for a face selection, and center the wp on the selected vertex. We would also have to handle what happens if you attempt to use a vertex without first choosing a face.

The more complex and probably better solution is to simply select the face as the workplane, but then make it possible to project geometry from selections on the object into the workplane.

The second option is much more flexible. The logic to select the center of the wp on a Vertex is a round about way to conveniently calculate a single point on the object in the new workplane, but it only let's you do a single point. It would be much better to be able to project any point, or even an edge, into the workplane.

sander76 commented 6 years ago

@dcowden I am not sure I understand.

Does it mean the example http://dcowden.github.io/cadquery/examples.html#locating-a-workplane-on-a-vertex is incorrect ?

dcowden commented 6 years ago

@sander76 My apologies, you are right! this should work. I forgot we had contemplated this case already.

In the case a vertex is selected, the plane normal will be that of the current workplane's normal, from this line: https://github.com/dcowden/cadquery/blob/master/cadquery/cq.py#L370

This code is expecting that the current CQ object's plane has been oriented to the face already, but i think that's not the case.

When a workplane is created from a face, the normal is computed at this line: https://github.com/dcowden/cadquery/blob/master/cadquery/cq.py#L362

I think the problem here is that faces() by itself doesnt create a workplane-- so the above is not called.

I suspect this may work:

result = result.faces("<Y").workplane().vertices("<XZ").workplane().circle(2.0).extrude(10)

But even if it does, the documentation clearly indicates that the extra workplane() shouldnt be necessary here. I think the right solution is to add code something like this around line 370:


if  isinstance(self.parent, Face):
      normal = self.parent.plane.zDir
      xDir = self.parent.plane.xDir
else:
     normal = self.plane.zDir
     xDir = self.plane.xDir

This would ensure that the orientation for a face is honored correctly.

I've got a lot of projects going on, so I'm not sure when we could get to this. We'd happily accept a PR if you'd be interested in contributing!

sander76 commented 6 years ago

@dcowden Thanks for diving into this. As much as I'd like to I am not able to contribute. Like you, a bit too much other stuff going on. Thanks anyway !

dcowden commented 6 years ago

No problem! I will label this as a good starter issue in case Anyone else is interested in helping

dcowden commented 5 years ago

This has been closed in favor of the CQ 2.0 implemention at https://github.com/cadquery/cadquery