Skip to content

Add clone#5462

Merged
jessfraz merged 19 commits intomainfrom
add-clone
Apr 24, 2025
Merged

Add clone#5462
jessfraz merged 19 commits intomainfrom
add-clone

Conversation

@jessfraz
Copy link
Contributor

@jessfraz jessfraz commented Feb 22, 2025

fixes we need:

  • scale, rotate, translate all fail with "No such object exists"
  • sketch on face w same tags work on the clone
  • get the paths ids from getchildrenuuids so that tags are good
  • calling fillet with the new solid/sketch id returns "The provided object is of type Object, which is not a valid input for use in this operation."
  • calling extrude on the new sketch fails with "No such object exists"
  • ensure loft / sweep / revolve are okay too, but hard to do without transforms
  • failed tests for the below so we know when we fix each
  • loft test
  • sweep test
  • revolve test

Still need but think okay to merge without

  • clone a foreign imported model (need @alteous help here) it clones but both of their appearances is changing
  • getExtrusionFaceInfo is not returning any cap ids for the end and start caps of the cloned solid but works for the original
  • need a way to get the edge cut ids, not being return in getAllChildrenUuids of the original and cloned object
  • need a way to get the MovePathPen id, not being return in getAllChildrenUuids of the original and cloned object
  • when calling close() on an already closed model then cloning it we cant get the id for the close to move to the new model

@qa-wolf
Copy link

qa-wolf bot commented Feb 22, 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!

@vercel
Copy link

vercel bot commented Feb 22, 2025

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

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Apr 24, 2025 3:44am

@codecov
Copy link

codecov bot commented Feb 22, 2025

Codecov Report

Attention: Patch coverage is 65.13912% with 213 lines in your changes missing coverage. Please review.

Project coverage is 85.20%. Comparing base (457ab28) to head (68867fc).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
rust/kcl-lib/src/std/clone.rs 66.42% 183 Missing ⚠️
rust/kcl-lib/src/execution/geometry.rs 43.75% 18 Missing ⚠️
rust/kcl-lib/src/std/args.rs 46.15% 7 Missing ⚠️
rust/kcl-lib/src/execution/kcl_value.rs 54.54% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5462      +/-   ##
==========================================
- Coverage   85.42%   85.20%   -0.22%     
==========================================
  Files         108      109       +1     
  Lines       46399    47010     +611     
==========================================
+ Hits        39638    40057     +419     
- Misses       6761     6953     +192     
Flag Coverage Δ
rust 85.20% <65.13%> (-0.22%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 28, 2025

CodSpeed Walltime Performance Report

Merging #5462 will degrade performances by 17.78%

Comparing add-clone (20cd89b) with main (7ca3aff)

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

⚡ 9 improvements
❌ 44 regressions
✅ 39 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
parse_80-20-rail 7.5 ms 8.9 ms -15.42%
parse_a-parametric-bearing-pillow-block 1.8 ms 2.1 ms -14.99%
parse_ball-bearing 3.2 ms 3.8 ms -15.67%
parse_bench 3 ms 3.6 ms -15.86%
parse_bracket 2.3 ms 2.8 ms -16.12%
execute_color-cube 1.4 s 1.7 s -15.79%
parse_color-cube 2.3 ms 2.7 ms -16.7%
parse_cycloidal-gear 3.4 ms 4.1 ms -16.41%
execute_dodecahedron 1.8 s 1.6 s +14.63%
parse_dodecahedron 10.1 ms 12.1 ms -16.74%
parse_dual-basin-utility-sink 12.6 ms 15.2 ms -17%
execute_enclosure 3.3 s 2.9 s +13.25%
parse_enclosure 11 ms 13 ms -15.63%
parse_exhaust-manifold 10.2 ms 12.1 ms -16.03%
parse_flange 1.1 ms 1.3 ms -15.78%
parse_focusrite-scarlett-mounting-bracket 8.3 ms 9.7 ms -14.32%
parse_food-service-spatula 11 ms 12.9 ms -15.17%
execute_french-press 4.5 s 3.9 s +15.96%
parse_french-press 10.4 ms 12.4 ms -16.01%
parse_gear-rack 2.1 ms 2.5 ms -15.05%
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@codspeed-hq
Copy link

codspeed-hq bot commented Apr 24, 2025

CodSpeed Instrumentation Performance Report

Merging #5462 will not alter performance

Comparing add-clone (68867fc) with main (457ab28)

Summary

✅ 54 untouched benchmarks

@jessfraz jessfraz requested a review from jtran April 24, 2025 01:18
jessfraz and others added 9 commits April 23, 2025 18:24
Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

update the extrude idds

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

fix sample

Signed-off-by: Jess Frazelle <github@jessfraz.com>

better docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

fix the start and end tag

Signed-off-by: Jess Frazelle <github@jessfraz.com>

better docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

new tests

Signed-off-by: Jess Frazelle <github@jessfraz.com>

codespell

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Copy link
Contributor

@jtran jtran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering how the cloned IDs will make it into the artifact graph. But I'd rather merge this first and figure it out later.

Co-authored-by: Jonathan Tran <jonnytran@gmail.com>
@jessfraz
Copy link
Contributor Author

oh yeah would be ideal if they returned the mapping on clone versus this getchildid thing i have to do, then maybe we could both use that way? or i could push ot the artifact graph from here after recreating thhe world which might be easier since like its a lot of logic

@jessfraz
Copy link
Contributor Author

OR maybe it just shows up in teh artifact graph as Clone and you dont need all the ids? idk i guess kurt does for clicking shit

Signed-off-by: Jess Frazelle <github@jessfraz.com>
Comment on lines +711 to +712
#[ignore = "this test is not working yet, need to fix the getting of ids if sketch already closed"]
async fn kcl_test_clone_cube_already_closed_sketch() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is marked as ignored due to an unresolved issue with retrieving IDs from already closed sketches. This indicates a known limitation in the current implementation that should be addressed before merging. Consider prioritizing a fix for this functionality to ensure the clone operation works correctly with all sketch states, as this edge case could affect users working with pre-closed sketches.

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Comment on lines +298 to +303
GeometryWithImportedGeometry::Solid(solid) => {
let mut new_solid = solid.clone();
new_solid.id = new_id;
new_solid.sketch.original_id = new_id;
new_solid.artifact_id = new_id.into();
GeometryWithImportedGeometry::Solid(new_solid)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sectional property from the original solid isn't being copied to the new solid. Since this property is now being properly initialized in the do_post_extrude function later in the code, it's not causing immediate issues. However, for consistency and to avoid potential bugs if the code path changes, consider adding new_solid.sectional = solid.sectional; after setting the other fields to ensure this property is preserved during the initial cloning operation.

Suggested change
GeometryWithImportedGeometry::Solid(solid) => {
let mut new_solid = solid.clone();
new_solid.id = new_id;
new_solid.sketch.original_id = new_id;
new_solid.artifact_id = new_id.into();
GeometryWithImportedGeometry::Solid(new_solid)
GeometryWithImportedGeometry::Solid(solid) => {
let mut new_solid = solid.clone();
new_solid.id = new_id;
new_solid.sketch.original_id = new_id;
new_solid.artifact_id = new_id.into();
new_solid.sectional = solid.sectional;
GeometryWithImportedGeometry::Solid(new_solid)

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its copied by the clone doofus

Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
@jessfraz jessfraz merged commit 510d74f into main Apr 24, 2025
60 of 62 checks passed
@jessfraz jessfraz deleted the add-clone branch April 24, 2025 04:26
modeling-app-github-app bot pushed a commit that referenced this pull request Apr 24, 2025
* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

update the extrude idds

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

fix sample

Signed-off-by: Jess Frazelle <github@jessfraz.com>

better docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

fix the start and end tag

Signed-off-by: Jess Frazelle <github@jessfraz.com>

better docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

new tests

Signed-off-by: Jess Frazelle <github@jessfraz.com>

codespell

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fix examples

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fix some stuff

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fixes

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* add another test for fillet

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* Update rust/kcl-lib/src/std/clone.rs

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

* fixes

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* add sweep test

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* revolve test;

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* Update rust/kcl-lib/src/std/clone.rs

Co-authored-by: Jonathan Tran <jonnytran@gmail.com>

* add another test for fillet

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* allow cloning an imported geometry;

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* allow for imported geometry

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* update docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

---------

Signed-off-by: Jess Frazelle <github@jessfraz.com>
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Tran <jonnytran@gmail.com>
modeling-app-github-app bot pushed a commit that referenced this pull request Apr 24, 2025
* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

update the extrude idds

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

fix sample

Signed-off-by: Jess Frazelle <github@jessfraz.com>

better docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

fix the start and end tag

Signed-off-by: Jess Frazelle <github@jessfraz.com>

better docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

new tests

Signed-off-by: Jess Frazelle <github@jessfraz.com>

codespell

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fix examples

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fix some stuff

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fixes

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* add another test for fillet

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* Update rust/kcl-lib/src/std/clone.rs

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

* fixes

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* add sweep test

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* revolve test;

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* Update rust/kcl-lib/src/std/clone.rs

Co-authored-by: Jonathan Tran <jonnytran@gmail.com>

* add another test for fillet

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* allow cloning an imported geometry;

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* allow for imported geometry

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* update docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

---------

Signed-off-by: Jess Frazelle <github@jessfraz.com>
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Tran <jonnytran@gmail.com>
franknoirot pushed a commit that referenced this pull request Apr 24, 2025
* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

update the extrude idds

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

fix sample

Signed-off-by: Jess Frazelle <github@jessfraz.com>

better docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

fix the start and end tag

Signed-off-by: Jess Frazelle <github@jessfraz.com>

better docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

new tests

Signed-off-by: Jess Frazelle <github@jessfraz.com>

codespell

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fix examples

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fix some stuff

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fixes

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* add another test for fillet

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* Update rust/kcl-lib/src/std/clone.rs

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

* fixes

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* add sweep test

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* revolve test;

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* Update rust/kcl-lib/src/std/clone.rs

Co-authored-by: Jonathan Tran <jonnytran@gmail.com>

* add another test for fillet

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* allow cloning an imported geometry;

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* allow for imported geometry

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* update docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

---------

Signed-off-by: Jess Frazelle <github@jessfraz.com>
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Tran <jonnytran@gmail.com>
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.

2 participants