UndoFX: Undo/Redo recording every pixel of drag

I'm using UndoFX & ReactFX for implementing the Undo/Redo function for my 2D shape application.

The problem is when i move my shape the EventStream records every X/Y pixel of movement. I just want to record the last position (when the user releases the drag).

What i have tried so far:

Instead of using changesOf(rect.xProperty()).map(c -> new xChange(c)); and changesOf(rect.yProperty()).map(c -> new yChange(c)); I created a DoubleProperty x,y, and saved the shape x,y Property to these variables when the user mouse is released. Lastly i change the changesOf to: changesOf(this.x).map(c -> new xChange(c)); and changesOf(this.y).map(c -> new yChange(c));

But that did not work, it behaved just like before.

....
private class xChange extends RectangleChange<Double> {

    public xChange(Double oldValue, Double newValue) {
        super(oldValue, newValue);
    }
    public xChange(Change<Number> c) {
        super(c.getOldValue().doubleValue(), c.getNewValue().doubleValue());
    }
    @Override void redo() { rect.setX(newValue); }
    @Override xChange invert() { return new xChange(newValue, oldValue); }
    @Override Optional<RectangleChange<?>> mergeWith(RectangleChange<?> other) {
        if(other instanceof xChange) {
            return Optional.of(new xChange(oldValue, ((xChange) other).newValue));
        } else {
            return Optional.empty();
        }
    }

    @Override
    public boolean equals(Object other) {
        if(other instanceof xChange) {
            xChange that = (xChange) other;
            return Objects.equals(this.oldValue, that.oldValue)
                && Objects.equals(this.newValue, that.newValue);
        } else {
            return false;
        }
    }
}

...
    EventStream<xChange> xChanges = changesOf(rect.xProperty()).map(c -> new xChange(c));
    EventStream<yChange> yChanges = changesOf(rect.yProperty()).map(c -> new yChange(c));
    changes = merge(widthChanges, heightChanges, xChanges, yChanges);
    undoManager = UndoManagerFactory.unlimitedHistoryUndoManager(
            changes, // stream of changes to observe
            c -> c.invert(), // function to invert a change
            c -> c.redo(), // function to undo a change
            (c1, c2) -> c1.mergeWith(c2)); // function to merge two changes

Solution

You need to merge the changes in x with the changes in y. At present, a change in x followed by a change in y cannot be merged, so if you move the shape so that it alternates x and y changes (e.g. moving it diagonally), then each individual change will not merge with the previous one.

One way to do this is to generate changes whose old and new values are the locations, e.g. represented by Point2D objects. Here's a quick example:

import java.util.Objects;
import java.util.Optional;

import org.fxmisc.undo.UndoManager;
import org.fxmisc.undo.UndoManagerFactory;
import org.reactfx.EventStream;
import org.reactfx.EventStreams;
import org.reactfx.SuspendableEventStream;

import javafx.application.Application;
import javafx.beans.binding.Bindings;
import javafx.geometry.Insets;
import javafx.geometry.Point2D;
import javafx.geometry.Pos;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.layout.BorderPane;
import javafx.scene.layout.HBox;
import javafx.scene.layout.Pane;
import javafx.scene.paint.Color;
import javafx.scene.shape.Rectangle;
import javafx.stage.Stage;

public class UndoRectangle extends Application {



    @Override
    public void start(Stage primaryStage) {
        Rectangle rect = new Rectangle(50, 50, 150, 100);
        rect.setFill(Color.CORNFLOWERBLUE);

        EventStream<PositionChange> xChanges = EventStreams.changesOf(rect.xProperty()).map(c -> {
            double oldX = c.getOldValue().doubleValue();
            double newX = c.getNewValue().doubleValue();
            double y = rect.getY();
            return new PositionChange(new Point2D(oldX, y), new Point2D(newX, y));
        });

        EventStream<PositionChange> yChanges = EventStreams.changesOf(rect.yProperty()).map(c -> {
            double oldY = c.getOldValue().doubleValue();
            double newY = c.getNewValue().doubleValue();
            double x = rect.getX();
            return new PositionChange(new Point2D(x, oldY), new Point2D(x, newY));
        });

        SuspendableEventStream<PositionChange> posChanges = EventStreams.merge(xChanges, yChanges)
                .reducible(PositionChange::merge);

        UndoManager undoManager = UndoManagerFactory.unlimitedHistoryUndoManager(posChanges, 
            PositionChange::invert,
            c -> posChanges.suspendWhile(() -> {
                rect.setX(c.getNewPosition().getX());
                rect.setY(c.getNewPosition().getY());
            }),
            (c1, c2) -> Optional.of(c1.merge(c2))
        );

        class MouseLoc { double x, y ; }

        MouseLoc mouseLoc = new MouseLoc();

        rect.setOnMousePressed(e -> {
            mouseLoc.x = e.getSceneX();
            mouseLoc.y = e.getSceneY();
        });

        rect.setOnMouseDragged(e -> {
            rect.setX(rect.getX() + e.getSceneX() - mouseLoc.x);
            rect.setY(rect.getY() + e.getSceneY() - mouseLoc.y);
            mouseLoc.x = e.getSceneX();
            mouseLoc.y = e.getSceneY();
        });

        rect.setOnMouseReleased(e -> undoManager.preventMerge());

        Pane pane = new Pane(rect);

        Button undo = new Button("Undo");
        undo.disableProperty().bind(Bindings.not(undoManager.undoAvailableProperty()));
        undo.setOnAction(e -> undoManager.undo());

        Button redo = new Button("Redo");
        redo.disableProperty().bind(Bindings.not(undoManager.redoAvailableProperty()));
        redo.setOnAction(e -> undoManager.redo());

        HBox buttons = new HBox(5, undo, redo);
        buttons.setAlignment(Pos.CENTER);
        BorderPane.setMargin(buttons, new Insets(5));
        BorderPane root = new BorderPane(pane, null, null, buttons, null);

        Scene scene = new Scene(root, 600, 600);
        primaryStage.setScene(scene);
        primaryStage.show();
    }

    public static class PositionChange {
        private final Point2D oldPosition ;
        private final Point2D newPosition ;

        public PositionChange(Point2D oldPos, Point2D newPos) {
            this.oldPosition = oldPos ;
            this.newPosition = newPos ;
        }

        public Point2D getOldPosition() {
            return oldPosition;
        }

        public Point2D getNewPosition() {
            return newPosition;
        }

        public PositionChange merge(PositionChange other) {
            return new PositionChange(oldPosition, other.newPosition);
        }

        public PositionChange invert() {
            return new PositionChange(newPosition, oldPosition);
        }

        @Override
        public boolean equals(Object o) {
            if (o instanceof PositionChange) {
                PositionChange other = (PositionChange) o ;
                return Objects.equals(oldPosition, other.oldPosition)
                        && Objects.equals(newPosition, other.newPosition);
            } else return false ;
        }

        @Override
        public int hashCode() {
            return Objects.hash(oldPosition, newPosition);
        }

    }

    public static void main(String[] args) {
        launch(args);
    }
}

Note that it's important the "undo" is implemented as an "atomic" change, so the undo manager sees (and ignores) a single change when you implement the undo. This can be achieved by suspending the event stream during the undo.