Javascript: Canvas drawing - storing mouse coordinates

Question

I have a small JS application that allows a user, after clicking on a canvas, to draw a picture.

Once the user clicks the mouse, the user is allowed to drag the mouse wherever they want within the canvas and a line will be drawn from where they started moving the mouse to where the mouse stopped.

Only by clicking again will the drawing cease.

The code I have for this is below, and one concern of mine is how I'm storing the coordinates of the mouse's position each time it moves.

var draw = false;
var coords = [];
var canvas = document.getElementById('canvas');
var context = canvas.getContext('2d');
canvas.addEventListener('click', function (event) {
 coords = [];
 draw = !draw;
});
canvas.addEventListener('mousemove', function (event) {
 if (draw) {
 context = canvas.getContext("2d");
 var coord = { 'x': event.x - this.offsetLeft, 'y': event.y - thisoffsetTop };
 coords.push(coord);
 var max = coords.length - 1;
 if (typeof coords[max - 1] !== "undefined") {
 var curr = coords[max], prev = coords[max - 1];
 context.beginPath();
 context.moveTo(prev.x, prev.y);
 context.lineTo(curr.x, curr.y);
 context.stroke();
 }
 }
});

I have a feeling that I could be storing the coordinates more efficiently, rather than just add them to an ever-increasing Array. Is this the right approach, or is there a more efficient way of handling this sort of storage?

• Lazar Ljubenović: Do you need the coordinates later for something, like storing them on a server? With the code as-is, you do not need to store all coordinates at all: only the previous one in order to draw the line.
• Delfino: No, I only need the current pair of coordinates and the previous pair. That was my idea, but I'm unsure of how to go about doing that.

Answer 1

Firstly, I want to give you a thumbs up for a clear description of your code. It really helps with reading.

^{_{And another before-we-begin thing: you've got a missing . in your longest line of the code, near the end: - thisoffsetTop };.}}

---

Main changes

Indeed, as you do not need to store all the coordinates, it's not efficient to save them all. To draw a line between the current and previous point, all you need are those two points. So, the first thing we can do with the code is to refactor that away: use prevCoord instead of an array.

Get rid of the array

First, we remove the coords declaration and replace it with a prevCoord. Without adding initialization, it'll stay at undefined for the first round. We're fine with that.

// old
var coords = [];
// new
var prevCoord;

Slightly different condition

We do not need to compute the max anymore -- the original code uses it to grab the "current" and "previous" coordinate, but we're moving away from that. Also, the condition is now different. We just need to ask if prevCoord is undefined.

// old
var max = coords.length - 1;
if (typeof coords[max - 1] !== "undefined") {
// new
if (typeof prevCoord !== "undefined") {

Simplify the contents of if

Let's see what happens inside. Before we incorporate our change, let's notice that in your first line within if, you've got an expression coords[max]. That's actually the element you've just push'd, right? So we can replace coords[max] with just coord (the current coordinate).

Then, we also not need the prev = coords[max - 1] part because we already have the previous coordinate stored in prevCoord.

In other words, that line's a goner because we have the refs we need at hand.

// old
var curr = coords[max], prev = coords[max - 1];
// new
// not needed: curr is coord and prev is prevCoord

Now, the drawing commands become the following:

// old
context.moveTo(prev.x, prev.y);
context.lineTo(curr.x, curr.y);
// new
context.moveTo(prevCoord.x, prevCoord.y);
context.lineTo(coord.x, coord.y);

Store the current as previous

Now, the main part.

The idea is not that far from what you have here. Instead of pushing into array, you can just overwrite the prevCoord:

prevCoord = coord

Of course, you cannot do it as soon as you grab the coord from the event and offset stuff -- you need the prevCoord to draw the line. So, just overwrite it after drawing.

// old
coords.push(coord);
if (typeof prevCoord !== "undefined") {
 // ...
}
// new
if (typeof prevCoord !== "undefined") {
 // ...
}
prevCoord = coord;

See -- just after using the prevCoord to draw a line between previous and current, we don't need the prevCoord anymore. We can use this variable to prepare for the next cycle: on next mouse move, the "current" will become "previous"... meaning that when we run this function the next time, prevCoord will be just what it needs to be: the coordinate that was "mouse overed" the previous time.

Polish up

There's one more place let to change: the click handler. Just as you reset your array, you should reset the prevCoord back to undefined.

// old
coords = [];
// new
prevCoord = undefined;

Full code

Here's the complete deal.

var draw = false;
var prevCoord;
var canvas = document.getElementById('canvas');
var context = canvas.getContext('2d');
canvas.addEventListener('click', function (event) {
 prevCoord = undefined
 draw = !draw;
});
canvas.addEventListener('mousemove', function (event) {
 if (draw) {
 context = canvas.getContext("2d");
 var coord = { 'x': event.x - this.offsetLeft, 'y': event.y - this.offsetTop };
 if (typeof prevCoord !== "undefined") {
 context.beginPath();
 context.moveTo(prevCoord.x, prevCoord.y);
 context.lineTo(coord.x, coord.y);
 context.stroke();
 }
 prevCoord = coord
 }
});

<canvas id="canvas" style="border: 1px solid red"></canvas>

More suggestions

A couple of things...

Redefining context

You have, on your fourth line, in the global scope a var context = ..., and then inside the if (draw) you have a context = .... There's no need to run that each time. The context doesn't change.

// old
if (draw) {
 context = canvas.getContext("2d");
// new
if (draw)

Check for undefined

You don't need to use typeof x != 'undefined' here. It's used when you're not sure if x has been declared. A simple check would do.

// old
if (typeof prevCoord !== "undefined") {
// new
if (prevCoord !== undefined) {

---

That's all!

• Delfino: Thank you for the solution. To be honest, I didn't notice much performance improvement, but I always have this nagging voice asking "could this be done better?". Also, I'm still unsure what you're trying to say in the 'Check for undefined' section. Is one if statements, performance-wise, better?
• Lazar Ljubenović: The typeof way is useful when you're afraid that accessing something could throw, without caring if the value is undefined or not. It's usually used by frameworks when testing for presence of a library or environment. For example, a plugin could ask typeof window != 'undefined' to check if it's executed in browser or typeof $ != 'undefined' to check if you're using jQuery. If they used $ != null, it'd break the script if you're not using jQuery. In your example, you know your code (you've got var prevCoord), so it's weird to use that pattern.
• Lazar Ljubenović: You wouldn't see any performance issues with your old approach, by the way. It's more about memory. With a very large canvas and a very persistent user, after some hours you could start accumulating gigabytes which would use up all RAM. It's more of a theoretical than practical problem, to be fair. That said, if you were storing more stuff, it might turn into an issue sooner than expected. It's worth knowing the prev/curr trick with swapping variables -- it's a common one.

Answer 2

Inside the if (typeof coords[max - 1] !== "undefined") loop, adding the following two lines will ensure that only two pairs of coordinates are kept inside coords when drawing

if (typeof coords[max - 1] !== "undefined") {
 var curr = coords[max], prev = coords[max - 1];
 context.beginPath();
 context.moveTo(prev.x, prev.y);
 context.lineTo(curr.x, curr.y);
 context.stroke();
 coords = [];
 coords.push(curr);
}

The reason it's necessary to add curr into the array is because on the next mouse move curr will be prev and therefore the line will be continuous. Adding just the line

if (typeof coords[max - 1] !== "undefined") {
 var curr = coords[max], prev = coords[max - 1];
 context.beginPath();
 context.moveTo(prev.x, prev.y);
 context.lineTo(curr.x, curr.y);
 context.stroke();
 coords = [];
}

Would cause the drawn line to appear spotty, as after a line is drawn, it will only draw again after two more mouse moves.

Note: this is the first time I've answered my own question, so please let me know if there are aspects of the answer I could improve upon