Javascript - fill() is filling using the wrong colour

Question

I'm drawing on an HTML canvas using Javascript. I'm having an issue where one of my shapes isn't getting filled with the correct colour. It's a complicated situation but I'll do my best to simplify it.

The shape that is causing me grief is representing a plane on a radar screen. I have a diamond (which I draw using lineTo commands), a rectangle which will hold some information about that plane, and a line connecting the two. The plane has a property selected, which is set to false by default, but if you click on the diamond or rectangle, it selects that plane and sets its selected property to true.

When the plane is not selected, I want the shape to be drawn with a red outline, and filled with a black background. This is currently working. However, when the plane is selected, I want the diamond to be filled in with red. I have coded this scenario but when I click on a plane to select it, the fill colour of the diamond stays black, and the border gets bigger.

I can't figure out why this is happening. I'm guessing it has something to do with the previous settings of the canvas carrying over from the last time I drew a shape, but I set these values each time, so I can't work it out.

I have created a JSFiddle here: JSFiddle and here is the same code snippet

//---------------------------- MARK: Declarations ----------------------------\\ 
var canvas = document.getElementById("gameCanvas");
//canvas.width = window.innerWidth;
//canvas.height = window.innerHeight;
canvas.width = 1280;
canvas.height = 627;
var ctx = canvas.getContext("2d");

var timer = 0;
var seconds = 0;
var textToDisplay = "";
var strLevel = "";
var intPlaneInfoBoxSelected = -1;
var intPlaneInfoBoxSelectedXOffset = 0;
var intPlaneInfoBoxSelectedYOffset = 0;
var border = new Border();
var intLessWidth = 0;
var intLessHeight = 0;
var aRunways = [];
var aWayPoints = [];
var aPlanes = [];
var aAircraftTypes = [];
var aAirlineName = [];

var colourBrightGreen = "#1EFF00";
var colourWaypointBorder = "#FFFFFF";
var colourWaypointBackground = "#188C08";
var intWaypointLineWidth = 5;
var intRunwayWaypointLineWidth = 4;
var intInfoBoxLineWidth = 3;
var intEntrySpeed = 0.1;

//---------------------------- MARK: Object Creation ----------------------------\\

function WayPoint() {
  this.topLeft = new Point();
  this.widthHeight = 15;
  this.name = "";
  this.inbound = false;
  this.outbound = false;
  this.border = "";
  this.highlighted = false;
}

function Runway() {
  this.topLeft = new Point();
  this.height = 0;
  this.width = 0;
  this.highlighted = false;
  this.name = "";
  this.wayPointTopLeft = new Point();
  this.wayPointWidthHeight = 0;
}

function Plane() {
  this.flightNumber = "";
  this.status = "";
  this.selected = false;
  this.airRoute = [];
  this.heading = 0;
  this.speed = 0;
  this.topPoint = new Point();
  this.widthHeight = 20;
  this.trailing1TopLeft = new Point();
  this.trailing2TopLeft = new Point();
  this.trailing3TopLeft = new Point();
  this.trailing4TopLeft = new Point();
  this.infoBoxTopLeft = new Point();
  this.infoBoxWidth = 60;
  this.infoBoxHeight = 30;
  this.dx = 3;
  this.dy = 3;
}

function AircraftType() {
  this.type = "";
  this.minSpeed = 0;
}

function Point() {
  this.x = 0;
  this.y = 0;
}

function Border() {
  this.topLeft = new Point();
  this.width = 0;
  this.height = 0;
  this.borderTop = 0;
  this.borderBottom = 0;
  this.borderLeft = 0;
  this.borderRight = 0;
  this.lineThickness = 10;
  this.lineColour = "#1EFF00";
}

//---------------------------- MARK: Event Listeners ----------------------------\\

document.addEventListener("click", mouseClickHandler, false);
document.addEventListener("mousedown", mouseDownHandler, false);
document.addEventListener("mouseup", mouseUpHandler, false);
document.addEventListener("mousemove", mouseMoveHandler, false);

function mouseClickHandler(e) {
  // Get Mouse Position
  var rectCanvas = canvas.getBoundingClientRect();
  var positionX = e.clientX;
  var positionY = e.clientY;

  var booClickedOnWaypoint = false;
  var booClickedOnRunway = false;
  var booClickedOnPlane = false;

  for (i = 0; i < aPlanes.length; i++) {
    var intPlaneLeft = aPlanes[i].topPoint.x - (aPlanes[i].widthHeight / 2);
    var intPlaneRight = aPlanes[i].topPoint.x + (aPlanes[i].widthHeight / 2);
    var intPlaneTop = aPlanes[i].topPoint.y;
    var intPlaneBottom = aPlanes[i].topPoint.y + aPlanes[i].widthHeight;

    var intInfoBoxLeft = aPlanes[i].infoBoxTopLeft.x - (intInfoBoxLineWidth / 2);
    var intInfoBoxRight = aPlanes[i].infoBoxTopLeft.x + aPlanes[i].infoBoxWidth + (intInfoBoxLineWidth / 2);
    var intInfoBoxTop = aPlanes[i].infoBoxTopLeft.y - (intInfoBoxLineWidth / 2);
    var intInfoBoxBottom = aPlanes[i].infoBoxTopLeft.y + aPlanes[i].infoBoxHeight + (intInfoBoxLineWidth / 2);

    if (((positionX >= intPlaneLeft) && (positionX <= intPlaneRight) && (positionY >= intPlaneTop) && (positionY <= intPlaneBottom)) || ((positionX >= intInfoBoxLeft) && (positionX <= intInfoBoxRight) && (positionY >= intInfoBoxTop) && (positionY <= intInfoBoxBottom))) {
      aPlanes[i].selected = true;
      booClickedOnPlane = true;
    } else {
      aPlanes[i].selected = false;
    }
  }


}

function mouseDownHandler(e) {
  var positionX = e.clientX;
  var positionY = e.clientY;

  for (i = 0; i < aPlanes.length; i++) {
    var intInfoBoxLeft = aPlanes[i].infoBoxTopLeft.x - (intInfoBoxLineWidth / 2);
    var intInfoBoxRight = aPlanes[i].infoBoxTopLeft.x + aPlanes[i].infoBoxWidth + (intInfoBoxLineWidth / 2);
    var intInfoBoxTop = aPlanes[i].infoBoxTopLeft.y - (intInfoBoxLineWidth / 2);
    var intInfoBoxBottom = aPlanes[i].infoBoxTopLeft.y + aPlanes[i].infoBoxHeight + (intInfoBoxLineWidth / 2);

    if ((positionX >= intInfoBoxLeft) && (positionX <= intInfoBoxRight) && (positionY >= intInfoBoxTop) && (positionY <= intInfoBoxBottom)) {
      intPlaneInfoBoxSelected = i;
      intPlaneInfoBoxSelectedXOffset = positionX - aPlanes[i].infoBoxTopLeft.x;
      intPlaneInfoBoxSelectedYOffset = positionY - aPlanes[i].infoBoxTopLeft.y;
    }
  }
}

function mouseUpHandler(e) {
  intPlaneInfoBoxSelected = -1;
}

function mouseMoveHandler(e) {
  var positionX = e.clientX;
  var positionY = e.clientY;

  if (intPlaneInfoBoxSelected > -1) {
    aPlanes[intPlaneInfoBoxSelected].infoBoxTopLeft.x = positionX - intPlaneInfoBoxSelectedXOffset;
    aPlanes[intPlaneInfoBoxSelected].infoBoxTopLeft.y = positionY - intPlaneInfoBoxSelectedYOffset;
  }
}

//---------------------------- MARK: Setup ----------------------------\\

function SetupArrays() {}

function SetupLevel() {
  if (strLevel == "YSSY") {

    //Waypoints
    addWaypoint("LEFT", {
      x: border.borderLeft,
      y: 100
    }, true, false);
  }
}

function SetupCanvas() {
  strLevel = "YSSY";
}

function LoadAircraftTypes() {

}

function LoadAirlineNames() {}

//---------------------------- MARK: Draw Existing Things ----------------------------\\

function drawPlanes() {
  for (i = 0; i < aPlanes.length; i++) {

    // Line
    ctx.lineWidth = 4;
    ctx.fillStyle = 'red';
    ctx.strokeStyle = 'red';
    ctx.beginPath();
    ctx.moveTo((aPlanes[i].topPoint.x), (aPlanes[i].topPoint.y + (aPlanes[i].widthHeight / 2)));
    ctx.lineTo((aPlanes[i].infoBoxTopLeft.x + (aPlanes[i].infoBoxWidth / 2)), (aPlanes[i].infoBoxTopLeft.y + (aPlanes[i].infoBoxHeight / 2)));
    ctx.stroke();

    // Plane
    ctx.fillStyle = 'red';
    ctx.strokeStyle = 'red';

    if (aPlanes[i].selected == true) {
      var pointX = aPlanes[i].topPoint.x;
      var pointY = aPlanes[i].topPoint.y;
      var width = aPlanes[i].widthHeight;

      ctx.beginPath();
      ctx.moveTo(pointX, pointY);
      ctx.lineTo(pointX + (width / 2), pointY + (width / 2));
      ctx.lineTo(pointX, pointY + width);
      ctx.lineTo(pointX - (width / 2), pointY + (width / 2));
      ctx.closePath();
      ctx.fill();
    } else {
      var lineThickness = 3;
      var pointX = aPlanes[i].topPoint.x;
      var pointY = aPlanes[i].topPoint.y + lineThickness;
      var width = aPlanes[i].widthHeight - (lineThickness * 2);

      ctx.beginPath();
      ctx.lineWidth = lineThickness;
      ctx.moveTo(pointX, pointY);
      ctx.lineTo(pointX + (width / 2), pointY + (width / 2));
      ctx.lineTo(pointX, pointY + width);
      ctx.lineTo(pointX - (width / 2), pointY + (width / 2));
      ctx.closePath();
      ctx.stroke();
    }

    // Flight Information Box
    ctx.rect(aPlanes[i].infoBoxTopLeft.x, aPlanes[i].infoBoxTopLeft.y, aPlanes[i].infoBoxWidth, aPlanes[i].infoBoxHeight);
    ctx.strokeStyle = 'red';
    ctx.fillStyle = 'black';
    ctx.lineWidth = clone(intInfoBoxLineWidth);
    ctx.fill();
    ctx.stroke();
  }
}






















function drawRunways() {

}

function drawText() {

}

function DrawWaypoints() {

}

function DrawCanvas() {

}

//---------------------------- MARK: Create Motion ----------------------------\\

function movePlanes() {
  //TODO
  if (aPlanes.length > 0) {
    for (i = 0; i < aPlanes.length; i++) {
      aPlanes[i].topPoint.x += aPlanes[i].dx;
      aPlanes[i].infoBoxTopLeft.x += aPlanes[i].dx;
      aPlanes[i].topPoint.y += aPlanes[i].dy;
      aPlanes[i].infoBoxTopLeft.y += aPlanes[i].dy;

    }
  }

}

//---------------------------- MARK: Collision Detection ----------------------------\\

function detectLanded() {

}

//---------------------------- MARK: Create New Things ----------------------------\\

function addPlane() {
  var tempPlane = new Plane();

  var astrInboundOutbound = ["Inbound", "Outbound"];
  var strRouteDirection = astrInboundOutbound[random(astrInboundOutbound.length)];

  strRouteDirection = "Inbound";

  if (strRouteDirection == "Inbound") {
    // Start at an inbound waypoint
    var astrInboundWaypoints = [];
    for (var i = 0; i < aWayPoints.length; i++) {
      if (aWayPoints[i].inbound == true) {
        astrInboundWaypoints.push(aWayPoints[i]);
      }
    };
    var intArrayPosition = random(astrInboundWaypoints.length);
    var selectedWaypoint = astrInboundWaypoints[intArrayPosition];

    tempPlane.topPoint = clone(selectedWaypoint.topLeft);

    tempPlane.topPoint.x += (selectedWaypoint.widthHeight / 2);
    tempPlane.topPoint.y += ((selectedWaypoint.widthHeight - tempPlane.widthHeight) / 2);

    switch (selectedWaypoint.border) {
      case "Left":
        tempPlane.dx = intEntrySpeed;
        tempPlane.dy = 0;
        break;
      case "Right":
        tempPlane.dx = -1 * intEntrySpeed;
        tempPlane.dy = 0;
        break;
      case "Top":
        tempPlane.dx = 0;
        tempPlane.dy = intEntrySpeed;
        break;
      case "Bottom":
        tempPlane.dx = 0;
        tempPlane.dy = -1 * intEntrySpeed;
        break;
    }
  } else {
    // Start at the upwind end of a runway
    // TODO
  }

  tempPlane.infoBoxTopLeft = {
    x: (tempPlane.topPoint.x - (tempPlane.infoBoxWidth / 2)),
    y: (tempPlane.topPoint.y - tempPlane.infoBoxHeight - 20)
  };

  aPlanes.push(tempPlane);
}

function addRunway(name, topLeft, width, height) {


}

function addWaypoint(name, topLeft, inbound, outbound) {
  var tempWaypoint = new WayPoint();
  tempWaypoint.name = name;
  tempWaypoint.topLeft = topLeft;
  tempWaypoint.inbound = inbound;
  tempWaypoint.outbound = outbound;

  if (tempWaypoint.topLeft.x == border.borderLeft) {
    tempWaypoint.border = "Left";
    tempWaypoint.topLeft.x = border.borderLeft - (tempWaypoint.widthHeight / 2);
  }

  if (tempWaypoint.topLeft.y == border.borderTop) {
    tempWaypoint.border = "Top";
    tempWaypoint.topLeft.y = border.borderTop - (tempWaypoint.widthHeight / 2);
  }

  if (tempWaypoint.topLeft.x == border.borderRight) {
    tempWaypoint.border = "Right";
    tempWaypoint.topLeft.x = border.borderRight - (tempWaypoint.widthHeight / 2);
  }

  if (tempWaypoint.topLeft.y == border.borderBottom) {
    tempWaypoint.border = "Bottom";
    tempWaypoint.topLeft.y = border.borderBottom - (tempWaypoint.widthHeight / 2);
  }

  if (tempWaypoint.border != "") {
    // Plus half the line width one all sides
    tempWaypoint.topLeft.x -= (intWaypointLineWidth / 2);
    tempWaypoint.topLeft.y -= (intWaypointLineWidth / 2);
    tempWaypoint.widthHeight += intWaypointLineWidth;
  }

  aWayPoints.push(tempWaypoint);
}

//---------------------------- MARK: Timer ----------------------------\\

function timerTick() {
  timer += 1;
  if (timer % 1000 == 0) {
    seconds++;
  }

  if (timer == 1) {
    SetupArrays();
    SetupCanvas();
    SetupLevel();
  }

  if (timer == 300) {
    addPlane();
  }

  /*if(timer==5){
	        document.location.reload();
		}*/
}

//---------------------------- MARK: Supplimentary Routines ----------------------------\\



function random(intNumber) {
  return Math.floor((Math.random() * intNumber));
}


function getTextWidth(text, font) {
  ctx.font = font;
  var metrics = ctx.measureText(text);
  return metrics.width;
}

function getTextHeight(text, font) {
  ctx.font = font;
  var metrics = ctx.measureText(text);
  return metrics.height;
}

function clone(obj) {
  // Handle the 3 simple types, and null or undefined
  if (null == obj || "object" != typeof obj) return obj;

  // Handle Date
  if (obj instanceof Date) {
    var copy = new Date();
    copy.setTime(obj.getTime());
    return copy;
  }

  // Handle Array
  if (obj instanceof Array) {
    var copy = [];
    for (var i = 0, len = obj.length; i < len; i++) {
      copy[i] = clone(obj[i]);
    }
    return copy;
  }

  // Handle Object
  if (obj instanceof Object) {
    var copy = {};
    for (var attr in obj) {
      if (obj.hasOwnProperty(attr)) copy[attr] = clone(obj[attr]);
    }
    return copy;
  }

  throw new Error("Unable to copy obj! Its type isn't supported.");
}


//---------------------------- MARK: DRAW ----------------------------\\

function draw() {
  // All movement and display
  // Call functions to make things happen, listeners will work on their own, no need to call

  ctx.clearRect(0, 0, canvas.width, canvas.height);
  movePlanes();
  drawPlanes();

  requestAnimationFrame(draw);
}

//---------------------------- MARK: Other ----------------------------\\

setInterval(timerTick, 1);
draw();

//---------------------------- END OF SCRIPT ----------------------------\\

<canvas id="gameCanvas"></canvas>

The function that draws the plane is called drawPlanes(), and I have put in a dozen or so blank lines after it so you can reach it quickly when scrolling.

You will see that when you run the code, the plane shape starts 'flying' from the left to right, and that it is in its unselected state. However, clicking on it to select it doesn't fill it in like it should, it just enlarges the shape size and moves the border out.

Can anyone help me out?

Answer 1

Turns out this issue had quite a simple fix. My problem was that I wasn't using beginPath() or closePath() correctly. I was able to fix my issues by putting a beginPath() before modifying the parameters for each shape, and a closePath() when I'd finished drawing it.

Here is my code after fixing the issue:

        // Line
        ctx.beginPath();
        ctx.lineWidth = 4;
        ctx.fillStyle = 'red';
        ctx.strokeStyle = 'red';
        ctx.moveTo((aPlanes[i].topPoint.x), (aPlanes[i].topPoint.y + (aPlanes[i].widthHeight / 2)));
        ctx.lineTo((aPlanes[i].infoBoxTopLeft.x + (aPlanes[i].infoBoxWidth / 2)), (aPlanes[i].infoBoxTopLeft.y + (aPlanes[i].infoBoxHeight / 2)));
        ctx.stroke();
        ctx.closePath();

        // Plane
        ctx.beginPath();
        ctx.fillStyle = 'red';
        ctx.strokeStyle = 'red';

        if(aPlanes[i].selected == true) {
            var pointX = aPlanes[i].topPoint.x;
            var pointY = aPlanes[i].topPoint.y;
            var width = aPlanes[i].widthHeight;

            ctx.beginPath();
            ctx.moveTo(pointX, pointY);
            ctx.lineTo(pointX + (width / 2), pointY + (width / 2));
            ctx.lineTo(pointX, pointY + width);
            ctx.lineTo(pointX - (width / 2), pointY + (width / 2));
            //ctx.closePath();
            ctx.fill();
        } else {
            var lineThickness = 3;
            var pointX = aPlanes[i].topPoint.x;
            var pointY = aPlanes[i].topPoint.y + lineThickness;
            var width = aPlanes[i].widthHeight - (lineThickness * 2);

            ctx.beginPath();
            ctx.lineWidth = lineThickness;
            ctx.moveTo(pointX, pointY);
            ctx.lineTo(pointX + (width / 2), pointY + (width / 2));
            ctx.lineTo(pointX, pointY + width);
            ctx.lineTo(pointX - (width / 2), pointY + (width / 2));
            //ctx.closePath();
            ctx.stroke();
        }
        ctx.closePath();

        // Flight Information Box
        ctx.beginPath();
        ctx.rect(aPlanes[i].infoBoxTopLeft.x, aPlanes[i].infoBoxTopLeft.y, aPlanes[i].infoBoxWidth, aPlanes[i].infoBoxHeight);
        ctx.strokeStyle = 'red';
        ctx.fillStyle = 'black';
        ctx.lineWidth = clone(intInfoBoxLineWidth);
        ctx.fill();
        ctx.stroke();
        ctx.closePath();

Hopefully this will help someone!

Answer 2

Not 100% sure what you are trying to do so this is my best guess.

You missed a ctx.beginPath() at the bottom of the draw function. This caused the diamond to be rerendered. And in the selected if clause you were not calling fill, and the unselected else clause not calling stroke. But then I am not sure what you wanted.

Here is the code that makes it do what you describe in the question. Be careful with variable declarations. The i in the for loop was undeclared so was a global. If you called a function that did the same you would have had a very hard to trace bug.

function drawPlanes() {
    var i,pointX,pointY,width,lineThickness;  // Always declare the variables with var, const or let
    lineThickness = 3;// <<================= Moved this out of loop as it did not seem to change
    for (i = 0; i < aPlanes.length; i++) {


        // Line
        ctx.lineWidth = 4;
        ctx.fillStyle = 'red';
        ctx.strokeStyle = 'red';
        ctx.beginPath();
        ctx.moveTo((aPlanes[i].topPoint.x), (aPlanes[i].topPoint.y + (aPlanes[i].widthHeight / 2)));
        ctx.lineTo((aPlanes[i].infoBoxTopLeft.x + (aPlanes[i].infoBoxWidth / 2)), (aPlanes[i].infoBoxTopLeft.y + (aPlanes[i].infoBoxHeight / 2)));
        ctx.stroke();

        if (aPlanes[i].selected == true) {
            // set fill and stroke style
            ctx.fillStyle = 'red';  // <<===================  I added this line
            ctx.strokeStyle = 'red';  // <<===================  I added this line
            ctx.lineWidth = lineThickness; // <<===================  I added this line

            pointX = aPlanes[i].topPoint.x;
            pointY = aPlanes[i].topPoint.y;
            width = aPlanes[i].widthHeight;

            ctx.beginPath();
            ctx.moveTo(pointX, pointY);
            ctx.lineTo(pointX + (width / 2), pointY + (width / 2));
            ctx.lineTo(pointX, pointY + width);
            ctx.lineTo(pointX - (width / 2), pointY + (width / 2));
            ctx.closePath();
            ctx.stroke(); // <<===================  I added this line
            ctx.fill();
        } else {
            ctx.fillStyle = 'black'; // <<===================  I added this line
            ctx.strokeStyle = 'red'; // <<===================  I added this line
            ctx.lineWidth = lineThickness; // <<===================  I added this line

            pointX = aPlanes[i].topPoint.x;
            pointY = aPlanes[i].topPoint.y + lineThickness;
            width = aPlanes[i].widthHeight - (lineThickness * 2);

            ctx.beginPath();
            ctx.moveTo(pointX, pointY);
            ctx.lineTo(pointX + (width / 2), pointY + (width / 2));
            ctx.lineTo(pointX, pointY + width);
            ctx.lineTo(pointX - (width / 2), pointY + (width / 2));
            ctx.closePath();
            ctx.stroke();
            ctx.fill(); // <<===================  I added this line
        }

        // Flight Information Box
        ctx.beginPath(); // <<=================== I added this
        ctx.rect(aPlanes[i].infoBoxTopLeft.x, aPlanes[i].infoBoxTopLeft.y, aPlanes[i].infoBoxWidth, aPlanes[i].infoBoxHeight);
        ctx.strokeStyle = 'red';
        ctx.fillStyle = 'black';
        ctx.lineWidth = clone(intInfoBoxLineWidth); // <<==?????? you are cloning a number. Not sure why maybe you have something else in mind. 
        ctx.fill();
        ctx.stroke();
    }
}