Giter VIP home page Giter VIP logo

Comments (52)

jmwright avatar jmwright commented on August 23, 2024

I'm starting to tinker with this a little bit, so if anyone else takes on this feature request please coordinate with me.

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

I've roughed in the revolve operation, but it's not working properly yet. It seems like there might be something strange going on with the way I'm handling the wires.

https://github.com/jmwright/cadquery

You can paste the following lines into FreeCAD's Python console to try the revolution example that I added.

import sys
sys.path.append('/home/user/Documents/cadquery/examples/FreeCAD')
import Ex024_Revolution

The trick is that I've hard coded the position and axis vectors for now while I'm trying to get this feature working. If you want to change how the revolve operation works beyond just the angle, you'll need to change that code directly.

BTW - The following line will let you install/upgrade a local package (like this one after you clone it) using pip.

pip install --upgrade -e /path/to/package/

The files that I changed or added are:
cadquery/CQ.py
cadquery/freecad_impl/shapes.py
cadquery/examples/FreeCAD/Ex024_Revolution.py

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

When I use CQ wires in a revolve operation that should result in a cylinder, I get the following solid.

wild_cq_solid

When I substitute native FreeCAD wires for the CQ ones I end up with a properly formed cylinder from a revolved rectangular face.

I've simplified the code to where it only deals with one wire (the rectangle) and have tried different variations of the code. I'm having trouble getting around this.

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

can i see your CQ code? i'm sure it something broken in CQ-- but maybe i
can track it down

On Tue, Oct 7, 2014 at 4:31 PM, Jeremy Wright [email protected]
wrote:

When I use CQ wires in a revolve operation that should result in a
cylinder, I get the following solid.

[image: wild_cq_solid]
https://cloud.githubusercontent.com/assets/1015439/4549666/ff77ebfe-4e5f-11e4-9f92-1b4186bc1b52.png

When I substitute native FreeCAD wires for the CQ ones I end up with a
properly formed cylinder from a revolved rectangular face.

I've simplified the code to where it only deals with one wire (the
rectangle) and have tried different variations of the code. I'm having
trouble getting around this.


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

Sure thing. You can grab it off my fork of CQ, the master branch. I had done quite a bit of the revolve work before I realized that I forgot to set up a new branch for it, so that's why it's on master. I don't like adding significant new features like this on master, but that's what I get for not paying attention.

https://github.com/jmwright/cadquery

The code is kind of a mess right now because I've been experimenting. I also haven't created any tests yet. The files you'll be interested in are the following:

examples/FreeCAD/Ex025_Revolution.py
cadquery/CQ.py
cadquery/freecad_impl/shapes.py

Just search for the keyword "revolve".

I used the extrude operation that already exists as my template of how to add the revolve operation.

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

ok i looked at the code. is the code i see the code that breaks or the one
that works. the code looks really good, actually.

is this the line you are referring to in saying you're grabbing the native
wires :

freeCADWires = [outerWire.wrapped]

On Tue, Oct 7, 2014 at 10:47 PM, Jeremy Wright [email protected]
wrote:

Sure thing. You can grab it off my fork of CQ, the master branch. I had
done quite a bit of the revolve work before I realized that I forgot to set
up a new branch for it, so that's why it's on master. I don't like adding
significant new features like this on master, but that's what I get for not
paying attention.

https://github.com/jmwright/cadquery

The code is kind of a mess right now because I've been experimenting. I
also haven't created any tests yet. The files you'll be interested in are
the following:

examples/FreeCAD/Ex025_Revolution.py
cadquery/CQ.py
cadquery/freecad_impl/shapes.py

Just search for the keyword "revolve".

I used the extrude operation that already exists as my template of how to
add the revolve operation.


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

I just verified that the code in my fork is indeed the code that breaks and gives the peanut butter pretzel bites looking solid.

Yes, that line of code you've included is where I'm thinking the native wire(s) come into play. If I replace that line with the following, the solid renders correctly (ends up being a revolved cylinder with the correct proportions).

my_shape = FreeCADPart.Shape([FreeCADPart.Line(FreeCAD.Vector(0,0,0),FreeCAD.Vector(0,15,0)),FreeCADPart.Line(FreeCAD.Vector(0,15,0),FreeCAD.Vector(15,15,0)),FreeCADPart.Line(FreeCAD.Vector(15,15,0),FreeCAD.Vector(15,0,0)),FreeCADPart.Line(FreeCAD.Vector(15,0,0),FreeCAD.Vector(0,0,0))])
freeCADWires = FreeCADPart.Wire(my_shape.Edges)

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

I just committed the two native-wire lines above. They're in shapes.py. The lines are commented out, but if you uncomment them and comment the outerWire.wrapped line you can try this experiment out for yourself.

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

ok perfect. yeah that's weird. Not sure whats going on with the wires and
how they are different....

On Wed, Oct 8, 2014 at 9:05 AM, Jeremy Wright [email protected]
wrote:

I just committed the two native-wire lines above. They're in shapes.py.
The lines are commented out, but if you uncomment them and comment the
outerWire.wrapped line you can try this experiment out for yourself.


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

So, I tried an experiment. I first created a rectangle wire with CQ, and then created one with FreeCAD and displayed them both.

The CQ code:

import Part
cqWire = cadquery.Workplane("front").rect(15, 15)
Part.show(cqWire.val().wrapped)

The FreeCAD code:

fcShape = Part.Shape([Part.Line(FreeCAD.Vector(0,0,0),FreeCAD.Vector(0,15,0)),Part.Line(FreeCAD.Vector(0,15,0),FreeCAD.Vector(15,15,0)),Part.Line(FreeCAD.Vector(15,15,0),FreeCAD.Vector(15,0,0)),Part.Line(FreeCAD.Vector(15,0,0),FreeCAD.Vector(0,0,0))])
fcWire = Part.Wire(fcShape.Edges)
Part.show(fcWire)

You can see the result below. The FreeCAD rectangle is the right-most one, and was created in that position. I didn't move it.

cq_and_freecad_rectangles

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

It just occurred to me that screenshot might be more useful with the axes.

cq_and_freecad_rectangles_with_axes

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

Got it. All you have to do is set centered to false when you create the rectangle and the revolve operation works correctly. Now the question becomes, how do we handle this when a user wants to revolve something? Do we simply put in the documentation that the wire(s) you're revolving can't be centered, or do we enforce it through the code?

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

ah great sleuthing!

Remember we were having a conversation about how BREP packages can create
'weirdness' thats hard to deal with? this is one of those things.

Technically, centered=false should not be necessary. In the case that it
is not, you are attempting to create a solid by rotating the square around
the z axis-- which still results in a valid solid ( and, in fact, still a
cylinder-- but with diameter equal to the width of the square, not 2x the
width.

The issue, probably, is that FreeCAD is getting confused because when you
rotate a face around its center by 360 degrees, you create an overlapping
solid.

I don't have a test envirionment set up yet-- but can you try using the CQ
code , and with centered = True, but try angleDegrees=180 instead of 360?
In theory that should produce a cylinder as well.

On Wed, Oct 8, 2014 at 11:20 AM, Jeremy Wright [email protected]
wrote:

Got it. All you have to do is set centered to false when you create the
rectangle and the revolve operation works correctly. Now the question
becomes, how do we handle this when a user wants to revolve something? Do
we simply put in the documentation that the wire(s) you're revolving can't
be centered, or do we enforce it through the code?


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

So, it sounds like I should make a note in the docstring that centered has to be set to false to work around an issue with FreeCAD and leave it at that?

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

no, perhaps not... Can you try to use centered=True, and then try
angleDegrees=180 first. That might work-- it should produce a valid solid.
I think FreeCAD is trying to be very fancy in correcting what happens when
you try to rotate a face enough that it starts to overlap itself.

So for example i'll bet with centered=False, if you put angleDegrees=720
you would get a similar result as you got with centered=True and
angledegress=360. just guessing though!

On Wed, Oct 8, 2014 at 12:56 PM, Jeremy Wright [email protected]
wrote:

So, it sounds like I should make a note in the docstring that centered has
to be set to false to work around an issue with FreeCAD and leave it at
that?


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

Here's centered=True, 180 degree angle. There's part that looks like a 180 degree revolution, but then there's that weird shell thing the rest of the way around.

centered_true_180_revolve

And here's centered=False, 270 degrees angle.

centered_false_270_revolve

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

interesting. so basicaly yeah FreeCAD doesnt know how to properly sew
together the surfaces in the case that a face is revolved about a center
that is within the face.

Technically, it doesnt mean that centered must be False when you create the
wire. it means that the axis of rotation must not lie inside of the created
face. So for example, if you have a 10x10 square face, and you use
centered=False, but then you were to specify the axis of rotation as
Base.Vector(-30,-30,0) -> Base.Vector(-30,-30,30), it would work just fine.

On Wed, Oct 8, 2014 at 1:12 PM, Jeremy Wright [email protected]
wrote:

Here's centered=True, 180 degree angle. There's part that looks like a 180
degree revolution, but then there's that weird shell thing the rest of the
way around.

[image: centered_true_180_revolve]
https://cloud.githubusercontent.com/assets/1015439/4563382/ac36038c-4f0d-11e4-8ae8-2b805ca8eeac.png

And here's centered=False, 270 degrees angle.

[image: centered_false_270_revolve]
https://cloud.githubusercontent.com/assets/1015439/4563430/17c4906e-4f0e-11e4-9368-a5aaeda213e7.png


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

So, are you saying that if I do the following, a proper solid should result?

Ex25_Revolution.py

cadquery.Workplane("front").rect(10, 10).revolve(360)

shapes.py

f.revolve(FreeCAD.Base.Vector(-30,-30,0), FreeCAD.Base.Vector(-30,-30,30), angleDegrees)

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

well wait what plane is "front" ? is that x-y?

basically you want the two vectors to draw a line that defines the axis of
rotation, and you want the axis of rotation to be such that the shape drawn
does not self intersect.

On Wed, Oct 8, 2014 at 3:30 PM, Jeremy Wright [email protected]
wrote:

So, are you saying that if I do the following, a proper solid should
result?

Ex25_Revolution.py

cadquery.Workplane("front").rect(10, 10).revolve(360)

shapes.py

f.revolve(FreeCAD.Base.Vector(-30,-30,0), FreeCAD.Base.Vector(-30,-30,30), angleDegrees)


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

Correct. I've changed it from "front" to "XY" in the example to make that apparent.

The example I looked at to see how FreeCAD's revolve function works is here, and it says that the first vector is the center and the second is the axis of rotation. That's what you're referring two as the two vectors, correct? The following is what I get when I appply the vectors from above with or without centered being false.

hoop_with_vector_usage

I'm not used to having to think in vectors like this and I'm getting myself confused. I think I'll have to draw it out.

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

well, actually no. i'm a bit confused by the two axis thing too.

the trouble is that Base.Vector is really just a 3-tuple. it can mean a
point in space or a vector. they use the same class to do both. so
Base.Vector(0,0,1) could mean the point x=0,y=0,z=1, or it could mean the
direction vector 0,0,1 ( the z direction).

if their first vector is the 'center', they must be using it as a point.
the second would be a vector

I think the idea is that you start with the 'center point'-- and then
imagine extending a vector( the 2nd argument) from that point, and drawing
a line. then, you revolve the face around that axis.

On Wed, Oct 8, 2014 at 3:52 PM, Jeremy Wright [email protected]
wrote:

Correct. I've changed it from "front" to "XY" in the example to make that
apparent.

The example I looked at to see how FreeCAD's revolve function works is
here http://www.cardanco.com/blog/basic-designs-with-freecad-python.html,
and it says that the first vector is the center and the second is the axis
of rotation. That's what you're referring two as the two vectors, correct?
The following is what I get when I appply the vectors from above with or
without centered being false.

[image: hoop_with_vector_usage]
https://cloud.githubusercontent.com/assets/1015439/4565939/2b8a97d6-4f24-11e4-9bb3-b122e1db0d60.png

I'm not used to having to think in vectors like this and I'm getting
myself confused. I think I'll have to draw it out.


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

Following that line of thinking I changed the revolve call to the following, with centered at the default of true on the rectangle.

result = f.revolve(FreeCAD.Base.Vector(-5,-5,0), FreeCAD.Base.Vector(0,1,0), angleDegrees)

Which I think would set the center of rotation at the bottom left (looking straight down the Z into the XY plane), and use the Y axis as the axis of rotation.

If that makes sense, I guess the next thing to do would be to figure out how to handle that properly in the most CadQuery-esque way possible.

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

yeah true! well, thinking out loud:

  • in cq we have a 2-d workplane already, so it seems reasonable to assume
    that the axis of rotation is in the same plane we're working in. But then
    again someone might want to use a different axis in 3-D

I think the resolve interface CQ exposes can just ask for two 3-tuples--
they define the beginning and end of the rotation axis.

we could further assume that the first argument defaults to the workplane
origin, and the other point defaults to a point along the workplane normal

On Wed, Oct 8, 2014 at 4:09 PM, Jeremy Wright [email protected]
wrote:

Following that line of thinking I changed the revolve call to the
following, with centered at the default of true on the rectangle.

result = f.revolve(FreeCAD.Base.Vector(-5,-5,0), FreeCAD.Base.Vector(0,1,0), angleDegrees)

Which I think would set the center of rotation at the bottom left (looking
straight down the Z into the XY plane), and use the Y axis as the axis of
rotation.

If that makes sense, I guess the next thing to do would be to figure out
how to handle that properly in the most CadQuery-esque way possible.


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

Would the function definition look like this then?

def _revolve(self, angleDegrees, axisStart=None, axisEnd=None):

Then if the user doesn't supply the axisStart and/or axisEnd parameters, we check for None and then use self to get the workplane to define the origin and a point along the normal. Is there a more elegant way to do this?

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

That's the best I can think of. Usually I would prefer to avoid none, but
in this case there are no constant values that make sense.

You might want to default angledegrees to 360, and also consider using %
360 so that you avoid self overlapping geometry if someone passes in values
bigger than 360.
On Oct 8, 2014 10:24 PM, "Jeremy Wright" [email protected] wrote:

Would the function definition look like this then?

def _revolve(self, angleDegrees, axisStart=None, axisEnd=None):

Then if the user doesn't supply the axisStart and/or axisEnd parameters,
we check for None and then use self to get the workplane to define the
origin and a point along the normal. Is there a more elegant way to do this?


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

Sorry, I'm not familiar with using % to set a max value of a function parameter. Would the definition look something like this?

def _revolve(self, angleDegrees=% 360.0, axisStart=None, axisEnd=None):

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

oh, no sorry. i meant you'd do this:

def _revolve(self, angleDegrees=360, axisStart=None, axisEnd=None):

realAngle = angleDegrees % 360

#now if the user did some multiple of 360 or somethign, it doesnt

result in oddness

On Thu, Oct 9, 2014 at 11:28 AM, Jeremy Wright [email protected]
wrote:

Sorry, I'm not familiar with using % to set a max value of a function
parameter. Would the definition look something like this?

def _revolve(self, angleDegrees=% 360.0, axisStart=None, axisEnd=None):


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

I'm modifying the revolve example and then I'll work back from there. It's backwards from how it probably should be done, but I'm working from the end user's perspective inward. Here are the example revolve calls I have set up, assuming the rectangle width and length are both 10.

result = cadquery.Workplane("XY").rect(rectangle_width, rectange_length).revolve()
#result = cadquery.Workplane("XY").rect(rectangle_width, rectange_length).revolve(angleDegrees)
#result = cadquery.Workplane("XY").rect(rectangle_width, rectange_length).revolve(angleDegrees,(-5,-5,0))
#result = cadquery.Workplane("XY").rect(rectangle_width, rectange_length).revolve(angleDegrees,(-5,-5,0),(-5,5,0))
#result = cadquery.Workplane("XY").rect(rectangle_width, rectange_length).revolve(angleDegrees,(0,0,0),(0,5,0),False)

Here's the new revolve function definition.

def revolve(self,angleDegrees=360,axisStart=None,axisEnd=None,combine=True):

Does this all look right?

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

yep looks perfect!

On Thu, Oct 9, 2014 at 10:52 PM, Jeremy Wright [email protected]
wrote:

I'm modifying the revolve example and then I'll work back from there. It's
backwards from how it probably should be done, but I'm working from the end
user's perspective inward. Here are the example revolve calls I have set
up, assuming the rectangle width and length are both 10.

result = cadquery.Workplane("XY").rect(rectangle_width, rectange_length).revolve()#result = cadquery.Workplane("XY").rect(rectangle_width, rectange_length).revolve(angleDegrees)#result = cadquery.Workplane("XY").rect(rectangle_width, rectange_length).revolve(angleDegrees,(-5,-5,0))#result = cadquery.Workplane("XY").rect(rectangle_width, rectange_length).revolve(angleDegrees,(-5,-5,0),(-5,5,0))#result = cadquery.Workplane("XY").rect(rectangle_width, rectange_length).revolve(angleDegrees,(0,0,0),(0,5,0),False)

Here's the new revolve function definition.

def revolve(self,angleDegrees=360,axisStart=None,axisEnd=None,combine=True):

Does this all look right?


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

I started working backwards through the implementation and have a question about the following code.

#The default start point of the vector defining the axis of rotation will be the origin of the workplane
if axisStart is None:
    axisStart = self.plane.origin.toTuple()

#The default end point of the vector defining the axis of rotation should be along the normal from the plane
if axisEnd is None:
    pass
    #I would be able to call normalAt on a Face, so do I need to define a face on this plane?
    #axisEnd = self.plane.normal.toTuple() #Does it seem logical that this should work?

I'm not sure if I did the axisStart code correctly, but it seemed logical. The next thing that seemed logical was being able to grab the normal directly from the plane, especially since you specify the normal when instantiating a Plane. However, that's not an option. It seems like I can only grab the normal from a face (based on a quick look at the source). Should I be grabbing the normal another way?

One other thing that occurred to me just now is that for an XY plane, the normal will be in the Z direction, correct? Do we want that in this case? It seems that if we're defining the axis of rotation, it should be like the hinge pin that the face revolves around. If our face is in the XY plane and we're revolving around the Y axis, than the/a Y axis vector would basically be the hinge pin. This concept worked when I hard coded the axis of rotation in the following FreeCAD implementation.

result = f.revolve(FreeCAD.Base.Vector(-5,-5,0), FreeCAD.Base.Vector(0,1,0), angleDegrees)

My understanding is that the second vector defines only the Y axis to be the axis of rotation. That makes sense to me intuitively, but I'm not used to thinking in vectors.

So, thoughts?

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

Hi, Jeremy:

Yes, you are right: most users, when drawing in the X-Y plane, would think
about revolving "around" an axis in the X-Y plane. I dont know what i was
thinking, but you are right, having the axis normal to the 'sketch plane'
is dumb-- it won't ever product a solid.

so probably computing the axis in 2-d coorindates of the sketch plane would
work. ...

On Mon, Oct 13, 2014 at 9:55 PM, Jeremy Wright [email protected]
wrote:

I started working backwards through the implementation and have a question
about the following code.

#The default start point of the vector defining the axis of rotation will be the origin of the workplaneif axisStart is None:
axisStart = self.plane.origin.toTuple()
#The default end point of the vector defining the axis of rotation should be along the normal from the planeif axisEnd is None:
pass
#I would be able to call normalAt on a Face, so do I need to define a face on this plane?
#axisEnd = self.plane.normal.toTuple() #Does it seem logical that this should work?

I'm not sure if I did the axisStart code correctly, but it seemed logical.
The next thing that seemed logical was being able to grab the normal
directly from the plane, especially since you specify the normal when
instantiating a Plane. However, that's not an option. It seems like I can
only grab the normal from a face (based on a quick look at the source).
Should I be grabbing the normal another way?

One other thing that occurred to me just now is that for an XY plane, the
normal will be in the Z direction, correct? Do we want that in this case?
It seems that if we're defining the axis of rotation, it should be like the
hinge pin that the face revolves around. If our face is in the XY plane and
we're revolving around the Y axis, than the/a Y axis vector would basically
be the hinge pin. This concept worked when I hard coded the axis of
rotation in the following FreeCAD implementation.

result = f.revolve(FreeCAD.Base.Vector(-5,-5,0), FreeCAD.Base.Vector(0,1,0), angleDegrees)

My understanding is that the second vector defines only the Y axis to be
the axis of rotation. That makes sense to me intuitively, but I'm not used
to thinking in vectors.

So, thoughts?


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

Ok, thanks.

In the case where the user doesn't specify the axisEnd, it seems like we'll have to chose an axis for them before we can compute anything. For instance, if we default to the workplane origin for the axisStart and it's an XZ plane, how do we decide whether to default to the X or Z axis when defining the axisEnd?

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

I suspect nothing we do would work often.

The only idea I can come up with is to assume that most users will revolve
things left to right not top to bottom. So you could probably assume the
axis that runs vertical when viewing the working plane from its normal.

Somewhere in CQ I made an assumption that the X axis points left when its
present..
On Oct 13, 2014 11:01 PM, "Jeremy Wright" [email protected] wrote:

Ok, thanks.

In the case where the user doesn't specify the axisEnd, it seems like
we'll have to chose an axis for them before we can compute anything. For
instance, if we default to the workplane origin for the axisStart and it's
an XZ plane, how do we decide whether to default to the X or Z axis when
defining the axisEnd?


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

I'm looking at the line you gave above:

realAngle = angleDegrees % 360

That line gives 0 for realAngle if angleDegrees is 360 or a multiple of it. The FreeCAD revolve function throws an exception because it doesn't assume that you mean 360 when you give it 0 degrees.

Here's my proposed solution that I'm going to run with unless there's a logic error in it that I'm not aware of.

#Make sure we account for users specifying angles larger than 360 degrees
angleDegrees = angleDegrees % 360

#Compensate for FreeCAD not assuming that a 0 degree revolve means a 360 degree revolve
angleDegrees = 360 if angleDegrees == 0 else angleDegrees

Thoughts?

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

We talked above about defaulting to the origin of the workplane if the user doesn't specify a revolution vector start. Using the following code, the origin of the plane is still in the middle of the rectangle that's being revolved and so the revolve gives the funky solid. I assume this still stems from centered=true when the rectangle is created.

#The default start point of the vector defining the axis of rotation will be the origin of the workplane
    if axisStart is None:
        axisStart = self.plane.origin.toTuple()

It almost seems like I'll need to check for centered=True and then move the axisStart and axisEnd to be at the far "left" of the wire(s) being revolved. Does this seem reasonable?

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

yep i agree. a more elegant implementation might be:

angleDegrees = ( angleDegrees + 360 ) % 360

On Thu, Oct 16, 2014 at 1:09 PM, Jeremy Wright [email protected]
wrote:

I'm looking at the line you gave above:

realAngle = angleDegrees % 360

That line gives 0 for realAngle if angleDegrees is 360 or a multiple of
it. The FreeCAD revolve function throws an exception because it doesn't
assume that you mean 360 when you give it 0 degrees.

Here's my proposed solution that I'm going to run with unless there's a
logic error in it that I'm not aware of.

#Make sure we account for users specifying angles larger than 360 degreesangleDegrees = angleDegrees % 360
#Compensate for FreeCAD not assuming that a 0 degree revolve means a 360 degree revolveangleDegrees = 360 if angleDegrees == 0 else angleDegrees

Thoughts?


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

i still think that checking for centered=True is a mistake. The issue is
that the axis of rotation overlaps with the wire. This can happen even if
centered=False. Example use cases would include specifying an off-center
rotation axis, or, a translated workplane.

it is best to avoid trying to guess what the user is trying to do, and
instead simply to provide predictable semantics. I can think of two
possible semantics that might be reasonable when the axis of rotation is
not specified:

(1) assume the axis lies along the x or y axis through the origin. Yes,
this could result in a funky solid when the user uses a rectagle with
centered=true. but why try to prevent that?

(2) assume that the axis of rotation should be at the far left extent of
the wires drawn on the existing plane. That would involve calculating the
boundaries of the wires on the existing plane--which is harder than it
sounds.

I'd vote for option 1. Simple and easy. If we think that
rectangle().revolve(360) is going to be very common, i think we can just
put in the documentation that this very common case doesnt make much sense.

we can also provide a sample that uses centered=False

On Thu, Oct 16, 2014 at 2:03 PM, Jeremy Wright [email protected]
wrote:

We talked above about defaulting to the origin of the workplane if the
user doesn't specify a revolution vector start. Using the following code,
the origin of the plane is still in the middle of the rectangle that's
being revolved and so the revolve gives the funky solid. I assume this
still stems from centered=true when the rectangle is created.

#The default start point of the vector defining the axis of rotation will be the origin of the workplane
if axisStart is None:
axisStart = self.plane.origin.toTuple()

It almost seems like I'll need to check for centered=True and then move
the axisStart and axisEnd to be at the far "left" of the wire(s) being
revolved. Does this seem reasonable?


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

Thanks Dave.

angleDegrees = ( angleDegrees + 360 ) % 360

I don't think that will work still, because if you add 360 to 360 you get a multiple which still returns 0 when using the modulo.

Option 1 is fine with me, but I roughed out the following an hour or so ago to seek out the left extent. I put this within the FreeCAD implementation because it's working around a FreeCAD (or OCC?) limitation.

f = FreeCADPart.Face(freeCADWires)

#If the user has chosen to revolve about a center, we have to override it because FreeCAD cannot do that
vectorX = axisStart[0]
vectorY = axisStart[1]
vectorZ = axisStart[2]

#Seek out the lower left corner as our origin so that revolve will work correctly
for curVertex in f.Vertexes:
    vectorX = curVertex.X if curVertex.X < vectorX else vectorX
    vectorY = curVertex.Y if curVertex.Y < vectorY else vectorY
    vectorZ = curVertex.Z if curVertex.Z < vectorZ else vectorZ

result = f.revolve(FreeCAD.Base.Vector(vectorX, vectorY, vectorZ),
                   FreeCAD.Base.Vector(axisEnd), angleDegrees)

Even if my logic holds though, this is still likely to result in problems with edge cases later.

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

I changed back to option 1 where CQ respects centered=True. I think the last thing that I'm having trouble with is figuring out which axis is the vertical so I can revolve around that if the user doesn't specify an axis of revolution end point. I've looked at different ways of doing it, and I can't even figure out what plane I'm working in currently (whether it's XY or XZ).

Any ideas?

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

duh right! that seemed like it would work :)

On Thu, Oct 16, 2014 at 4:27 PM, Jeremy Wright [email protected]
wrote:

Thanks Dave.

angleDegrees = ( angleDegrees + 360 ) % 360

I don't think that will work still, because if you add 360 to 360 you get
a multiple which still returns 0 when using the modulo.

Option 1 is fine with me, but I roughed out the following an hour or so
ago to seek out the left extent. I put this within the FreeCAD
implementation because it's working around a FreeCAD (or OCC?) limitation.

f = FreeCADPart.Face(freeCADWires)
#If the user has chosen to revolve about a center, we have to override it because FreeCAD cannot do thatvectorX = axisStart[0]vectorY = axisStart[1]vectorZ = axisStart[2]
#Seek out the lower left corner as our origin so that revolve will work correctlyfor curVertex in f.Vertexes:
vectorX = curVertex.X if curVertex.X < vectorX else vectorX
vectorY = curVertex.Y if curVertex.Y < vectorY else vectorY
vectorZ = curVertex.Z if curVertex.Z < vectorZ else vectorZ
result = f.revolve(FreeCAD.Base.Vector(vectorX, vectorY, vectorZ),
FreeCAD.Base.Vector(axisEnd), angleDegrees)

Even if my logic holds though, this is still likely to result in problems
with edge cases later.


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

each Workplane defines a local, 2d coorindate system, that can be
translated to 3d coordinates through the plane object.

so if you want to use the y-axis of any plane as the axis of rotation, you
can start with two 2-D points (0,0), and (0,1).

Then, if you need to convert those to 3-d coordinates, you use
Workplane.plane.toWorldCoords(..)

so for example, Workplane.plane.toWorldCoords((0,0)) should return the
absolute 3d coordinates of the origin of the current plane, and
Workplane.plane.toWorldCoords((0,1)) should return the world coordinates of
the point (0,1) in the current workplane.

its always a little tricky to define what the inputs of a function like
revolve mean. What might work well is to accept 2-D coordinates instead of
3-d coordinates to your resolve method. that way, its clear to the user
that the axis of rotation is expected to be in the same workplane as the
other wires they will revolve

On Thu, Oct 16, 2014 at 4:46 PM, Jeremy Wright [email protected]
wrote:

I changed back to option 1 where CQ respects centered=True. I think the
last thing that I'm having trouble with is figuring out which axis is the
vertical so I can revolve around that if the user doesn't specify an axis
of revolution end point. I've looked at different ways of doing it, and I
can't even figure out what plane I'm working in currently (whether it's XY
or XZ).

Any ideas?


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

The revolve feature now seems to be working as we've discussed above, but I'm not totally convinced it's handling all cases correctly. I've put different revolve function call variations in the Example025_Revolution.py file, and have made comments on what appears correct and what's suspect.

https://github.com/jmwright/cadquery

The other files of interest are shapes.py and CQ.py.

I can post screenshots of the resulting solids that are suspect if that will help.

Please note that I haven't cleaned up the doc strings or written tests yet. I'm just now getting the base functionality down and would like a sanity check if possible.

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

I just realized that the suspect revolve calls in the example aren't providing FreeCAD with the propper axis vector format. I'll have to figure out how to convert them.

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

My latest commit fixed the issue, but I'm not totally convinced that it's the best fix.

https://github.com/jmwright/cadquery

FreeCAD's revolve function expect the first vector to define the center of rotation, and the second to define the axis.

http://www.cardanco.com/blog/basic-designs-with-freecad-python.html (search for "revolve")

If your axis vector has more than one non-zero axis, I guess it attempts to revolve the geometry around all the non-zero axes simultaneously. So, I decided to attempt to zero out the axes that were the same between the start and end axes (meaning that the CQ user wanted the rotation axis vector end to stay in line with that axis. Here's the code I came up with that I'm not totally sold on.

rotateCenter = FreeCAD.Base.Vector(axisStart)
rotateAxis = FreeCAD.Base.Vector(axisEnd)

#Convert our axis end vector into to something FreeCAD will understand (an axis specification vector)
rotateAxis[0] = 0.0 if axisStart[0] == axisEnd[0] else axisEnd[0]
rotateAxis[1] = 0.0 if axisStart[1] == axisEnd[1] else axisEnd[1]
rotateAxis[2] = 0.0 if axisStart[2] == axisEnd[2] else axisEnd[2]

#FreeCAD wants a rotation center and then an axis to rotate around rather than an axis of rotation
result = f.revolve(rotateCenter, rotateAxis, angleDegrees)

Is there a nice, clean way of doing this? I checked into things like cross and dot products, but they didn't get me there. Is this even the right approach?

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

I've updated the doc strings related to the revolve functions. I'll start on the tests next. Even if the code above is changed, the tests should still be valid since they're testing the output solid from the operation.

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

i think an easy way to do the logic you are describing is simply to
subtract one vector from the other. IE, if your goal is to convert (2,2,3)
--> ( 2, 2, 5 ) to 0,0,5, you can get that by doing v2 - v1. you'll end up
with 0,0,3, not 0,0,5, but since the vector is an axis it doesnt matter.

On Fri, Oct 17, 2014 at 2:15 PM, Jeremy Wright [email protected]
wrote:

My latest commit fixed the issue, but I'm not totally convinced that it's
the best fix.

https://github.com/jmwright/cadquery

FreeCAD's revolve function expect the first vector to define the center of
rotation, and the second to define the axis.

http://www.cardanco.com/blog/basic-designs-with-freecad-python.html
(search for "revolve")

If your axis vector has more than one non-zero axis, I guess it attempts
to revolve the geometry around all the non-zero axes simultaneously. So, I
decided to attempt to zero out the axes that were the same between the
start and end axes (meaning that the CQ user wanted the rotation axis
vector end to stay in line with that axis. Here's the code I came up with
that I'm not totally sold on.

rotateCenter = FreeCAD.Base.Vector(axisStart)rotateAxis = FreeCAD.Base.Vector(axisEnd)
#Convert our axis end vector into to something FreeCAD will understand (an axis specification vector)rotateAxis[0] = 0.0 if axisStart[0] == axisEnd[0] else axisEnd[0]rotateAxis[1] = 0.0 if axisStart[1] == axisEnd[1] else axisEnd[1]rotateAxis[2] = 0.0 if axisStart[2] == axisEnd[2] else axisEnd[2]
#FreeCAD wants a rotation center and then an axis to rotate around rather than an axis of rotationresult = f.revolve(rotateCenter, rotateAxis, angleDegrees)

Is there a nice, clean way of doing this? I checked into things like cross
and dot products, but they didn't get me there. Is this even the right
approach?


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

OK. I tried the subtraction method at first, but I may have gotten confused in thinking that there were edge cases where that wouldn't work. I think that was before I reviewed the documentation on FreeCAD's revolve function though. I'll convert the code to using subtraction. Thanks.

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

I used the sub function to get the axis of rotation as discussed above.

rotateCenter = FreeCAD.Base.Vector(axisStart)
rotateAxis = FreeCAD.Base.Vector(axisEnd)

#Convert our axis end vector into to something FreeCAD will understand (an axis specification vector)
rotateAxis = rotateCenter.sub(rotateAxis)

I did make one adjustment in the code for when a user specifies an axis start, but not the end:

result = cadquery.Workplane("XY").rect(rectangle_width, rectangle_length).revolve(angleDegrees,(-5,-5))

In this case, I figured we'd want to assume an axis of rotation for them. I ended up with this code to handle that case.

 #The default start point of the vector defining the axis of rotation will be the origin of the workplane
if axisStart is None:
    axisStart = self.plane.toWorldCoords((0,0)).toTuple()
else:
    axisStart = self.plane.toWorldCoords(axisStart).toTuple()

#The default end point of the vector defining the axis of rotation should be along the normal from the plane
if axisEnd is None:
    #Make sure we match the user's assumed axis of rotation if they specified an start but not an end
    if axisStart[1] != 0:
        axisEnd = self.plane.toWorldCoords((0,axisStart[1])).toTuple()
    else:
        axisEnd = self.plane.toWorldCoords((0,1)).toTuple()
else:
    axisEnd = self.plane.toWorldCoords(axisEnd).toTuple()

r = self._revolve(angleDegrees, axisStart, axisEnd) # returns a Solid ( or a compound if there were multiple )

It assumes that the second coordinate in the tuple will define the axis of rotation. Is this reasonable?

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

yep seems reasonable to me!

On Sun, Oct 19, 2014 at 10:32 PM, Jeremy Wright [email protected]
wrote:

I used the sub function to get the axis of rotation as discussed above.

rotateCenter = FreeCAD.Base.Vector(axisStart)rotateAxis = FreeCAD.Base.Vector(axisEnd)
#Convert our axis end vector into to something FreeCAD will understand (an axis specification vector)rotateAxis = rotateCenter.sub(rotateAxis)

I did make one adjustment in the code for when a user specifies an axis
start, but not the end:

result = cadquery.Workplane("XY").rect(rectangle_width, rectangle_length).revolve(angleDegrees,(-5,-5))

In this case, I figured we'd want to assume an axis of rotation for them.
I ended up with this code to handle that case.

#The default start point of the vector defining the axis of rotation will be the origin of the workplaneif axisStart is None:
axisStart = self.plane.toWorldCoords((0,0)).toTuple()else:
axisStart = self.plane.toWorldCoords(axisStart).toTuple()
#The default end point of the vector defining the axis of rotation should be along the normal from the planeif axisEnd is None:
#Make sure we match the user's assumed axis of rotation if they specified an start but not an end
if axisStart[1] != 0:
axisEnd = self.plane.toWorldCoords((0,axisStart[1])).toTuple()
else:
axisEnd = self.plane.toWorldCoords((0,1)).toTuple()else:
axisEnd = self.plane.toWorldCoords(axisEnd).toTuple()
r = self._revolve(angleDegrees, axisStart, axisEnd) # returns a Solid ( or a compound if there were multiple )

It assumes that the second coordinate in the tuple will define the axis of
rotation. Is this reasonable?


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

Everything should be good to go on this. I've added several tests and the changes are in pull request #43 . Even if that request is rejected because of an oversight on my part, we're still extremely close to having this working.

I'm tagging @rosenhouse in case he wants to take another look at CadQuery now that a revolve operation has been implemented. If so, please let us know if we can help you get started.

from cadquery.

dcowden avatar dcowden commented on August 23, 2024

man are you kidding its great work! I merged it!

On Mon, Oct 20, 2014 at 10:59 PM, Jeremy Wright [email protected]
wrote:

Everything should be good to go on this. I've added several tests and the
changes are in pull request #43
#43 . Even if that request is
rejected because of an oversight on my part, we're still extremely close to
having this working.

I'm tagging @rosenhouse https://github.com/rosenhouse in case he wants
to take another look at CadQuery now that a revolve operation has been
implemented. If so, please let us know if we can help you get started.


Reply to this email directly or view it on GitHub
#32 (comment).

from cadquery.

jmwright avatar jmwright commented on August 23, 2024

Sweet, thanks! I revolved a fairly complex shape earlier today and this operation worked great. I think we can close this feature request.

from cadquery.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.