Undo/Redo Support

[rickshane]

What would be the easiest way to handle support for undo/redo?

I tried saving the Mantis.Config object from an existing CropViewController object and then passing it in to Mantis.setupCropViewController() on an existing CropViewController object and also to a new version in the Mantis.cropViewController init call. But this does not update the UI for the previous state. Is there a single function that will rebuild the UI based on the current state in the config object?

Thanks,

Rick

[guoyingtao]

Currently Mantis does not save view status into the config object, so it doesn't support undo/redo as you mentioned.

[rickshane]

I have undo/redo support working for all transformations except 90 degree rotations:

I took the logic from CropView.processPresetTransformation() for the .presetInfo case, and created a function for the CropViewProtocol called applyTransform() which contains this logic.

Then I take a snapshot of the Transformation object before and after a transformation by calling cropView.makeTransformation(), and send both snapshots back to my delegate. I can then undo/redo the UI state by calling back in to a new cropViewController delegate which will call the applyTransform() function.

It can successfully undo/redo the UI state for all transformations except a 90 degree rotation. Both undo and redo of a 90 degree rotation transform fail.

It works for:

1. Aspect Ratio Change
2. Degree Rotation
3. Flip Horizontal
4. Flip Vertical
5. Crop by changing crop box
6. Crop by panning
7. Crop by zooming

func applyTransform(byTransformInfo transformation: Transformation, isUpdateRotationControlView: Bool = true) {

    viewModel.horizontallyFlip = transformation.horizontallyFlipped
    viewModel.verticallyFlip = transformation.verticallyFlipped
    
    if transformation.horizontallyFlipped {
        flipOddTimes.toggle()
    }
    
    if transformation.verticallyFlipped {
        flipOddTimes.toggle()
    }
    
    var newTransform = getTransformInfo(byTransformInfo: transformation)
    
    // The first transform is just for retrieving the final cropBoxFrame
    transform(byTransformInfo: newTransform, isUpdateRotationControlView: isUpdateRotationControlView)
    
    // The second transform is for adjusting the scale of transformInfo
    let adjustScale = (viewModel.cropBoxFrame.width / viewModel.cropBoxOriginFrame.width)
    / (transformation.maskFrame.width / transformation.initialMaskFrame.width)
    
    newTransform.scale *= adjustScale
    
    transform(byTransformInfo: newTransform)
}

Any ideas what is missing? Is it the rotation type or the rotations in the viewModel?

[rickshane]

I just added the ImageRotationType state to the Transformation object. If I then set the rotationType in the viewModel from the new applyTransform() function, I can successfully undo a 90 degree rotation.

Redo of a 90 degree rotation still fails.

[rickshane]

func applyTransform(byTransformInfo transformation: Transformation, isUpdateRotationControlView: Bool = true) {

    viewModel.rotationType = transformation.rotationType
    
    viewModel.horizontallyFlip = transformation.horizontallyFlipped
    viewModel.verticallyFlip = transformation.verticallyFlipped
    
    if transformation.horizontallyFlipped {
        flipOddTimes.toggle()
    }
    
    if transformation.verticallyFlipped {
        flipOddTimes.toggle()
    }
    
    var newTransform = getTransformInfo(byTransformInfo: transformation)
    
    // The first transform is just for retrieving the final cropBoxFrame
    transform(byTransformInfo: newTransform, isUpdateRotationControlView: isUpdateRotationControlView)
    
    // The second transform is for adjusting the scale of transformInfo
    let adjustScale = (viewModel.cropBoxFrame.width / viewModel.cropBoxOriginFrame.width)
    / (transformation.maskFrame.width / transformation.initialMaskFrame.width)
    
    newTransform.scale *= adjustScale
    
    transform(byTransformInfo: newTransform)
}

[guoyingtao]

@rickshane
Would you mind posting your project with undo/redo features here? Maybe a zip file? I can do some tests with that to see why redo a 90 degree fails.

[rickshane]

Mantis_Undo_Redo.zip

Here is a zip file of the project.

Run the xcode project from the /Examples folder and select the "Embedded" version of the cropViewController.

You should see yellow "Undo" and "Redo" buttons on the left side of the toolbar.

Thanks,

Rick

[rickshane]

It also looks like undoing an Aspect Ratio change back to "Freeform" (unlocked) will not remember the aspect ratio that was set prior to changing to the "Freeform" (unlocked) . Some additional state for that is needed somewhere to match the crop rect frame.

[guoyingtao]

Restoring image crop from Transformation is always considered Freeform currently. We may need to save more state information into Transformation for undo/redo

[guoyingtao]

@rickshane
Please try viewModel.rotationType = .none instead of viewModel.rotationType = transformation.rotationType in applyTransform
The rotation value in Transformation already includes the effect of rotationType

[rickshane]

That doesn't work for redo (I get same results as before).

I also tried setting "viewModel.rotationType = .none" at beginning of applyTransform() and then "viewModel.rotationType = transformation.rotationType" as the last line of applyTransform() after the two calls to "transform()".

The rotationType state will still need to be restored, becase subsequent transforms sometimes look at rotationType.isRotatedByMultiple180

[guoyingtao]

Below is the code I am using. It works OK after changing viewModel.rotationType = transformation.rotationType to viewModel.rotationType = .none

Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2024-03-17.at.02.05.00.mp4

[rickshane]

Does it work in Catalyst for you?

[rickshane]

It works on iPhone Portrait mode for me.

It fails on iPad Landscape and Catalyst.

[rickshane]

Landscape and Portrait mode on iPad fails
Landscape mode on iPhone 12 Pro Max fails

[guoyingtao]

I noticed that too. I haven't figured out the reason yet.

[guoyingtao]

@rickshane
Can you comment out the code below to see if it works. It is for applying transform data to a different device, redo/undo happens on the same device, looks like we don't need them.
I don't know why adding them causes the issue, but looks like it works OK without this adjustScale.

        // The second transform is for adjusting the scale of transformInfo
        let adjustScale = (viewModel.cropBoxFrame.width / viewModel.cropBoxOriginFrame.width)
        / (transformation.maskFrame.width / transformation.initialMaskFrame.width)
        
        newTransform.scale *= adjustScale

[rickshane]

That's much better. I still add "viewModel.rotationType = transformation.rotationType" to restore the rotationType state so I can apply other transforms after redoing a 90 degree rotation.

The only outstanding issue seems to be undoing an aspect ratio change. I think this is because it starts in (free) unlocked without a ratio and then if you apply a new ratio and then undo back to the unlocked (free) version, the ratio does not get reset to what the ratio was when it was initially unlocked.

Thanks for spending time on this. I believe it is a very important feature.

[guoyingtao]

I agree the feature looks very useful.
For undoing an aspect ratio change, we may need save aspect operations along with transformation data. Maybe we need a operation status like

CropOperationStatus {
   var transformation
   var aspectType
}

Then apply aspectType when undo/redo

I need another time to think about it but you are welcome to modify the code in your way to see if it works

[rickshane]

My Transformation type now includes aspect ratio and lock status, which seems to fix the problem with aspect ratio undo/redo. I also changed "rotations" to "radians" and don't include rotationType radians in that number. I then make the necessary changes in applyTransform() and transform() to account for the two types of radian changes.

public typealias Transformation = (
offset: CGPoint,
radians: CGFloat,
scale: CGFloat,
isManuallyZoomed: Bool,
initialMaskFrame: CGRect,
maskFrame: CGRect,
cropWorkbenchViewBounds: CGRect,
horizontallyFlipped: Bool,
verticallyFlipped: Bool,
rotationType: ImageRotationType,
aspectRatioLockEnabled: Bool,
aspectRato: CGFloat
)

[rickshane]

I will also need to store flipOddTimes to correctly undo a flip that occurred after a rotation. I will then work on refactoring into new CropState object that stores Transformation plus all the other state needed for undo/redo.

But should rotationType stay in the Transformation object along with radians?

[guoyingtao]

I think rotationType , aspectRatioLockEnabled , and aspectRatio can all be moved out of Transformation. For me they are part of cropping status but not part of Transformation.

[rickshane]

Here is an updated version of the project with new CropState data structure
Mantis_Undo_Redo.zip

[guoyingtao]

Looks nice!

[rickshane]

I notice there is a big bug in the control that is affecting Undo/Redo logic. If I touch down just outside the crop rectangle, it causes the grid to move, even though I have not moved my finger. At a high level, this always prevents pulling the crop grid to the edge of the image. This is the biggest bug I have noticed since using this control.

This affects undo/redo because if I click just outside the lower right anchor point, it causes handle to move and inside the function "imageStatusChanged" there is a check on the point, "getImageRightBottomAnchorPoint", and since this moved, this function returns true which causes a new transform record to be pushed on to the undo stack.

[guoyingtao]

@rickshane
I figured it out.
For Mac Catalyst, even you just click a spot without moving, it still trigged touchesMoved which is different on iPhone/iPad
You can change updateCropBoxFrame(withTouchPoint: touchPoint) of touchesMoved in CropView+Touches to the code below, it should solve the issue

if touchPoint != viewModel.panOriginPoint {
    updateCropBoxFrame(withTouchPoint: touchPoint)
}

[rickshane]

This is better. There is still a Catalyst touch bug in the Rotation Dial that affects undo/redo results. When you move the dial to the limit in Catalyst, if you keep going, it inverts to the negative value.

Here is a video:

Mantis_RotationDial_Catalyst.mov

[guoyingtao]

@rickshane
Can you add this code into private func updateRotation(bySteppingAngle:) in RotationDial after let radians = steppingAngle.radians to see if it solved issue?

        guard radians != 0 else {
            return false
        }

[rickshane]

That seems to fix it. Thanks!

I am still seeing the high level bug that prevents pulling grid to edge of image in catalyst. Here is an updated video:

Mantis.Catalyst.Grid.edge.condition.mov

[guoyingtao]

@rickshane
I haven't found a good solution to fix this issue. (If you do it slower, it seems fine)

[rickshane]

Mantis_Undo_Redo.zip

Here is the new version of the project with the Equatable struct versions of CropState and Transformation.

[guoyingtao]

Looks good. If it works most of times, maybe we can consider to integrate this feature to Mantis.

[rickshane]

Mantis_Undo_Reset.zip

This latest version has support for undo/redo of the "Reset" button. There are now two different types of Transform records pushed on the stack - "Transform" and "ResetTransforms". It also disables/enables the undo/redo/reset buttons based on the state of the UndoManager and Transform Stack.

I have also hooked up the "Undo" + "Redo" + "Revert to Original" menu commands in Catalyst using the AppDelegate.buildMenu function.

[guoyingtao]

Great!
BTW, when doing reset, I think we need to clear undo/redo stack.

[rickshane]

Only clear undo/redo stack when enter or exit crop mode. This is why there are two types of records, "Transform" and "ResetTransforms". You need to be able to undo/redo a "Reset", therefore that record needs to be present on the stack.

[rickshane]

This version has the fix for Catalyst touchesMoved bug. I have added clockwiseRotate, and both flip buttons to tool bar for testing undo/redo.

Mantis_Catalyst_Touch_Fix.zip

[rickshane]

I just sent a pull request for this feature.

[rickshane]

I will close this as "Answered".