Skip to content

Conversation

@Ayush2k02
Copy link

@Ayush2k02 Ayush2k02 commented Dec 27, 2025

Summary

Screen.Recording.2025-12-27.at.6.41.56.AM.mov

Fixes #3519

This PR fixes the issue where joining paths from different layers with Ctrl+J fails because PointIds may change after merge_layers() runs due to ID collision resolution.

Details

When we try to join two points from different layers, we combine those layers into one. During this merge, the internal IDs of the points can change to avoid conflicts. The old code tried to connect the points immediately, but by then the IDs it was looking for didn't exist anymore. This fix works in three steps:

  1. Save the locations first - Before merging the layers, remember where the two points are located on the canvas (their x,y coordinates)
  2. Wait for the merge to finish - Let the layers merge completely. The merge happens in the background, so we schedule our next step to run after it's done
  3. Find the points again and connect them - Once the merge is complete, look for points at the saved locations. These points might have new IDs now, but they're still in the same spot. Use these updated IDs to create the connecting line

When joining paths from different layers with Ctrl+J, the PointIds
may change after merge_layers() runs due to ID collision resolution.
This fix:
- Captures the positions of the selected points before merging
- Defers the segment creation until after the document graph runs
- Re-resolves the PointIds by finding points at the saved positions
- Creates the segment only if both points are found in the merged vector

Fixes GraphiteEditor#3519
@Ayush2k02 Ayush2k02 marked this pull request as draft December 27, 2025 01:04
@Keavon
Copy link
Member

Keavon commented Dec 27, 2025

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 Ayush2k02 marked this pull request as ready for review December 27, 2025 01:12
@Ayush2k02
Copy link
Author

Ayush2k02 commented Dec 27, 2025

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 .

@Ayush2k02 Ayush2k02 changed the title fix: defer segment creation after merge to handle PointId remapping fix: joining paths from different layers with Ctrl+J fails Dec 27, 2025
@Keavon
Copy link
Member

Keavon commented Dec 27, 2025

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.

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.

Joining paths with the same PointIds doesn't work

2 participants