-
Notifications
You must be signed in to change notification settings - Fork 56
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
write AST-mod for adding fillet, piped to extrude variable expression #2607
Comments
@Irev-Dev, thank you for the sketch on face tip; it helped a lot to understand the situation. Please see my findings and ideas and check if I am missing anything. We need to create a UI button for filleting, which involves selecting an edge, tagging it, and modifying the AST to include the fillet operation within the Extrude. Data Flow
Main Components
Steps
<ActionButton
onClick={() => send({ type: 'Enter fillet', data: { forceNewFillet: true } })}
>
on: { 'Enter fillet': 'filletMode', },
states: { filletMode: { /* State Logic */ }, }
sendSelectEventToEngine(e, videoRef.current, streamDimensions)
modelingSend({ type: 'Process Fillet', data: edgeSelection })
function applyFilletToEdge(ast, edge) { /* Logic */ } ////////////////// Functionality Discussion
|> fillet({
radius: 5,
// edges
tags: ["seg01”, "seg02”, "seg03"]
// or chained edges
tags: [“chain01”]
// or faces
tags: [“face01”]
const sketch001 = startSketchOn('XZ')
|> startProfileAt([2.16, 49.67], %)
|> line([101.49, 139.93], %)
|> line([60.04, -55.72], %, 'seg01')
|> line([1.29, -115.74], %)
// other segments
|> close(%)
const extrude001 = extrude(50, sketch001)
|> fillet({
radius: 5,
tags: ["seg01(1)”, "seg01(2)”, “seg01(A)”, "seg01(E)”]
}, %) ![]() Or we can skip all that and just make fresh tags for each fillet without referencing the sketch segments.
const fillet001 = fillet(10, [extrude001, extrude002])
|> edge01
|> edge02 UPD: Besides, following common design practices and RSM, it is important to gather all fillet-related operations at the end of the code. I think it is a good practice to follow. |
Nice digging around, I couldn't actually tell you if you found everything or not without digging through it again myself, but it sounds about right. I had thought the steps involved will roughly be the three tasks in #1206, i.e. we could start with just the code mod, write unit test for them etc, Then we look into getting the most basic UI selections working. Then we look into fixing up some of the selections. I think I want to stick with tags, and we are able to use the tags to get to all edges from an extrusion, but we can talk about it because I didn't quiet follow what you were talking about with some of the alternatives. We should talk about gathering the fillets at the end because maybe that will effect the code-mod we want to right. |
For refernce we were talking about
|
@Irev-Dev the mod and test are still in progress, but it would be great if you could check them out: 78fb73a It works with 3 cases now:
const sketch001 = startSketchOn('XZ')
|> startProfileAt([2.16, 49.67], %)
|> line([101.49, 139.93], %)
|> line([60.04, -55.72], %)
|> line([1.29, -115.74], %)
|> line([-87.24, -47.08], %)
|> tangentialArcTo([56.15, -94.58], %)
|> tangentialArcTo([14.68, -104.52], %)
|> lineTo([profileStartX(%), profileStartY(%)], %)
|> close(%)
const extrude001 = extrude(50, sketch001)
const sketch001 = startSketchOn('XZ')
|> startProfileAt([2.16, 49.67], %)
|> line([101.49, 139.93], %)
|> line([60.04, -55.72], %) // selected edge
|> line([1.29, -115.74], %)
|> line([-87.24, -47.08], %, 'seg01') // tagged edge
|> tangentialArcTo([56.15, -94.58], %)
|> tangentialArcTo([14.68, -104.52], %)
|> lineTo([profileStartX(%), profileStartY(%)], %)
|> close(%)
const extrude001 = extrude(50, sketch001)
const sketch001 = startSketchOn('XZ')
|> startProfileAt([2.16, 49.67], %)
|> line([101.49, 139.93], %)
|> line([60.04, -55.72], %)
|> line([1.29, -115.74], %)
|> line([-87.24, -47.08], %, 'seg03') // selected edge
|> tangentialArcTo([56.15, -94.58], %)
|> tangentialArcTo([14.68, -104.52], %)
|> lineTo([profileStartX(%), profileStartY(%)], %)
|> close(%)
const extrude001 = extrude(50, sketch001) I will continue to work on edge cases. |
Hi @Irev-Dev Update: 5ff9239 I've started to cover the cases where the fillet already exists, like here: const sketch001 = startSketchOn('XZ')
|> startProfileAt([2.16, 49.67], %)
|> line([101.49, 139.93], %)
|> line([60.04, -55.72], %,) // selection
|> line([1.29, -115.74], %)
|> line([-87.24, -47.08], %, 'seg03') // existing tag
|> tangentialArcTo([56.15, -94.58], %)
|> tangentialArcTo([14.68, -104.52], %)
|> lineTo([profileStartX(%), profileStartY(%)], %)
|> close(%)
const extrude001 = extrude(50, sketch001)
|> fillet({ radius: 10, tags: ['seg03'] }, %) // existing fillet Terraformed to: const sketch001 = startSketchOn('XZ')
|> startProfileAt([2.16, 49.67], %)
|> line([101.49, 139.93], %)
|> line([60.04, -55.72], %, 'seg01') // new tag
|> line([1.29, -115.74], %)
|> line([-87.24, -47.08], %, 'seg03')
|> tangentialArcTo([56.15, -94.58], %)
|> tangentialArcTo([14.68, -104.52], %)
|> lineTo([profileStartX(%), profileStartY(%)], %)
|> close(%)
const extrude001 = extrude(50, sketch001)
|> fillet({ radius: 10, tags: ['seg03'] }, %)
|> fillet({ radius: 5, tags: ['seg01'] }, %) // new fillet The next case to cover would be: const sketch001 = startSketchOn('XZ')
|> startProfileAt([2.16, 49.67], %)
|> line([101.49, 139.93], %)
|> line([60.04, -55.72], %,)
|> line([1.29, -115.74], %)
|> line([-87.24, -47.08], %, 'seg03') // select line with existing tag
|> tangentialArcTo([56.15, -94.58], %)
|> tangentialArcTo([14.68, -104.52], %)
|> lineTo([profileStartX(%), profileStartY(%)], %)
|> close(%)
const extrude001 = extrude(50, sketch001)
|> fillet({ radius: 10, tags: ['seg03'] }, %) // modify existing fillet |
Fillet the fillet how to tag the edge of the fillet ? I think it would be good to follow the same logic as with the general shatter case, when extruded edge gets shattered after the boolean operation, like this: and treat the fillet like the last part of the shattered edge in this case the code will look like this: for example: // initail edge
getOppositeEdge('seg01', extrude001)
// shattered edge parts:
getOppositeEdgeStart('seg01', extrude001)
getOppositeEdge01('seg01', extrude001)
getOppositeEdge02('seg01', extrude001)
getOppositeEdge03('seg01', extrude001)
getOppositeEdge05('seg01', extrude001)
getOppositeEdgeEnd('seg01', extrude001) |
@Irev-Dev added new edge case, with existing fillet that has to be modified: take this: const sketch001 = startSketchOn('XZ')
|> startProfileAt([2.16, 49.67], %)
|> line([101.49, 139.93], %)
|> line([60.04, -55.72], %,)
|> line([1.29, -115.74], %)
|> line([-87.24, -47.08], %, 'seg03') // select line with existing tag
|> tangentialArcTo([56.15, -94.58], %)
|> tangentialArcTo([14.68, -104.52], %)
|> lineTo([profileStartX(%), profileStartY(%)], %)
|> close(%)
const extrude001 = extrude(50, sketch001)
|> fillet({ radius: 10, tags: ['seg03'] }, %) // modify existing fillet and transform to this: const sketch001 = startSketchOn('XZ')
|> startProfileAt([2.16, 49.67], %)
|> line([101.49, 139.93], %)
|> line([60.04, -55.72], %)
|> line([1.29, -115.74], %)
|> line([-87.24, -47.08], %, 'seg03')
|> tangentialArcTo([56.15, -94.58], %)
|> tangentialArcTo([14.68, -104.52], %)
|> lineTo([profileStartX(%), profileStartY(%)], %)
|> close(%)
const extrude001 = extrude(50, sketch001)
|> fillet({ radius: 5, tags: ['seg03'] }, %) // new radius there are still a lot of other cases, that I've mentioned above |
Right!, another idea would be to keep the convention we have so far, of being able to tag faces and edges in the operation in which they are created, so that if you have But having said that, is there a real need to select this edge? I ask because when you have a straight edge going into a tangential arc, that then follows with another tangential segment, in most CAD can you fillet these separately? I thought they must be all filleted together? in which case using
Absolutely! I might make you branch so far into a PR again, I think it's the best way to leave comments in line. |
See video in #2606 for context but essentially write an ast-mod that takes
And transforms it to
The text was updated successfully, but these errors were encountered: