search

GraphiteEditor / Graphite

2D vector & raster editor that melds traditional layers & tools with a modern node-based, non-destructive, procedural workflow.
https://graphite.rs
Apache License 2.0
7.31k stars 387 forks source link

Fix orphaned child layers left behind when a group is ungrouped or deleted #1655

Closed haikalvidya closed 3 months ago

haikalvidya commented 3 months ago

this is resolve delete_layer function that failing to fully remove all child nodes associated with the parent layer.

Closes #1654

Keavon commented 3 months ago

!build

github-actions[bot] commented 3 months ago
📦 Build Complete for c78820f23056e37ffe0d5164569791a27e6b1a4f
https://338fb2a8.graphite.pages.dev
Keavon commented 3 months ago

I may have assumed something different when I renamed this PR. But what I assumed this PR does, now that I'm trying it at the build link above, doesn't seem to work. I draw an ellipse layer, select the layer, and hit delete; the nodes that fed into that layer are still left behind. Is that expected? Or can you please direct me to what this PR is intended to do if I misinterpreted its purpose? Thanks.

0HyperCube commented 3 months ago

@Keavon This works fine for me. Note that if you have the node graph open, the selected node rather than layer will be deleted (which in your case is just the layer node). If you close the node graph and then delete the layer, it will work as expected. This is the same as on the main branch.

This PR fixes an issue whereby when deleting a group containing >=2 layers, the nodes only for the top most child would be deleted (as we followed the primary flow from the group which led through only the top most child layer). However this has now been resolved by also following the primary flow of all child layers.

haikalvidya commented 3 months ago

This PR fixes an issue whereby when deleting a group containing >=2 layers, the nodes only for the top most child would be deleted

Yeah, there seems to be a misunderstanding in this PR. The title or message from the PR that has been modified is inaccurate, the correct title or messages should be those related to "removing all children from layers/nodes".

How to reproduce the effect of this PR's modification:

  1. Create two object
  2. Grouping it
  3. and then ungrouping it
  4. Look at the node graph

or you can do this too

  1. Create two object
  2. Grouping both of it
  3. Delete the parent layer or layer that group those two object
  4. Check the node graph

In production it would be leave one object with the attribute nodes that related. The expected from this PR modification is all of object under that group layer is deleted.

Keavon commented 3 months ago

Ah, that makes sense. Thanks for clarifying and pardon my confusion! This looks nice.