p5.prototype.Line (line 7925 on v0.5.7) - why copy arguments[] into a new args[] array?

jaschanarveson

I'm looking at the source code for p5.js, in particular at the p5.prototype.Line function, which starts by taking the arguments[] array from the function and creating a new Array to copy them in to:

p5.prototype.line = function() {
  if (!this._renderer._doStroke) {
    return this;
  }
  var args = new Array(arguments.length);
  for (var i = 0; i < args.length; ++i) {
    args[i] = arguments[i];
  }

The rest of the function just uses the args[] array without making any changes to it - if that's the case, why copy the arguments[] array instead of just using it directly? I'm sure there's a reason - i would love to know what it is.

cheers,

j

GoToLoop

I'm sure there's a reason...

There ain't none! There are many inconsistent & under-performed code throughout the library! =P~

Here's the exact link for p5::line() within p5.js' repo:
https://GitHub.com/processing/p5.js/blob/master/src/core/2d_primitives.js#L237-#L262

p5::line() is just a proxy to call the actual method @ p5.Renderer2D::line():
https://GitHub.com/processing/p5.js/blob/master/src/core/p5.Renderer2D.js#L485-L504

p5.Renderer2D.prototype.line = function(x1, y1, x2, y2) {
  var ctx = this.drawingContext;
  if (!this._doStroke) {
    return this;
  } else if(ctx.strokeStyle === styleEmpty){
    return this;
  }
  // Translate the line by (0.5, 0.5) to draw it crisp
  if (ctx.lineWidth % 2 === 1) {
    ctx.translate(0.5, 0.5);
  }
  ctx.beginPath();
  ctx.moveTo(x1, y1);
  ctx.lineTo(x2, y2);
  ctx.stroke();
  if (ctx.lineWidth % 2 === 1) {
    ctx.translate(-0.5, -0.5);
  }
  return this;
};

Everything in proxy p5::line() is slow, unnecessary & redundant! 3:-O
Even the if (!this._renderer._doStroke) { test, b/c that same check already happens in p5.Renderer2D::line()!

EDIT:
Actually, the whole p5::line() proxy method can be turned into only 1 2 single statements: >-)

p5.prototype.line = function () {
  this._renderer.line.apply(this._renderer, arguments);
  return this;
};

This .apply(this, arguments) technique is already used across many parts of the p5.js library, btW.

Learn more about it at these links below: :-B

1. https://developer.Mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply
2. https://developer.Mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/arguments

GoToLoop

And while I was at it, I've refactored p5.Renderer2D::line() as well: :P

p5.Renderer2D.prototype.line = function (x1, y1, x2, y2) {
  const ctx = this.drawingContext;
  if (!this._doStroke || ctx.strokeStyle === styleEmpty)  return this;

  // Adjust all coords. by 0.5 for a crisp draw when lineWidth is odd:
  if (ctx.lineWidth & 1)  x1 += 0.5, y1 += 0.5, x2 += 0.5, y2 += 0.5;

  ctx.beginPath();
  ctx.moveTo(x1, y1);
  ctx.lineTo(x2, y2);
  ctx.stroke();

  return this;
};

jeremydouglass

@GoToLoop -- ...submit a pull request? https://github.com/processing/p5.js/pulls

GoToLoop

I'm eternally banned from p5.js repo. I can't submit anything there myself. =;
Most I can do (I guess), is edit the repo's wiki. 8-X