4
\$\begingroup\$

I wrote the Sierpinski’s Gasket Triangle in JavaScript, but I feel the code can be better, especially from L32 to L47. Could you make it more organized?

var canvas = document.getElementById('chaos');
var ctx = canvas.getContext('2d');

const GenerateRand = () => Math.floor(Math.random() * 7);
const updateDot = (x, y, point) => {
  let X = Math.min(x,point.x)+(Math.max(x,point.x)-Math.min(x,point.x))/2;
  let Y = Math.min(y,point.y)+(Math.max(y,point.y)-Math.min(y,point.y))/2;
  return {x: X, y: Y};
}
const createDot = (obj) => {
  ctx.beginPath();
  ctx.arc(obj.x, obj.y, 1, 0, 2 * Math.PI, false);
  ctx.lineWidth = 1;
  ctx.strokeStyle = '#fc3';
  ctx.stroke();
}

const pA = {x: canvas.width/2, y: 5};
const pB = {x: 5, y: canvas.height-5} 
const pC = {x: canvas.width-5, y: canvas.height-5}

createDot(pA);
createDot(pB);
createDot(pC);

const begin = (iterations) => {
  let x = canvas.width/4;
  let y = canvas.height/2;
    for(let i=0;i<iterations;i++) {
    createDot({x, y});
    let randN = GenerateRand();
    if(randN == 1 || randN == 2) {
      const currentDot = updateDot(x, y, pA);
      x = currentDot.x;
      y = currentDot.y;
    }
    else if(randN == 3 || randN == 4) {
      const currentDot = updateDot(x, y, pB);
      x = currentDot.x;
      y = currentDot.y;
    }
    else if(randN == 5 || randN == 6){
      const currentDot = updateDot(x, y, pC);
      x = currentDot.x;
      y = currentDot.y;
    }
  }
}

let time=0;
let timer = setInterval(() => {
   if(time >= 500) return clearInterval(timer)
   begin(500);
   time++;
}, 200);
<div>
      <canvas id="chaos" width="500" height="500"></canvas>
</div>

\$\endgroup\$
3
\$\begingroup\$

Performance

  • For positive numbers < 2 ^ 31 use num | 0 (bitwise or zero) to floor
  • You are drawing an arc that is 1 pixel in radius, with the stroke width of 1 the diameter is 3 pixels. This covers an area much greater than the point you sample. Use fillRect to draw a single pixel as its much quicker. Better yet as they are all the same color create a single path and use ctx.rect to add to it. Render all rect in one pass at the end of begin function.
  • Avoid creating objects needlessly. Create a working object and use that to hold intermediate values. This can greatly reduce memory allocation and GC overheads. Eg the object you return in updateDot is a waste of memory and time.
  • If you test two numbers to find the max or min, knowing either means you also know the other and thus do not need to test for it. The long lines Math.min(p.y, p1.y) + (Math.max(p.y, p1.y) - Math.min(p.y, p1.y)) / 2 can be reduced by a single test and give significant performance improvement.

Style

  • Use const for constants. Eg canvas and ctx should be const.
  • Capitals only for names of objects that are instantiated with the new token. Eg GenerateRand should be generateRand
  • Avoid repeated code by using functions. Eg you create many instances of an object {x,y}, would be better as a function.
  • Spaces between operators, commas, etc.
  • Use === rather than ==
  • else on the same line as the closing }
  • The final statement in function begin does not need the test (randN == 5 || randN == 6) (assuming you want a new point each iteration)

Code

The random number generated is from 0 to 6 and you ignore 0, redrawing the same point 1 in 7 times. You can reduce the random to give 3 values 0,1,2 and perform the correct calculation on that or use a counter and cycle the points.

You could also put the points pA, pB, pC in an array and index them directly via the random number.

Rather than use setInterval, use setTimeout. That way you don't need to clear the timer each time.

Put magic numbers in one place and name them as constants.

You reset the start point each time delay is called (first two lines). Better to just let it keep going. It may also pay to stop the rendering after a fixed amount of points have been rendered.

The rewrite.

This is just an example of the various points outlined above.

Also a few modifications

  • Automatically adjust number of points rendered to keep the GPU load steady.
  • Stop rendering after a fixed number of points rendered.
  • The starting points pA,pB,pC are in an array.
  • Magic numbers as constants.
  • Using a single render path to draw all points per render cycle.
  • Using a working point wPoint to hold coordinates rather than create a new point for each point rendered.

const ctx = canvas.getContext('2d');


const padding = 5;
const renderDelay = 200;
const maxTime = 2;  // time in ms allowed to render points.
const maxPointsToDraw = canvas.width * canvas.height * (1 / 3); 

var pointsPerRender = 500; // points to render per render pass
var totalPoints = 0; // count of total points drawn
ctx.fillStyle = '#fc3';

const generateRand = () => Math.random() * 3 | 0;
const point = (x, y) => ({x, y});
const drawDot = p => ctx.rect(p.x, p.y, 1, 1);

const updateDot = (p, p1) => {
    p.x = p.x < p1.x ? p.x + (p1.x - p.x) / 2 : p1.x + (p.x - p1.x) / 2;
    p.y = p.y < p1.y ? p.y + (p1.y - p.y) / 2 : p1.y + (p.y - p1.y) / 2;
    return p;
}

const points = [
    point(canvas.width / 2, padding),
    point(padding, canvas.height - padding),
    point(canvas.width - padding, canvas.height - padding)
];
const wPoint = point(canvas.width / 4, canvas.height / 2); // working point


const renderPoints = iterations => {
    totalPoints += iterations;
    
    const now = performance.now(); 
    ctx.beginPath();
    while (iterations --) { drawDot(updateDot(wPoint, points[generateRand()])) }
    ctx.fill();
    const time = performance.now() - now;

    // use render time to tune number points to draw 
    // Calculates approx time per point and then calcs number of points
    // to render next time based on that speed.
    // Note that security issues mean time is rounded to much higher
    // value than 0.001 ms so must test for 0 incase time is zero
    pointsPerRender = maxTime / ((time ? time : 0.1)/ pointsPerRender);

    if (totalPoints < maxPointsToDraw) {
        setTimeout(renderPoints, renderDelay, pointsPerRender | 0);
    }
}
renderPoints(pointsPerRender);
<canvas id="canvas" width="500" height="500"></canvas>

\$\endgroup\$
  • \$\begingroup\$ Thank you so much, I'll consider them from now on. \$\endgroup\$ – Zeyad Etman Dec 18 '18 at 12:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.