javamultidimensional-arrayvectorundo-redoimage-editor

Java - Cannot gather 2D array inside vector


As I mentioned in my previous (not ansvered) question, I'm making a image editor that can be embeded to my program. So I came to a point, where I decided that image editor must have undo and redo. So I wrote this code:

public Vector<Color[][]> undo = new Vector<Color[][]>();
size = 16;
public Color[][] chroma = new Color[size][size];
//These are variables

public void saveRevision(){
    System.out.println("Saving: " + current);
    undo.insertElementAt(chroma.clone(), current);
    current++;
    /*for (int i = 0; i < (undo.size()-1); i++) {
        if(i > current){
        undo.remove(i);
        }
    }*/
}

public void undo(){
    if(current > 0){
        System.out.println("Gathering: " + (current-1));
        current--;
        /*Color[][] c = undo.get(current);
        for (int i = 0; i < c.length; i++) {
            for (int j = 0; j < c[i].length; j++) {
                System.out.print(c[i][j]);
            }
            System.out.println();
        }*/
        /*for(int i = 0; i < size; i++){
            for(int j = 0; j < size; j++){
                chroma[i][j] = null;
            }
        }*/
        chroma = undo.get(current);
        repaint();
    }
}

public void redo(){
    if((undo.size()-1) > current){
        System.out.println("Gathering: " + (current+1));
        current++;
        for(int i = 0; i < size; i++){
            for(int j = 0; j < size; j++){
                chroma[i][j] = null;
            }
        }
        chroma = undo.get(current);
        repaint();
    }
}

The problem is that I somehow cant write inside an array from that vector that has all the chroma revisions. As you can see, I tried everything, but "chroma" variable does not seem to be changed. Em I doing something wrong?

Just forgot to mention: Undo and Redo are triggered using JButton, Revision is made everytime the mouse button is released.


Solution

  • The problem might be, that for undo-redo to work the undo queue has to contain immutable data.

    This means that every change (atomic group of changes) to chroma would need a new 2D array copy; expensive.

    The solution is to keep only the change actions in the undo/redo queue.

    In Java there is the AbstractAction which implements the Action interface, and as example for edits: UndoManager. Maybe do a bit of research on that.

    (Also abstain from Vector, an ArrayList will not give comments.)


    A bit better explained:

    chroma[][] is the current state. For an undo/redo facility one need to change this state to an earlier one. Now one could store for every point in time with an undo to a prior moment the entire state, a copy of chroma. A better way could be to store the change between two states. So if chroma[47][11] was changed from A to B store just the action pair:

    undo: chroma[47][11] = A;
    redo: chroma[47][11] = B;
    

    This (more effort needing) alternative is often more practical.

    It might be that the original choice is more direct: saving copies of chroma. Then assigning to chroma by some user action, should never change a saved state: the copies must be immutable = unchangeable, its internal state not depending on something changeable.

    So, the copy ie need because of:

    Color[][] chroma = ...;
    Color[][] falseCopied = chroma;
    chroma[3][4] = c;
    // Now also `falseCopied[3][4].equals(c)`.
    

    A real copy can be done:

    public static Color copy(Color[][] grid) {
        Color[][] copied = new Color[grid.length][];
        for (int i = 0; i < grid.length; ++i) {
            copied[i] = Arrays,copyOf(grid[i], grid[i].length);
        }
        return copied;
    }
    

    Here one does not need to make a copy of every Color itself, because Color is "immutable", i.e. you cannot change its internal state. Something like color.setBlue(23) does not exist.