Skip to content
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

Deletion of Sketch on Offset Plane in the feature tree misses the profiles #5385

Open
pierremtb opened this issue Feb 14, 2025 · 9 comments
Open
Assignees

Comments

@pierremtb
Copy link
Collaborator

          Just noticed this deletion of Sketch in the feature tree leads to the sketch var to be delete but not the profiles in it. This can totally be fixed in a subsequent PR, I can take care of opening up a new issue for this. 
Screen.Recording.2025-02-14.at.8.43.35.AM.copy.mov

Originally posted by @pierremtb in #5196 (comment)

@pierremtb pierremtb added bug Something isn't working and removed bug Something isn't working labels Feb 14, 2025
@pierremtb pierremtb added this to the v1 Modeling App Launch milestone Feb 14, 2025
@franknoirot
Copy link
Collaborator

This is definitely something to do with deleteFromSelection, which added cases at the top of the function (using artifact graph logic) to delete the paths when a sketch plane is deleted. It must not be properly catching the case of a plane artifact that is an offset plane.

@jtran expressed some concern that this has something to do with the artifact graph representation of offset planes, so I want to leave space in case that is where the problem actually lies. That would probably be something about sketches on offset planes creating new plane artifacts.

@franknoirot
Copy link
Collaborator

Okay yes, this is related to the artifact graph representation of offset planes. Selecting the sketch item that is on an offset plane gets a null artifact, then the source range is naive and only deletes the plane's declaration line.

@franknoirot
Copy link
Collaborator

franknoirot commented Feb 18, 2025

@jtran
Copy link
Collaborator

jtran commented Feb 19, 2025

We should chat about this.

@jtran
Copy link
Collaborator

jtran commented Feb 20, 2025

Does this branch, adding the code ref, help at all? main...jtran/plane-code-ref

@franknoirot
Copy link
Collaborator

Does this branch, adding the code ref, help at all? main...jtran/plane-code-ref

Let me try @jtran, thanks!

@franknoirot
Copy link
Collaborator

@jtran I don't think I can see this codeRef in the artifactGraph because "sketch" type artifacts are not included there? Let me know if I'm mistaken though

Screenshare.-.2025-02-20.12_22_23.PM-compressed.mp4

@jtran
Copy link
Collaborator

jtran commented Feb 20, 2025

The TODO and the code_ref branch so far only affects sketch-on-face. Specifically if you use startSketchOn() with a Plane or a Solid. You're using 'XZ', which uses the PlaneData type. So the branch has no effect there. We can huddle about this to discuss next steps.

@franknoirot
Copy link
Collaborator

The TODO and the code_ref branch so far only affects sketch-on-face. Specifically if you use startSketchOn() with a Plane or a Solid. You're using 'XZ', which uses the PlaneData type. So the branch has no effect there. We can huddle about this to discuss next steps.

I'm a little confused, here's my KCL code:

plane001 = offsetPlane('XZ', offset = 5)
sketch001 = startSketchOn(plane001)
    |> circle({ center = [0, 0], radius = 5 }, %)

I though I was using startSketchOn() with a Plane; a new plane offset from 'XZ' by 5 units?

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

No branches or pull requests

4 participants