0

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
Senpai
  • 515
  • 6
  • 18
  • I suggest _merging_ subsequent changes of the same kind (here moving the shape) into one change. It is demonstrated in the [demo](https://github.com/TomasMikula/UndoFX/#demo) (see the `mergeWith` methods in `CenterXChange` and `CenterYChange`). – Tomas Mikula May 24 '16 at 16:09
  • I just tried that, did not work :( same behaviour again! Actually the exact problem is when i move the shape horizontally and vertically the same time. If i move it exactly to one direction then its working fine. – Senpai May 24 '16 at 16:36
  • whoa, i just realized you are the one that created UndoFX :D. Thanks for your work mate! – Senpai May 24 '16 at 16:48

1 Answers1

2

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.

James_D
  • 201,275
  • 16
  • 291
  • 322
  • Thank you so much, this way it's indeed working fine. Although i'm having some difficulties to include the width and height EventStream to the undoManager. – Senpai May 24 '16 at 23:07
  • Do the same thing, i.e. create a `DimensionChange` and merge event streams from the width and height properties into an event stream of `DimensionChange`s. (Or, if you want to merge changes in both position and dimension, create something like a `BoundsChange` that encapsulates x, y, width, and height.) – James_D May 24 '16 at 23:24
  • 1
    As ever, Tomas' APIs are fun to play around with. [Here's a fuller example](https://gist.github.com/james-d/458f8af01da13fad0a29c8f9fbf622df), dragging and resizing a rectangle, with animated undo/redo, and both repositioning and resizing factored as a change of `BoundingBox`. – James_D May 25 '16 at 02:00
  • Mate, you probably are the most helpful guy in stackoverflow. Thank you so much, for your simple and clean solution! Up to the top! :) – Senpai May 25 '16 at 12:37