How do I draw a complete polygon using a for loop in Javascript?

Question

So I'm trying to utilize the Canvas of HTML to draw a convex polygon.

In the code below, t2 is an array of points(I already declared that class, the code works and everything, getX() returns X, getY() returns Y.)

The drawing function works, until it reaches the code after the for loop. I want to draw a line connecting the first point and the last point, but for some reason, it just doesn't execute.

var i;
var c = document.getElementById("myCanvas");
var ctx = c.getContext("2d");
ctx.beginPath();
for(i = 0; i<t2.length-2; i++){
    var t3 = t2[i];
    var t4 = t2[i+1];


    ctx.moveTo(t3.getX(), t3.getY());
    ctx.lineTo(t4.getX(), t4.getY());
    ctx.stroke();
}
var t5 = t2[t2.length-1];
var t6 = t2[0];



ctx.moveTo(t5.getX(), t5.getY());
ctx.lineTo(t6.getX(), t6.getY());
ctx.stroke();

The canvas should show a line connecting points t5 and t6, yet nothing happens. Everything inside the for loop works, but everything afterwards does not.

Answer 1

The code is doing some strange things.

1. It's calling stroke inside the loop. That means it's drawing the first line, then the first and the second, then the first and the second and the third, etc...

   If you want to draw each line separately then you need to call beginPath after each call to stroke. If you just want to draw all the lines then you should just call stroke once at the end.

2. It's using moveTo/lineTo for every segment. This is not making a polygon, it's drawing N line segments. The difference is if you call fill instead of stroke it will fail because the code didn't make a polygon, it just made separate lines.

3. You don't need the first moveTo. The first call to lineTo after calling beginPath is automatically a moveTo (thanks @Kaiido)

4. It's not connecting the last point with the first.

   You can do that using closePath

Here's working example

class Vec2 {
  constructor(x, y) {
    this.x = x;
    this.y = y;
  }
  getX() {
    return this.x;
  }
  getY() {
    return this.y;
  }
}
const t2 = [
  new Vec2(10, 10),
  new Vec2(100, 10),
  new Vec2(100, 100),
  new Vec2(10, 100),
];

const c = document.getElementById("myCanvas");
const ctx = c.getContext("2d");
ctx.beginPath();
for(let i = 0; i < t2.length; ++i){
    const t3 = t2[i];
    ctx.lineTo(t3.getX(), t3.getY());
}
ctx.closePath();
ctx.stroke();

<canvas id="myCanvas"></canvas>

Still, there's a few things that could be simpler if the code was more JavaScript like.

For one, If we make Vec2 hold the x and y in an array and return that array with get then instead of

ctx.lineTo(t3.getX(), t3.getY());

we can just do

ctx.lineTo(...t3.get());

Now that the code is using this.value to store the 2 values we can add getters for x and y if we still want them. Then t3.getX() becomes t3.x

Another is you can use for ... of instead of a counter for your loop

for (const point of t2) {
   ctx.lineTo(...point.get());
}

Just some suggestions.

class Vec2 {
  constructor(x, y) {
    this.value = [x, y];
  }
  get() {
    return this.value;
  }
  get x() {
    return this.value[0];
  }
  set x(v) {
    this.value[0] = v;
  }
  get y() {
    return this.value[1];
  }
  set y(v) {
    this.value[1] = v;
  }
}
const t2 = [
  new Vec2(10, 10),
  new Vec2(100, 10),
  new Vec2(100, 100),
  new Vec2(10, 100),
];

const c = document.getElementById("myCanvas");
const ctx = c.getContext("2d");
ctx.beginPath();
for (const point of t2) {
    ctx.lineTo(...point.get());
}
ctx.closePath();
ctx.stroke();

<canvas id="myCanvas"></canvas>

Also instead of using for ... of loop you could use a forEach loop

    t2.forEach(point => ctx.lineTo(...point.get()));

class Vec2 {
  constructor(x, y) {
    this.value = [x, y];
  }
  get() {
    return this.value;
  }
  get x() {
    return this.value[0];
  }
  set x(v) {
    this.value[0] = v;
  }
  get y() {
    return this.value[1];
  }
  set y(v) {
    this.value[1] = v;
  }
}
const t2 = [
  new Vec2(10, 10),
  new Vec2(100, 10),
  new Vec2(100, 100),
  new Vec2(10, 100),
];

const c = document.getElementById("myCanvas");
const ctx = c.getContext("2d");
ctx.beginPath();
t2.forEach(point => ctx.lineTo(...point.get()));
ctx.closePath();
ctx.stroke();

<canvas id="myCanvas"></canvas>

Answer 2

closePath() is your friend here... see: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/closePath

Without a working example here's a rough guess at what you need, optimised.

let c = document.getElementById("myCanvas");
let ctx = c.getContext("2d");
//Start the path.
ctx.beginPath();
//Move to the initial vector.
ctx.moveTo(t2[0].getX(), t2[0].getY());
//Loop through the rest of the vectors skipping the first one.
for(let i = 1; i < t2.length; i++){
    //Add a line to the path. This always originates from the previous lineTo end vector.
    ctx.lineTo(t2[i].getX(), t2[i].getY());
}
//closePath() attempts to draw a line from the last vector to the first vector in the current path.
ctx.closePath();
//Stroke.
ctx.stroke();