Page MenuHome

Fix T66142: Redo panel doesn't work correctly after certain transformations.
Needs ReviewPublic

Authored by Sebastian Parborg (zeddb) on Jul 3 2019, 4:24 PM.

Details

Summary

The redo panel was broken in edit mode when transforming mesh elements individually.
It was also broken in object mode when for example transforming multiple object along their respective local transform space.

I have made changes to fix this. However I did notice that this breaks the rotational redo if you simply rotate a object without constraining it to any axis.
The redo panel will say that it transform along the view axis, but will not do so when you nudge it.

Even with this broken, I would like to have to feedback as it seems like the transform system is a very delicate beast.

Diff Detail

Event Timeline

Sebastian Parborg (zeddb) updated this revision to Diff 16219.

Fixed the rotation problem. It now seems to work as expected.

I looked at this for an hour but just can't properly understand what is going on in this fix. I think we should postpone this for after 2.80 even if it's a serious bug, we risk breaking something more important.

What would help would be some code comments to clarify what each change is doing.

The T_EDIT changes are particularly unclear, whatever that is doing should probably be fixed elsewhere.

I agree the with T_EDIT thing. But the fact is that this is how it is currently coded in the other parts of the transform code too.

The reason to check for T_EDIT is that the transform values work differently in object mode and edit mode (I have no clue why this is a good idea or how this came to be).
In edit mode, only the first of the three transform channels are used. So if you have a X,Y,Z transform channel, regardless if you transform in Y or Z the transform amount will only be written to the X channel. This is then converted back into the actual channel:

Single channel transform value:

Object mode:
Input | Transform channel | Saved value
  X   ->      X          ->     X
  Y   ->      Y          ->     Y
  Z   ->      Z          ->     Z

Edit mode:
Input | Transform channel | Saved value
  X   ->      X          ->     X
  Y   ->      X          ->     Y
  Z   ->      X          ->     Z

This is one of many reasons I think that the transform code should be refactored/rewritten to not be a mess of spagetti code with strange special cases like this.
However for this fix, I just wanted to get it to work. So I copied the logic from the other functions to get it to work. And it seems to be working on my end.

It's just as you said very hard to understand what it going on and why. But I don't think this code addition will make it any harder. But I will of course add comments and change the current patch if you wish.

I haven't tested it but at first glance it doesn't look like a dangerous patch.

Really the code is a mess, it would be nice to rewrite the parts that could be improved (as in this case try to unify the logic of object mode and edit mode).
One thing I learned from investigating the transformation code is that the final transformation is calculated on the callbacks in tranform.c (applyTranslation, applyResize, applyRotation ...).
During modal, t->values is like an input that is calculated from the mouse position. It can be a simple value, or a vector depending on the type of transformation (Scaling for example is a ratio while translating is a vector).
From the value in t->values, t->values_final is calculated and this is the value that will appear in the redo panel.

Your solution apparently was to change the calculation of the contrain matrix and set some properties to be read in the redo.
So to me it looks ok.