Undo / Redo Not Working Properly And Painting After Zoom Not Working Properly Too

I am trying to implement a paint bucket tool with undo and redo functionality. The issue is that undo and redo are working properly the first time, but when I do undo redo multiple

Solution 1:

Canvas sizes & State History

Canvas size

If you have ever had a look around in the DOM you will notice that many element have both a height and width as an attribute and a height and a width as a style attribute.

For the canvas these have two different meanings. So lets create a canvas.

var canvas = document.createElement("canvas");

Now the canvas element width and height can be set. This defines then number of pixels in the canvas image (the resolution)

canvas.width = 500;canvas.height = 500;

By default when an image (canvas is just an image) is displayed in the DOM it is displayed with a one to one pixel size. That means that for each pixel in the image there is one pixel on the page.

You can change this by setting the canvas style width and height

canvas.style.width = "1000px"; // Note you must add the unit type "px" in this casecanvas.style.width = "1000px";

This does not change the canvas resolution, just the display size. Now for each pixel in the canvas it takes up 4 pixels on the page.

This becomes a problem when you are using the mouse to draw to the canvas as the mouse coordinates are in screen pixels that no longer match the canvas resolution.

To fix this. And as an example from the OP code. You need to rescale the mouse coordinates to match the canvas resolution. This has been added to the OP mousedown event listener. It first gets the display width/height then the resolution width and height. It normalises the mouse coords by dividing by the display width/height. This brings the mouse coords to a range of 0 <= mouse < 1 which then we multiply to get the canvas pixel coordinates. As the pixels need to be at integer locations (whole numbers) you must floor the result.

// assuming that the mouseX and mouseY are the mouse coords.if(this.style.width){   // make sure there is a width in the style // (assumes if width is there then height will be toovar w = Number(this.style.width.replace("px",""));  // warning this will not work if size is not in pixelsvar h = Number(this.style.height.replace("px","")); // convert the height to a numbervar pixelW = this.width;  // get  the canvas resolutionvar pixelH = this.height;
    mouseX = Math.floor((mouseX / w) * pixelW); // convert the mouse coords to pixel coords
    mouseY = Math.floor((mouseY / h) * pixelH);
}

That will fix your scaling problem. But looking at your code, its is a mess and you should not be searching the nodetree each time, re getting the context. I am surprised it works, but that might be Jquery (I don't know as I never use it) or might be that you are rendering elsewhere.

State History

The current state of a computer program is all the conditions and data that define the current state.. When you save something you are saving a state, and when you load you restore the state.

History is just a way of saving and loading states without the messing around in the file system. It has a few conventions that say that the stats are stored as a stack. The first in is the last out, it has a redo stack that allows you to redo previous undos but to maintain the correct state and because states are dependent on previous states the redo can only redo from associated states. Hence if you undo and then draw something you invalidate any existing redo states and they should be dumped.

Also the saved state, be it on disk, or undo stack must be dissociated from the current state. IF you make changes to the current state you do not want those changes to effect the saved state.

This I think is where you went wrong OP, as you were using the colorLayerData to fill (paint) when you got a undo or redo you where using the referenced data that remained in the undo/redo buffers thus when you painted you actually were changing the data still in the undo buffer.

History Manager

This is a general purpose state manager and will work for any undo/redo needs, all you have to do is ensure that you gather the current state into a single object.

To help I have written a simple history manager. It has two buffers as stacks one for undos and one for redos. It also holds the current state, which is the most recent state it knows about.

When you push to the history manager it will take the current state it knows about and push it to the undo stack, save the current state, and invalidate any redo data (making the redo array length 0)

When you undo it will push the current state onto the redo stack, pop a state from the undo stack and put it in current state, then it will return that current state.

When you redo it will push the current state onto the undo stack, pop a state from the redo stack and put it in current state, then it will return that current state.

It is important that you make a copy of the state returned from the state managers so that you do not inadvertently change the data stored in the buffers.

You may ask. "why cant the state manager ensure that the data is a copy?" A good question but this is not the role of a state manager, it saves states and it must do so no matter what it has to save, it is by nature completely unaware of the meaning of the data it stores. This way it can be used for images, text, game states, anything, just as the file system can, it can not (should not) be aware of the meaning and thus know how to create meaningful copies. The data you push to the state manager is just a single referance (64bits long) to the pixel data or you could push each byte of the pixel data, it does not know the difference.

Also OP I have added some UI control to the state manager. This allows it to display its current state Ie disables and enables the undo redo buttons. Its always important for good UI design to provide feedback.

The code

You will need to make all the following changes to your code to use the history manager. You can do that or just use this as a guide and write your own. I wrote this before I detected your error. If that is the only error then you may only need to change.

// your old code (from memory)
   colorLayerData = undoArr.pop(); 
   context.putImageData(colorLayerData, 0, 0);  


   // the fix same applies to redo and just makes a copy rather than use // the reference that is still stored in the undoe buff
   context.putImageData(undoArr, 0, 0);   // put the undo onto the canvas
   colorLayerData = context.getImageData(0, 0, canvasWidth, canvaHeight); 

Remove all the code you have for the undo/redo.

Change the undo/redo buttons at top of page to, with a single function to handle both events.

    <button id = "undo-button" onclick="history('undo')">Undo</button>
    <button id = "redo-button" onclick="history('redo')">Redo</button>

Add the following two functions to you code

        function history(command){ // handles undo/redo button events.vardata;
            if(command === "redo"){
                data = historyManager.redo(); // get data for redo
            }elseif(command === "undo"){
                data = historyManager.undo(); // get data for undo
            }
            if(data !== undefined){ // if data has been found
                setColorLayer(data); // set the data
            }
        }

        // sets colour layer and creates copy into colorLayerData
        function setColorLayer(data){
            context.putImageData(data, 0, 0);  
            colorLayerData = context.getImageData(0, 0, canvasWidth, canvasHeight);
            context.drawImage(backgroundImage, 0, 0, canvasWidth, canvasHeight);
            context.drawImage(outlineImage, 0, 0, drawingAreaWidth, drawingAreaHeight);            
        }

In the redraw function you have replace the stuff you had for undo and add this line at the same spot. This saves the current state in the history manager.

           historyManager.push(context.getImageData(0, 0, canvasWidth, canvasHeight));

In the start function you have to add the UI elements to he state manager. This is up to you and can be ignored the stat manager will just ignore them if they are not defined.

if(historyManager !== undefined){
                // only for visual feedback and not required for the history manager to function.
                historyManager.UI.assignUndoButton(document.querySelector("#undo-button"));
                historyManager.UI.assignRedoButton(document.querySelector("#redo-button"));
            }

And off course the historyManager its self. It encapsulates the data so that you can not access its internal state except via the interface provides.

The historyManager (hM) API

• hM.UI The ui manager just updates and assigns button disabled/enabled states
• hM.UI.assignUndoButton(element) set the undo element
• hM.UI.assignRedoButton(element) set the redo element
• nM.UI.update() Updates the button states to reflect the current internal state. All internal states are automatically call this so only needed if you are changing the redo/undo buttons stats your self
• hM.reset() Resets the history manager clearing all the stacks and current saved states. Call this when you load or create a new project.
• nM.push(data) Add the provided data to the history.
• nM.undo() get the previous history state and return the data stored. If no data then this will return undefined.
• nM.redo() get the next history state and return the data stored. If no data then this will return undefined.

The self invoking function creates the history manager, the interface is accessed via the variable historyManager

var historyManager = (function (){  // Anon for private (closure) scopevar uBuffer = []; // this is undo buffvar rBuffer = []; // this is redo buffvar currentState = undefined; // this holds the current history statevar undoElement = undefined;
    var redoElement = undefined;
    var manager = {
        UI : {  // UI interface just for disable and enabling redo undo buttons
            assignUndoButton : function(element){
                undoElement = element;
                this.update();
            },
            assignRedoButton : function(element){
                redoElement = element;
                this.update();
            },
            update : function(){
                if(redoElement !== undefined){
                    redoElement.disabled = (rBuffer.length === 0);
                }
                if(undoElement !== undefined){
                    undoElement.disabled = (uBuffer.length === 0);                                
                }
            }
        },
        reset : function(){
            uBuffer.length = 0;
            rBuffer.length = 0;
            currentState = undefined;
            this.UI.update();
        },
        push : function(data){
            if(currentState !== undefined){
                uBuffer.push(currentState);                        
            }
            currentState = data;
            rBuffer.length = 0;
            this.UI.update();
        },
        undo : function(){
           if(uBuffer.length > 0){
               if(currentState !== undefined){
                    rBuffer.push(currentState);                        
                }
                currentState = uBuffer.pop();
            }
            this.UI.update();
            return currentState; // return data or unfefined
        },
        redo : function(){
            if(rBuffer.length > 0){
               if(currentState !== undefined){
                    uBuffer.push(currentState);                        
                }
                currentState = rBuffer.pop();
            }
            this.UI.update();    
            return currentState;
        },
    }
    return manager;
})();

That will fix your Zoom problem and the undo problem. Best of luck with your project.

Solution 2:

This function matchOutlineColor takes in 4 numbers that represent a RGBA color.

Red, Green, Blue, Alpha (how transparent the color is)

RGBA colors range from 0-255, thus being from 0(no color) to 255(full color) with white being rgba(255,255,255,255), black being rgba(0,0,0,255) and transparent being rgba(0,0,0,0).

This code doesn't check to see if a color is black, just that the red + green + yellow colors added together are at least less than 100(out of a total of 750). I suspect the function checks if the color is a dark color.

For example this will all pass true:

<divstyle="background-color:rgba(99,0,0,255)">Dark RED</div><divstyle="background-color:rgba(0,99,0,255)">Dark GREEN</div><divstyle="background-color:rgba(0,0,99,255)">Dark BLUE</div>

If you want to check if the border is black you can change the function to

functionmatchOutlineColorBlack(r, g, b, a) {
    //Ensures red + green + blue is nonereturn (r + g + b == 0 && a === 255);
};

functionmatchOutlineColorWhite(r, g, b, a) {
    //Checks that color is white (255+255+255=750)return (r + g + b == 750 && a === 255);
};