Selecting the closest layer when deleting #613
Conversation
| if (doc.StructureHelper.GetAllLayers().Count > 1) | ||
| { | ||
| //Declare the layer that will be selected | ||
| var baseLayer = doc.StructureHelper.GetAllLayers()[doc.StructureHelper.GetAllLayers().ToArray().Length - 1]; | ||
|
|
||
| for (int i = 0; i < doc.StructureHelper.GetAllLayers().Count; i++) | ||
| { | ||
| //Checking that the selected layer and the deleted layer are not the same | ||
| if (doc.StructureHelper.GetAllLayers()[i] == member) | ||
| { | ||
| //Selecting the new layer | ||
| if (i > 0) | ||
| { | ||
| baseLayer = doc.StructureHelper.GetAllLayers()[i-1]; | ||
| } | ||
| else if (i == doc.StructureHelper.GetAllLayers().Count - 1) | ||
| { | ||
| baseLayer = doc.StructureHelper.GetAllLayers()[i]; | ||
| } | ||
| else if (i == 0) | ||
| { | ||
| baseLayer = doc.StructureHelper.GetAllLayers().ToArray()[i+1]; | ||
| } | ||
|
|
||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
You are calling GetAllLayers() 8 times, which is very inefficient, call it once and cast to array once.
|
I had to modify the 2024-04-02.02-05-17.mp4 |
| } | ||
| } | ||
|
|
||
| doc.InternalSetSelectedMember(baseLayer); |
There was a problem hiding this comment.
Hey, this is incorrect, "Internal" methods of DocumentViewModel can't be called outside DocumentUpdater. You should call doc.Operations.SetSelectedMember instead
In short, all updates to document have to go though a "change" pipeline for the document state to remain consistent. "Internal" methods bypass the pipeline. There is some stuff written up on the architecture of PixiEditor here: https://pixieditor.net/docs/start-here, you can read it if you want to really understand what is going on
| } | ||
|
|
||
| doc.InternalSetSelectedMember(baseLayer); | ||
| doc.Operations.AddSoftSelectedMember(baseLayer.GuidValue); |
There was a problem hiding this comment.
"Soft" selection is used in the case where you have multiple layers selected. To handle drawing in that case, we need to choose a single layer to draw on. Because of that there are two types of selection, "Hard" selection (only one layer can be hard-selected) and "Soft" selection (any selected layers that aren't the main one will be "soft" selected).
It looks like you are trying to hard-select baseLayer, and immediately afterwards also soft-select it, which doesn't make sense (and there is a check in DocumentUpdater.ProcessAddSoftSelectedMember which ensures that you can't soft-select a hard-selected layer)
| member.Document.Operations.DeleteStructureMember(member.GuidValue); | ||
| var allLayers = doc.StructureHelper.GetAllMembers().ToArray(); |
There was a problem hiding this comment.
you can iterate over doc.StructureRoot.Children directly, no need to copy the structure into a separate list. You can get rid of both GetAllMembers overloads (one of them seems to be unused already?)
Edit: although, you likely won't need doc.StructureRoot.Children at all, see further comments
| } | ||
| else | ||
| { | ||
| //Managing elements inside folders |
There was a problem hiding this comment.
Unless I'm missing something here (I haven't run the code, sry) this doesn't handle nested folders (since folders can contain other folders that can contain other folders..). I think you need an algorithm that goes something like:
- Find the parent of the member being deleted (
DocumentStructureModule.FindChildAndParentOrThrow) - If the parent has only one child (which is getting deleted) then select the parent
- Otherwise, use a loop to find the closest child to the one getting deleted (like you are already doing).
- Select found child
Selecting the closest layer when deleting (similar to GIMP)
pixiEditor.mp4
I noticed that when deleting a layer, there was no layer selected after, and this make working uncomfortable because you couldn't draw after deleting a layer, and you had to manually select the layer you wanted to work with.
The logic used was: