fix: joining paths from different layers with Ctrl+J fails

[Ayush2k02]

screenrecording.graphite.mp4

Fixes #3519

[Keavon]

Hello and thanks for the PR. I need to mention that we do not accept PR descriptions written by AI because they increase the noise-to-signal ratio and usually serve to mislead with obscured inaccuracies. (The same applies to code, which must be written by you.)

[Ayush2k02]

Oh, hey , I had this extension turned on in my IDE https://whatthediff.ai/ , will be mindful to disable it when creating PRs in this repo :) . Also regarding using AI in code, I did use claude code for targeting the files and the fix but did review that intensively before publishing .

[Keavon]

Thanks for the insight into your process (we've been seeing more highly-elaborate-but-not-insightful PR descriptions like this and it's good to know some of the details about what kinds of AI tools are producing them). For code authorship, AI is acceptable for auto-completing lines you would already be typing, but we need contributions to always come from the original thought processes of its human authors who can answer to the reasoning behind each line of changes. Code can't be mysteriously changed without anyone aware of why the change was made. Thanks for working to follow our requirements to building an effective software project.

[0HyperCube]

It doesn't work. I have tried it with some simple lines with a transform.

I guess the vibes haven't fully completed the code.

fail_to_join.graphite

cannot_join_lines_with_transform.mp4

[Ayush2k02]

Taking a look

[Ayush2k02]

After taking a second look at the issue, here is what I came up with: The fix uses index-based tracking to handle PointId remapping:

• Vector::concat() preserves point ordering (layer1 points first, then layer2 points)
• Save each point's index before merge
• Calculate post-merge indices using layer1's point count as offset
• Look up actual PointIds at those indices after merge

Other approaches I considered:

• Position-based matching: Save coordinates, search within 250px tolerance after merge. Fragile—Flatten Path shifts coordinates unpredictably, large tolerance risks wrong matches.
• Hash-based ID prediction: Replicate collision resolution to predict new PointIds. Impractical—hash seed needs internal parameters (current_index, vector_index, node_id) inaccessible from shape_editor.rs.

[Ayush2k02]

Let me know if I am missing something or if there could be any other edge case here

[Ayush2k02]

Screen.Recording.2025-12-28.at.7.12.43.AM.mov

More manual testing with data grid for when the conflicting PointId is not updated so the wrong segment is created , here it is not created .

[0HyperCube]

fail_with_repeat.mp4

However I guess it doesn't matter too much.

[Ayush2k02]

I have a better solution for this, let me code it up

So basically the issue with the current approach was when layers use the repeat modifier. The merge operation flattens repeated instances, breaking the expected point array structure.

Expected behavior:

• Layer1 has x points
• Layer2 has y points
• After merge: x + y points total

Actual behavior with repeat:

• Layer1 with 2 points × 3 repeat instances = 6 points (not 2)
• Layer2 with 2 points = 2 points
• After merge: 8 points total (not 4)

Since the old approach calculated indices using end_index = end_idx + layer1_count, it assumed layer1_count = 2, but after flattening the repeated instances, layer1 actually contributes 6 points. This causes the lookup to select the wrong points.
Solution

Instead of relying on indices (which change when instances are flattened), we now:

1. Before merge: Store the world-space coordinates of the selected points
2. After merge: Reverse-lookup the PointIDs by searching for those exact coordinates

[Ayush2k02]

screenrecording.graphite.mp4

Some manual testing after the recent commit

[Ayush2k02]

let me know if any other changes are required

[0HyperCube]

It doesn't work if you have two lines in a group with a transform. You can press G with a group selected to apply a transform.

fail_to_join_group.mp4

[Ayush2k02]

taking a look !

[Ayush2k02]

Is this like a big limitation , this seems like a big change . I'll dig deeper if this is like a game breaker , and its happening on PROD as well , like its not related to my code changes .

[0HyperCube]

@Ayush2k02 I thought that this PR was meant to "fix: joining paths from different layers with Ctrl+J".

From my very basic testing, it seems it still does not do this in some very common scenarios.

I hope that helps to inform your approach.

[Ayush2k02]

Oh, are there any more edge cases that you were able to find apart from this one "It doesn't work if you have two lines in a group with a transform. You can press G with a group selected to apply a transform" ,
I'll look into this and fix it then

[0HyperCube]

I have not found any more issues with the current approach. However I will be sure to test it again if you decide to modify the code.

[Ayush2k02]

Hey, fixed the case with group selection. Let me know if there is anything else or if some optimizations could be made

[0HyperCube]

When joining two points from the same layer, two segments are created.
The Ctrl + J is meant to insert exactly one segment between the two points.

two_segments.mp4

[Ayush2k02]

Completely missed this, I did try the same layer case with using shift and creating some lines but did'nt look well enough for the underlying line . Let me fix this quick, most likely will keep the same functionality for the group case and remove this from the same layer case . Sorry for the negligence :P

[Ayush2k02]

Fixed the last issue, the check for group / folder was broken

Now I am checking if the layer has any children, means it is a group folder . The method has_children also seems appropriate here, hopefully there shouldn't be anymore bugs in this code path at least

[0HyperCube]

It still seems to produce duplicate segments for me when joining points in the same layer inside a folder:

duplicated_segments.mp4

[Ayush2k02]

Okay, so added a check for if the children are more than 1 means we are inside a group , then only go down the handle_grouped_transform_close_path path , else directly create a segment . Have tested all the 3 cases down this path

1. simple single layer join paths
2. grouped lines which are in different layers
3. grouped image but we are in a single layer

[Ayush2k02]

wait, its still not correct. Dont review yet

[Ayush2k02]

Hmmm, for some reason when I connect straight lines inside a group with cmd + j it fails the first time as the point ids are getting remapped but when the exact same thing with creating lines from the pen tool or the spline tool , it succeeds and create the segment in the first try , weird

[Ayush2k02]

Okay, so changing the reorganisation method to use ApplyPointDelta instead of creating a dummy segment fixed all the edge cases for me . This is now ready for review from my side . But unsure why complex paths (created with the spline/pen tool) inside a grouped folder successfully created the dummy segment on the first try, while simpler straight lines failed to do so. Something related to the pointIds remapping with RunDocumentGraph

[Ayush2k02]

This is now reviewable from my side !