Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
dcowden opened this issue Dec 6, 2018 · 6 comments · Fixed by #480
Closed

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

dcowden opened this issue Dec 6, 2018 · 6 comments · Fixed by #480
Labels
bug Something isn't working good first issue Good for newcomers helpwanted

Comments

@dcowden
Copy link
Member

dcowden commented Dec 6, 2018

Issue by sander76
Saturday Aug 11, 2018 at 10:15 GMT
Originally opened as dcowden/cadquery#289


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 dcowden added bug Something isn't working good first issue Good for newcomers helpwanted labels Dec 6, 2018
@dcowden
Copy link
Member Author

dcowden commented Dec 6, 2018

Comment by dcowden
Saturday Aug 11, 2018 at 12:40 GMT


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.

@dcowden
Copy link
Member Author

dcowden commented Dec 6, 2018

Comment by sander76
Saturday Aug 11, 2018 at 12:57 GMT


@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
Copy link
Member Author

dcowden commented Dec 6, 2018

Comment by dcowden
Saturday Aug 11, 2018 at 13:54 GMT


@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:/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:/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!

@dcowden
Copy link
Member Author

dcowden commented Dec 6, 2018

Comment by sander76
Sunday Aug 12, 2018 at 06:57 GMT


@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
Copy link
Member Author

dcowden commented Dec 6, 2018

Comment by dcowden
Sunday Aug 12, 2018 at 11:58 GMT


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

@ArmoredBlood
Copy link
Contributor

I was able to recreate this in the current version. I would like to attempt this fix since I want to start contributing to this project while I learn how to use it for my personal projects. Is there anything else I should do for this PR in relation to #149?

It seems like @dcowden's proposed fix should solve it as well by checking for the parent's normal. The workplane() code has changed slightly so the fix may not be exactly as described and may take me a little longer to figure out the new equivalent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers helpwanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants