Pasting textures from face cannot be undone in a single Undo

[scar3crow]

Undoing a texture paste requires a separate Undo action per face on the brush.

Expected Behavior

Single actions such as ALT-double click can be undone with a single action of Undo.

Steps to Reproduce

1. Launch Trenchbroom and start a map
2. Create two brushes, set a texture to one
3. Select a face of the textured brush
4. ALT-double click the other brush
5. Undo

[ericwa]

I commented on discord:

Yeah - there is something off about undoing the alt+double-click texture application.. it's not undoing the faces one by one though. What I see with 2019.6 if I alt+double-click on a cube, first undo does nothing (the menu label says Select None), second restores 5/6 of the faces, then the next undo restores the last one
Likewise if you double click on a brush to select everything, and then try to undo those selection actions, it counts as 2 steps, the first brush selected, and then the double click

[kduske]

This seems to be because mouse double click events in Qt generate the following sequence of events:

• mouse down,
• mouse up,
• mouse double click,
• mouse up.

Our event recorder translates this to

• mouse down,
• mouse click,
• mouse up,
• mouse double click,
• mouse click,
• mouse up

[kduske]

Ideally, we would only enqueue three events:

• mouse down
• mouse double click
• mouse up

I think this is what we did on wxWidgets. I'll have to think about it a bit, but I think we don't want to enqueue the first click of a double click at all. I don't know how to ensure that yet though.

[kduske]

In the very least, we should translate the events to

• mouse down,
• mouse click,
• mouse up,
• (mouse down,)
• mouse double click,
• (mouse up)

And possible we could try and discard the second mouse down and up events altogether. The problem would still be that SetFaceAttributesTool will create two individual transactions, one for the first single click, and one for the double click. This is a problem in general I am afraid, as other tools that react to both single and double click will all behave in the same way.

[kduske]

One solution would be to delay the processing of the first mouse click event until we know it's not a double click using a timer with a timeout longer than QApplication::doubleClickInterval, but that would feel odd. Another solution is to collate the commands generated by the two events so that they are merged into a single transaction.

I think this is what the wx version actually did, but this behavior currently is broken because CommandProcessor does not collate individual transactions. I'll look into this, because that's the most likely fix. So to recap, a possible fix could be:

• fix the mouse events that we generate from the Qt events
• allow CommandProcessor to collate consecutive transactions (within a certain time limit).

[kduske]

Actually the correct fix is to just undo the last transaction when processing a double click event!