Skip to content

Do multiple fillets in one API call #6750

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

Merged
merged 3 commits into from
May 22, 2025
Merged

Conversation

adamchalmers
Copy link
Contributor

@adamchalmers adamchalmers commented May 7, 2025

KCL's fillet function takes an array of edges to fillet. Previously this would do n fillet API commands, one per edge. This PR combines them all into one call, which should improve performance. Here's a typical example, from a KCL artifact command snapshot:

Screenshot 2025-05-22 at 4 10 15 PM

Besides performance, this should fix a bug where some KCL fillets would fail, when they should have succeeded. Example from @max-mrgrsk:

sketch001 = startSketchOn(XY)
  |> startProfile(at = [-12, -6])
  |> line(end = [0, 12], tag = $seg04)
  |> line(end = [24, 0], tag = $seg03)
  |> line(end = [0, -12], tag = $seg02)
  |> line(endAbsolute = [profileStartX(%), profileStartY(%)], tag = $seg01)
  |> close()
extrude001 = extrude(
       sketch001,
       length = 12,
       tagEnd = $capEnd001,
       tagStart = $capStart001,
     )
  |> fillet(
       radius = 5,
       tags = [
         getCommonEdge(faces = [seg02, capEnd001]),
         getCommonEdge(faces = [seg01, capEnd001]),
         getCommonEdge(faces = [seg03, capEnd001]),
         getCommonEdge(faces = [seg04, capEnd001])
       ],
     )

This program fails on main, but succeeds on this branch.

@adamchalmers adamchalmers requested review from jtran and jessfraz May 7, 2025 21:27
Copy link

vercel bot commented May 7, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2025 9:25pm

Copy link

qa-wolf bot commented May 7, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

Copy link

codspeed-hq bot commented May 7, 2025

CodSpeed Instrumentation Performance Report

Merging #6750 will not alter performance

Comparing achalmers/fillets-again (8ba45e9) with main (4e2deca)

Summary

✅ 80 untouched benchmarks

@adamchalmers adamchalmers force-pushed the achalmers/fillets-again branch 2 times, most recently from 4d2a55b to f1dbe1b Compare May 22, 2025 20:30
@adamchalmers adamchalmers force-pushed the achalmers/fillets-again branch from f1dbe1b to e4ed7bb Compare May 22, 2025 20:30
@adamchalmers adamchalmers force-pushed the achalmers/fillets-again branch from df4d18a to 8ba45e9 Compare May 22, 2025 21:07
@adamchalmers adamchalmers enabled auto-merge (squash) May 22, 2025 21:14
@adamchalmers adamchalmers merged commit 85ccc69 into main May 22, 2025
74 checks passed
@adamchalmers adamchalmers deleted the achalmers/fillets-again branch May 22, 2025 21:25
@adamchalmers adamchalmers changed the title Do multiple chamfer/fillet in one API call Do multiple fillets in one API call May 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants