Rewrite temporary tool handling

[scribblemaniac]

This moves all the handling of temporary tools out of ScribbleArea and into ToolManager. There are also changes to the implementation and behavior that will make it more verstatile and able to handle weird corner cases better (ex. shortcuts while using temporary tools). There's too many changes to how the temporary tools work to list them all, but here is a list of the bigger changes that I can think of off the top of my head:

• Shortcuts work with temporary tools now. If you say used the right click to use the hand tool temporarily, then you could hold control and it would zoom just as holding control with the hand tool normally does.
• You cannot have nested temporary tools. The first temporary tool activated takes precedence and prevents any further temporary tools from being activated at the same time as it.
• Tablets with pen erasers switch has it's own temporary tool "level". When using an eraser, the eraser tool temporarily takes precedence over the currently selected tool, however you can still select a temporary tool and that will take precedence over the temporary eraser tool. For example, you could switch to the eraser side, press and hold space to activate the temporary hand tool, move the canvas, and then release space to return to the eraser tool (or return to the main tool if you have already swiched back to the pen side).
• Temporary tools are bound to the action that initiates them. They should only be cleared when the keys/buttons you pressed to initiate it are released. So if there are two shortcuts that lead to the same temporary tool, and you press both of them, you will need to release the first shortcut pressed to stop using the temporary tool. In practice this does not matter currently, however the fact that there are no longer hardcoded checks for release events for particular shortcuts means that it will hopefully be much simpler in the future to add support for custom temporary tool shortcuts (Ability to rebind the purpose of hard-coded actions. #1572).

One thing that I'm not entirely happy about even after this PR is the handling of tool leaving. In this PR it calls leavingThisTool when changing the main tool, but not when changing the temporary tool. This is basically how the code is intended to work now (this actually wasn't doing anything because of an error). However I struggle with what the more desirable behavior is. Currently it is only relevant to the select tool, which allows you to temporarily use the move tool with a shortcut. When the temporary tool is done, should the transformation automatically be applied? It is when switch tools normally, but not when using it as a temporary tool. The select tool will apply a transformation before modifying a selection so it doesn't really cause any issues, and it could potentially help if a person switches to the move tool temporarily multiple times so it mostly makes sense for this. On the flip side the polyline tool also uses leavingThisTool now. I can't imagine a situation where you would want to stop using the polyline tool without applying or cancelling the current path (going from a polyline main tool to say a temporary hand tool would be a valid use, but the polyline tool is still considered in use in this case). There is no way to temporarily activate the polyline tool though so the temporary tool distinction doesn't matter for it currently.

Fixes #1271.

[MrStevns]

Looks good to me, I only have one question. Why not use Qt::Key enum instead of int for setTemporaryTool and tryClearTemporaryTool. I assume it's because you want to have an invalid state, so you can clear the state but I feel like it would be more safe to use Key_unknown instead. What do you think?

[scribblemaniac]

Why not use Qt::Key enum instead of int for setTemporaryTool and tryClearTemporaryTool. I assume it's because you want to have an invalid state, so you can clear the state but I feel like it would be more safe to use Key_unknown instead. What do you think?

I chose int simply because that's what's returned by QKeyEvent::key(). Additionally the original intent was to include the keyboard modifiers in it with an OR operation, but I ended up making a different variable for them because I realized the KeyboardModifiers don't match their corresponding keys so I would have to convert between them anyway. Also I wanted to support shortcuts that used multiple keys non-modifier keys, but I realize now that does not work with the way I've currently implemented this. I would not be opposed to switching to a QFlags<Qt::Key> type, which should be able to handle that, but would require extra checks to set it to Key_unknown if QKeyEvent::key() returns an value (esp. 0).

[scribblemaniac]

Qt::Key suggestion has be implemented and pushed to master.