new Bezier() use of `arguments` is broken, and unrelatedly, slows down Chrome about bezierjs HOT 16 CLOSED

Before we go down that route, can you explain what you mean with "doesn't work"? these lines ensure that the following calls all work, by looking at whether the passed values act as iterable or not.

You can verify that they work by pulling up a dev console on http://pomax.github.io/bezierjs and running them in the console:

• new Bezier(1,2, 3,4, 5,6)
• (the same but for 4 pairs)
• new Bezier(1,2,3, 4 5,6, 7,8,9)
• (the same but for 4 pairs)
• new Bezier([{x:1,y:2},{x:3,y:4},{x:5,y:6}])
• (the same but with 4 pairs, z-coordinates, and both)

from bezierjs.

I knew I should have given an example! Sorry. new Bezier({x:1,y:2},{x:3,y:4},{x:5,y:6})

from bezierjs.

You're right, that won't work: if you put them in an array and pass that in, it'll work fine, but the code should probably be able to deal with this. The problem is not with the line you marked, though. That line should stay the same. The trouble is that https://github.com/Pomax/bezierjs/blob/gh-pages/bezier.js#L502 doesn't use the right container for its data.

from bezierjs.

We can easily make it work. Putting it in an array works fine too, of course (which is what I did in my code).

from bezierjs.

The fix here will be to replace this:

    if(typeof args[0] === "object") {
      args = [];
      for(var i=0; i<coords.length; i++) {
        ['x','y','z'].forEach(function(d) {
          if(typeof coords[i][d] !== "undefined") {
            args.push(coords[i][d]);
          }
        });
      }
    }

with this

    if(typeof args[0] === "object") {
      newargs = [];
      for(var i=0; i<args.length; i++) {
        ['x','y','z'].forEach(function(d) {
          if(typeof coords[i][d] !== "undefined") {
            newargs .push(args[i][d]);
          }
        });
      }
      args = newargs;
    }

from bezierjs.

Yep, that looks right! Probably var newargs = instead of newargs =.

from bezierjs.

For the other part, the odd Chrome performance issue, do you think the workaround makes sense?

from bezierjs.

sort of. Might as well just do a straight up array cast, though.

var args = ... [].slice.call( arguments )

from bezierjs.

That's what I thought too, but apparently even that isn't optimized well in Chrome (I read that in a bug report and just tested in my Chromium to double check).

from bezierjs.

I'd love to be missing something, though, so let me know if you find something different. I'm checking whether it's optimized well by running the profiler in Chromium and looking for the yellow exclamation-mark warning sign next to Bezier in the "Heavy (bottom up)" profile. Mouse over the warning sign to see the message describing the warning.

from bezierjs.

I've pushed the update, so you can check what Chrome thinks of that.

from bezierjs.

I'm still seeing a warning message in the profiler.

from bezierjs.

If you want to see where I'm testing, it's at https://github.com/idupree/trainworldii that you can compile with tsc script.ts and then open index.html in a browser (first edit bezier.js to be the version you want it to be). If that's too complicated I can make it easier to test.

from bezierjs.

On further investigation, even with the difference in ⚠ vs not, it's now taking about the same amount of time for me either way in the profile. I wonder if I missed something before. I am satisfied to tentatively close this ticket unless/until anyone can measure a large time difference in the future.

from bezierjs.

in that case let's close this, until such time someone else run into it.

from bezierjs.

Thanks!

from bezierjs.