Layer merge

[davidlamhauge]

This feature adresses an issue in the Official Roadmap #540.
It is possible to merge bitmap and vector layers. It is only tested with bitmap layers, and maybe vector layers should be commented out in the code, until the vector layers are useable again.
The feature is only available if there are at least two layers of the correct type in the timeline.

[MrStevns]

Personally I don't like that a dialog is needed to merge a layer. This can be done much simpler. I'd personally want to see this being a "select item on timeline to merge with", right-click and select "merge layers"

As for the technical side. Only UI related actions should be implemented in the widgets themselves, otherwise it's impossible to test.

The merge implementation should be added to Layer as a virtual function and then implemented in Vector layer and BitmapLayer respectively to handle each case.

[davidlamhauge]

It could be done in many ways, and of course I like my suggestion best.
In your suggestion @candyface , it is not clear to me what layer is merged onto which layer. If I merge a drawing and a coloring layer, it is vital that the artwork is on top of the color layer. Besides that, when you "right-click and select 'Merge Layers'", you need some sort of dialog. Don't you?
You're right that the merge should be done away from the widget. I implemented it in the Layermanager.cpp, which I thought was logical, but I guess it could be in Layer as you suggests.

[MrStevns]

Merging through a right-click menu and selecting the items is the default/status quo way of merging layers in a lot of applications.

Here's how it's done in Clip Studio Paint

cmd/ctrl+click on the the layer you want to merge into.

result:

another example:

step1 :
Layer 2
Layer 1

step 2:
Layer 2 - selected
Layer 1 - right-click and merge layers

Layer 1 - merged.

As I mentioned, it's the normal way of doing it in a lot of applications, so i'd prefer if we did the same too. I personally think it is more intuitive than seeing a dialog and having to select the layers you want to merge.

No dialog is needed, if you don't like the merge, then undo the process.

Merging in this case always happens downwards.

[davidlamhauge]

Some Questions:

1. Is Layer 2 automatically deleted after the merge?
2. If the layers are of different type, there is no merge. Is it okay with an Informational messagebox here?
3. Is there a standard way to make these context menus? (Are there any in Pencil2D?)

[MrStevns]

1. Yes when a merge is initiated, only the bottom layer will stay, the other layer is removed.
2. If two types are different or the merge for some other reason can't be done, the menu item should be grayed out and not be clickable.
3. There's a context menu in the Color palette widget, look at colorpalettewidget.cpp to see how I implemented that one.

[scribblemaniac]

I think my idea this is what @candyface is suggesting but with a different wording. There could be a "Merge Visible" option which would merge all layers that are visible (and compatible). Or it could be a "Merge Down"/"Merge with Layer Below" option which merges the currently selected layer (or layer selected with a right click) with the one below it if possible. I would be most in favor of the latter option for its simplicity, lack of dialog window, and similarity to functions in other applications such as GIMP and Krita. They could be implemented in the context menu or as a timeline button.

[davidlamhauge]

Thanks for the clarifications and suggestions @candyface and @scribblemaniac .
I'm on holiday from April 5'th to 12'th, and I haven't got time to do it before I leave, so we can think about the options the next week.
What about the merge function itself? I'll still suggest to have in LayerManager.cpp, while CandyFace suggests a virtual function in Layer.cpp.

[davidlamhauge]

I have had some time to work on it, and have made an update on the PR.
If you right-click on a layer, in the left part of the timeline, you get a popup menu where you can:

1. Merge layers, if the selected layer and the layer below are both bitmap layers. I couldn't find a way to disable the menu item (gray it out), so the action is not added, if the conditions are not met. If you know how to set Enabled to False, please tell me.
2. Delete the selected layer.

I also thought of the possibility to Duplicate the selected layer, but I wanted to hear your thoughts about that first. It would be a feature that I would make use of - that's for sure.
Like in GIMP, the Merge and Deletion is made without warning/confirmation.

[MrStevns]

Reviewing...

[MrStevns]

Bug:
Application crashes when you undo the process prior to the layers being merged (presumably because it's looking for a layer that's not there anymore) this will have to be fixed. Ideally you should add undo/redo to the behaviour but the current undo/redo system doesn't support that as far as I know and implementing it would be waste of time, considering that I have a much better and extensive undo/redo system waiting to be reviewed and merged too.

To get this approved before my undo/redo rewrite however we would most likely have to clear the undo/redo queue, so if the user decides to merge a layer, then that action will reset the undo/redo queue or ignore the action much like when deleting a layer... it's a ugly solution but I don't see other ways to fix this problem. This would of course require a dialog that explains that the action is destructive and can't be reverted.

UI/UX comments:
The merge behaviour is somewhat unintuitive, you can only select one of the layers, (the upper one in the hierarchy) and merge onto the layer below. Holding down shift and clicking on a layer should highlight it too, to visualize that you want to do an operation across multiple layers.

[davidlamhauge]

I have added duplicate layer to the context menu.
We still have to wait for the undo-redo, before this PR can be accepted.

[davidlamhauge]

I have added support for merging all visible layers -a feature mentioned in our discussion.
Test it and tell me what you think. It's easy to undo the commit, if we don't think we should add it.