🐛 Shape Drawing — Bug Report & Fix Log

Summary of Bugs

All three bugs exist identically in both script.js (vanilla version) and jjSketchDrawBoostrap5Sketch.js (Bootstrap version). The zero-suffix files (script0.js, jjSketchDrawBoostrap5Sketch0.js) are preserved as-is for comparison.

🐛 Bug 1 — Wrong Shape Start Coordinates

Why it happened: The variables lastX and lastY serve a dual purpose in the drawing engine. For the pen tool they act as a "previous point" so each new segment connects to the last drawn point. However, draw() always updates them at the bottom of every mousemove callback:

lastX = x;
lastY = y;

For shape tools (line, circle, rectangle), stopDrawing() was using lastX/lastY as the origin of the shape. Because the mouse had been moving since mousedown, by the time mouseup fired, lastX/lastY had been overwritten hundreds of times and now held the final mouse position — essentially the same point as currentX/currentY. The shape therefore had zero (or near-zero) dimensions.

Buggy Code (stopDrawing() in the zero files):

// lastX/lastY by this point = LAST mousemove position, NOT mousedown position
if (currentType === 'line') {
    redrawCanvas();
    ctx.moveTo(lastX, lastY);   // ← WRONG: both ends are basically the same point
    ctx.lineTo(currentX, currentY);
    ctx.stroke();
}

Fix Applied:

Two new variables, startX and startY, are captured once in startDrawing() and are never modified during a drag. They always hold the mousedown anchor point.

// In startDrawing():
startX = lastX;   // ← captured once at mousedown, never changed
startY = lastY;

// In stopDrawing():
if (currentType === 'line') {
    ctx.putImageData(shapeSnapshot, 0, 0);
    ctx.moveTo(startX, startY);   // ← CORRECT anchor
    ctx.lineTo(currentX, currentY);
    ctx.stroke();
}

🐛 Bug 2 — No Live Preview While Dragging

Why it happened: The draw() function is called on every mousemove. For the pen and eraser tools it draws incrementally. For shape tools the original code contained only empty comment stubs:

Buggy Code (draw() in the zero files):

} else if (currentType === 'line') {
    // Don't draw line in real-time, will be drawn on mouseup     ← nothing happens
} else if (currentType === 'circle') {
    // Preview circle                                              ← nothing happens
} else if (currentType === 'rectangle') {
    // Preview rectangle                                           ← nothing happens
}

The Bootstrap zero file was slightly different but equally broken — shapes were assigned to stopDrawing() via a comment, with no code in draw().

Fix Applied:

A shapeSnapshot (ImageData) is captured synchronously via ctx.getImageData() at the moment of mousedown. On every mousemove, the snapshot is restored and the preview shape is redrawn on top, giving a smooth rubber-band preview.

// In startDrawing() — capture clean snapshot before any preview is drawn:
if (t === 'line' || t === 'circle' || t === 'rectangle') {
    shapeSnapshot = ctx.getImageData(0, 0, canvas.width, canvas.height);
}

// In draw() — restore snapshot then render preview on every mousemove:
} else if (currentType === 'line') {
    ctx.putImageData(shapeSnapshot, 0, 0);   // ← wipe preview from last frame
    ctx.beginPath();
    ctx.moveTo(startX, startY);
    ctx.lineTo(x, y);
    ctx.stroke();
} else if (currentType === 'circle') {
    ctx.putImageData(shapeSnapshot, 0, 0);
    const radius = Math.sqrt(Math.pow(x - startX, 2) + Math.pow(y - startY, 2));
    ctx.beginPath();
    ctx.arc(startX, startY, radius, 0, 2 * Math.PI);
    ctx.stroke();
} else if (currentType === 'rectangle') {
    ctx.putImageData(shapeSnapshot, 0, 0);
    ctx.strokeRect(startX, startY, x - startX, y - startY);
}

🐛 Bug 3 — Async Canvas Restore Race Condition

Why it happened: redrawCanvas() restores the canvas using an Image object and an onload callback — this is asynchronous. In stopDrawing(), the original code called redrawCanvas() and then immediately drew the final shape in the very next line:

Buggy Code (stopDrawing() in the zero files):

if (currentType === 'line') {
    redrawCanvas();            // ← async: img.onload fires LATER
    ctx.moveTo(lastX, lastY);  // ← runs NOW, before restore is complete
    ctx.lineTo(currentX, currentY);
    ctx.stroke();              // ← shape drawn on the CURRENT dirty canvas
    // ...then img.onload fires, clears canvas, shape is GONE
}

The execution order was: draw shape → (milliseconds later) restore canvas. The restore wiped the just-drawn shape. Even if Bugs 1 and 2 were somehow absent, Bug 3 alone would have made shapes disappear on release.

Fix Applied:

redrawCanvas() is replaced by ctx.putImageData(shapeSnapshot, 0, 0) for shape tools. putImageData() is synchronous — it completes before the next line runs, so the restore and the final draw always happen in the correct order.

if (currentType === 'line') {
    ctx.putImageData(shapeSnapshot, 0, 0);  // ← synchronous: completes immediately
    ctx.strokeStyle = colorPicker.value;
    ctx.lineWidth   = brushSize.value;
    ctx.beginPath();
    ctx.moveTo(startX, startY);             // ← correct anchor
    ctx.lineTo(currentX, currentY);
    ctx.stroke();                           // ← shape stays on canvas ✓
}
shapeSnapshot = null;  // release the snapshot

Why does redrawCanvas() use async at all? Because history[] stores canvas state as data URLs (strings, not ImageData). Loading a data URL back onto the canvas requires creating an Image object and waiting for it to decode — hence img.onload. The undo button still uses redrawCanvas() correctly because there is no synchronous drawing immediately after it. For the shape workflow, shapeSnapshot is an ImageData object (raw pixel bytes) so it can be restored instantly with putImageData().

📁 Files Changed

🎓 Teaching Notes

🔗 Open the Apps