Open GlennWSo opened 1 year ago
@jimy-byerley this pr is ready for ur input
draft by groups, for adding draft on extruded(Mesh) draft by slice, for adding draft on any extrusion draft by axis, for adding draft any Mesh using Axis as definition of neutral plane draft extrusion, lazy for creating extrusion and adding a draft.
u can test on the darft funcs by running:
pytest tests/test_drafts.py
to plot all drafts:
pytest tests/test_drafts.py -v
draft extrusion, lazy for creating extrusion and adding a draft.
- maybe an optimal argument should be added extrusion instead? def extrusion(trans, line: Web, alignment:float=0, draft_angle=0) -> Mesh:
- should handling self-intersection be a separate pr?
Hello I have few concerns about the functions here:
The draft is made as a modification of an already generated mesh (the result of an extrusion()
) which can lead to different issues:
I discourage the creation of an API that creates a mesh, then operate modifications on already present content like in the following example. This is because such API is less convenient to combine (you cannot make a big python expression calling all those functions), plus it often leads to function that ask the user to know about what geometry is already in the mesh
# weird
some = generate(...)
modify_inplace(some)
return some
# prefer this
return modified_clone(generate(...))
# or this if modified cannot rely on generate, or for performance reasons
return generate_modified(...)
In this matter, chamfer()
and bevel()
are specific cases where I couldn't find a way to do things better than modifying the input mesh and asking the user to provide indices in it .. that's because chamfer()
and bevel()
really adjusts to their cutted place ... at least these input indices are easy to provide using the groups mechanisms.
draft_by_groups()
needs the user to know the groups in the mesh (definitely doable, but a bit demanding, the user will have to search the documentation and use Mesh.qualified_groups()
, making it less easy to call draft_by_groups()
)
draft_by_groups()
needs the input mesh to contain an extrusion of a loop profile, else there won't be a free
and a fixed
group. This leads to a situation where draft_by_group()
represent a very specific case and we prefer generic functions over specific.
draft_by_slice()
needs a specific order of the points in the buffer of points, which is not good since the way functions generating meshes choose to organize points in their output buffers is an implementation detail. it also needs all the points in the buffer to belong to this mesh (which is not always the case since points buffer are generally shared accross meshes to save memory and copies). One function having a mesh as input should not make assumptions on the points ordering.
draft_by_slice()
needs the user to provide an index in the buffer of points, this is very demanding since the madcad API tends to make abstraction of the mesh resolution, and we promote to do so in the user's code. using the exact same script but simply changing madcad.settings.primitives['curve_resolution']
can totally change the number of points and thus indices
draft_extusion()
makes assumption that the points buffer returned by extrusion()
is composed of: first half original points, second half moved points. Which in practice is true, but it is an implementation detail of extrusion()
and this may change so we shouldn't rely in that fact.
draft_by_axis()
does not seems to handle the case of curved surfaces, where moving the points to draft can break the geometry
I like however the draft_extrusion()
function: easy and simple signature. with no inputs modification. In my opînion, drafts should be like regular madcad generations functions: taking a wire or a surface and returning a mesh.
If I am correct, draft_by_axis()
is meant for any mesh, to add drafts where necessary. I like this, but I think this should be made more robust to handle the general case with curved surfaces, weird and tight geometries and so.
I propose the following:
draft_extrusion()
implement its own surface generation rather than using extrusion()
draft_extrusion()
to draftside(web/mesh, direction: vec3, angle: float) -> mesh
draft_by_axis()
to drafts(mesh, axis:Axis, angle:float) -> mesh
adding draft to a mesh, each place necessary, and handling self-intersectionSorry for the very long answer. I think your efforts deserved some long explanations of my thinkings
I forgot a minor point:
the syntax type | type
in annotations is not supported before python 3.10, so we should not use it in madcad uintil we stop the support for 3.8 and 3.9 ;)
I usually just put no annotation when multiple types are accepted and just mention it in the docstring
I forgot a minor point: the syntax
type | type
in annotations is not supported before python 3.10, so we should not use it in madcad uintil we stop the support for 3.8 and 3.9 ;) I usually just put no annotation when multiple types are accepted and just mention it in the docstring
oh, i wasn't aware that was new feature of python, ill fix it
draft_extusion() makes assumption that the points buffer returned by extrusion() is composed of: first half original points, second half moved points. Which in practice is true, but it is an implementation detail of extrusion() and this may change so we shouldn't rely in that fact.
Yeah I agree for a function to depend on "knowing" the implementation details(detailed dependency) of another this is bad design. And implementing the extrusion internally for draft_extusion resolves detailed dependency , and presents more optimizations opportunity.
What do think of this alternative solution: Add draft angle as argument to the existing function: extrusion. So it new signature becomes:
rename draft_by_axis() to drafts(mesh, axis:Axis, angle:float) -> mesh adding draft to a mesh, each place necessary, and handling self-intersection
Ill add some more tests cases of "weird" meshes ;) and take a shoot at "handling self-intersection"
update:
I have not done " handle self intersect" yet But still think the changes are useful, in fact i use them in production. Ill gladly tackle self intersection but in a separate pr.
handle dirty/unfinished mesh. Right now, the draft with neutral plane function dont work on meshes that mutated by the show method. I need to investigate why it fails in this case. i think goal is, "draft with a neutral plane" should work even if mesh is non-manifold in various ways
TODO
handle dirty/unfinished mesh. Right now, the draft with neutral plane function dont work on meshes that mutated by the show method. I need to investigate why it fails in this case. i think goal is, "draft with a neutral plane" should work even if mesh is non-manifold in various ways
The reason why draft failed in this case was that show([mesh]) mutated the mesh in way that extended mesh with points that where not used by any face. When i first wrote the function i did not consider the case of dirty meshes.
I have now added a test case for the draft function with dirty meshes.
the draft function can now deal with dirty meshes :) I think the pr is ready now
Also for the consistence of the API (and user convenience):
draft_edge()
should handle the groups from its input Webdraft()
should return a new Mesh instead of modifying inplace its input. The new mesh can share some of its buffers with the input one, but as most points are modified it makes more sense to clone the input buffer of points and then make the modifications.I'm trying to figure what exactly does the current implementation of draft()
it seems to be dilating the mesh in some way ? :thinking:
Given a sphere (which usually in molding only needs draft near its half), it returns the green part
@jimy-byerley
I'm trying to figure what exactly does the current implementation of
draft()
it seems to be dilating the mesh in some way ? thinking Given a sphere (which usually in molding only needs draft near its half), it returns the green part
what behavior do want when applying draft on sphere?
I dont not consider warping effect a bugg, its a feature. Consider the case: Cube with bevels.
If you test drafting with neutral plane in some proprietary CAD software like SolidWorks or CATIA on part that have fillets. The fillets become varible radius fillets in a similar way.
As a mechanical engineer i consider it bad practice to add drafts on beveled geometry. In most cases it makes more sense to add the bevel after the draft. But its nice to if both options are available.
i think i understand what behavior u want for the sphere now. Something like a chamfer but in way that caps the draft angles
...Given a sphere (which usually in molding only needs draft near its half) ... ill create seperate functions for this behavior: draft_split it will behave like a cut and i think it will give u the functionality that your are looking for when casting spheres
draft_cuts is perhaps a better name?
My thoughts about drafts on a sphere is something like this:
given a mesh sphere, a neutral direction and a draft angle, the function will expand the surface where the original mesh has a smaller angle to the neutral direction than the given angle alpha
. As you said: something that caps the draft angles
There is a self-intersection problem between the expanded flanges, because there is flanges drafting from the bottom and flanges drafting from the top. In more complex situations, this leads to the following:
The complex cases has to be handled if we are about to add a generic draft(mesh, direction, angle)
function in the library, because no function we propose must work only for a subset of its arguments. So this is difficult to acheive but definitely desirable for the library.
draft_cuts is perhaps a better name?
Not sure, as such this function will be a very general function, maybe a general name like draft(mesh, direction, angle)
is better 🤔
What I was thinking about when I mentioned draft cut was similar to ur drawings but instead of adding material to cap drafts I was thinking remove material.
So add_drafts we could call the behaviour u decribed. cut_drafts would be similar. But it would not be tangent but it can instead persevere the outline at the neutral plane
The draft() implementation I wrote behaves like a drafty offset. So maybe we should call it something like that. Perhaps draft_transform
So add_drafts we could call the behaviour u decribed. cut_drafts would be similar. But it would not be tangent but it can instead persevere the outline at the neutral plane
+1 on that
So maybe we should call it something like that. Perhaps draft_transform
perhaps yes
drafts on some sides
This is straightforward to do with mutation
cube = cad.brick(width=vec3(1, 2, 1))
draft(cube.group({0, 1}), z_axis, 10, inplace=True)
show([cube])
I propose that draft_transform have the fallowing signature:
def draft_transform(mesh: Mesh, axis: Axis, angle: float, inplace=False) -> Mesh:
"""
Offsets faces in mesh to set draft angle of orthogonal faces.
Parallel faces are not effected. The adjustment of intermediate faces are interpolated.
This funcion perserves topology and suitable for adding drafts on mesh with bevels.
"""
...
inplace toggles mutation
relates to: #64
added import helper