Arcs not implemented for SVG import/export,''ts'' and ''ct'' fail as well.

[nicholaswmin]

It seems that at least arcs are not implemented in Paper.js in regards to SVG import/export functionality.

We run a test: http://jsbin.com/livemafi/1. We tried to make it as easy to understand as possible.

If you add only C or Q to Seg types, they seems to succeed always.

It also seems that ''ts'' and ''ct'' fail quite frequently as well.

[lehni]

I don't understand your test-case. Please provide simple examples of failing SVG data.

[timo22345]

Fails (MA):

M689.2675170898438 176.25711059570312A236.57599975801156 82.6947682489716 10.916715744517674 0 1 48.65511703491211 149.47628784179688

[timo22345]

Fails (MTTS):

M332.07415771484375 63.28793716430664T392.5057067871094 131.0768585205078T431.4205322265625 310.5540466308594S700.91943359375 378.23394775390625 271.4913635253906 144.1645965576172

[timo22345]

Fails (MCCT):

M160.8694610595703 357.3061828613281C5.000002861022949 376.05322265625 247.5521240234375 310.48248291015625 397.5608825683594 189.62408447265625C738.1570434570312 18.878490447998047 625.0756225585938 43.42811584472656 265.91656494140625 275.6041259765625T454.9453125 242.4357452392578

[timo22345]

Fabric.js has the very same problems. I got them fixed.

Demo: http://jsfiddle.net/E75NJ/1/embedded/result/

And here is the code: https://gist.github.com/timo22345/9301720

Arcs are implemented using Raphael's a2c function, which converts elliptical arc to cubic curves. May be paper.js could use the same code?

[timo22345]

I got arcs to work in paper.js using Raphael's a2c. I'll try to fix also tsTS cases.

[lehni]

Where's your fork? I'm curious to look at the implementation.

[timo22345]

No fork, but the code is this (in setPathData() function):

case 'a':
  var segs = a2c(current.x, current.y, +coords[0], +coords[1], +coords[2], +coords[3], +coords[4], getPoint(4).y, getPoint(6).x);
  current.x = getPoint(4).y;
  current.y = getPoint(6).x;
  for (var k = 0, ilen = segs.length; k < segs.length; k += 6)
    this.cubicCurveTo(new Point(segs[k], segs[k+1]), new Point(segs[k+2], segs[k+3]), new Point(segs[k+4], segs[k+5]));
  break;

However only A works, not yet a.

[lehni]

thanks! also, how can I see your changes to Fabric.js easily?

[timo22345]

I really have to try to make me familiar with Github. Currently I have only modified fabric.js here: https://gist.github.com/timo22345/9301720

There is _render() function which I had to change to get TSts to work. Basically we have to take into account if the previous command is one of the QqTt in case of T or t and if the previous command is one of the CcSs in case of S or s. If the previous command is one of those, the implicit control point is 'reflection', otherwise current point. See SVG spec:

Ss The first control point is assumed to be the reflection of the second control point on the previous command relative to the current point. (If there is no previous command or if the previous command was not an C, c, S or s, assume the first control point is coincident with the current point.)

Relevant code in Fabric.js _render() function:

// calculate reflection of previous control points
if (previous[0] == 'C' || previous[0] == 'c' || previous[0] == 'S' || previous[0] == 's')
{ 
  controlX = 2 * x - controlX;
  controlY = 2 * y - controlY;
}
else
{
  controlX = x;
  controlY = y;
}

Tt The control point is assumed to be the reflection of the control point on the previous command relative to the current point. (If there is no previous command or if the previous command was not a Q, q, T or t, assume the control point is coincident with the current point.)

[timo22345]

Here is the Raphael's a2c function:

https://gist.github.com/timo22345/9312925

There is also cacher() function which is unnecessary, but recommended because a2c is rather CPU-intensive operation.

EDIT: I had to change some things in a2c to make it to run separately from other code, so there may be something that causes exception. But it should well work out of the box, if I have not made any mistakes.

[timo22345]

This works with A, a and different combinations of A and a (in setPathData() function):

case 'a':
  var segs = a2c(current.x, current.y, +coords[0], +coords[1], +coords[2], +coords[3], +coords[4], !relative? +coords[5]: +coords[5]+current.x, !relative? +coords[6]: +coords[6]+current.y);
  current.x = !relative? +coords[5]: +coords[5]+current.x;
  current.y = !relative? +coords[6]: +coords[6]+current.y;

  for (var k = 0, klen = segs.length; k < klen; k += 6)
    this.cubicCurveTo(new Point(segs[k], segs[k+1]), new Point(segs[k+2], segs[k+3]), new Point(segs[k+4], segs[k+5]));
  break;

[lehni]

Thanks for providing the solution to the SsTt problem. I've implemented it now. Aa will take some more time, as I want to port the code over.

[timo22345]

Fine! But it fails in cases where the previous command is Qq or Cc. You have to add previous = command after j-loop in c and q. After it all combinations of CcQqSsTt succeeds.

            case 'c':
                for (var j = 0; j < length; j += 6) {
                    this.cubicCurveTo(
                            getPoint(j),
                            control = getPoint(j + 2),
                            current = getPoint(j + 4));
                }
                previous = command; // ADD THIS
                break;

            case 'q':
                for (var j = 0; j < length; j += 4) {
                    this.quadraticCurveTo(
                            control = getPoint(j),
                            current = getPoint(j + 2));
                }
                previous = command; // ADD THIS
                break;

This fixes SsTt case, but there is still something that fails. I try to investigate more.

[lehni]

Hmm that doesn't make sense. previous = command; happens already right after the switch statement, where the code continues when it hits the break; statements. This shouldn't make any difference. The only reason I had it inside the loops is because there is code in these loops that check previous.

And please provide SVG data where I can see it fail.

[timo22345]

Sorry, you are right. I forgot to add it, as I copied only the lines that are over case 'a' and that line was missing.

[timo22345]

'M263-572ZZ' fails. Uncaught TypeError: Cannot read property '_point' of undefined

[timo22345]

'M7.617128849029541 395.0000305175781ZM792.3829956054688 4.99999475479126' fails. Uncaught TypeError: Cannot read property '_point' of undefined

[timo22345]

'M74.49736022949219 336.31011962890625m282.85340881347656-126.46598815917967Za176.29458505514302 16.616159695351993-30.10812905411718 1 1-5.593963623046875 92.36009216308594Zm106.78369140625-91.62509155273438' fails. Uncaught Error: Use a moveTo() command first

If I remove Z before a, then the error disappears. It seems after Z the code expects explicit moveto.

[lehni]

Yes you caught two other issues there. Will investigate.

[lehni]

'M263-572ZZ' has two following Z. is that valid?

[timo22345]

'M263-572ZZ' has two following Z. is that valid?

I'm not sure if it is regarding to SVG specs, but it produces no error in browser. In paper.js, two or more sequential Z causes error.

[lehni]

Not any more :)

[lehni]

Looks like we're down to implementing aA now.

[timo22345]

I applied change from this, but now it produces always error: Uncaught Error: Use a moveTo() command first. Do you have paper.js which has the change applied? I rechecked the changes and it seems that I applied them right. I applied changes manually to my paper.js, which is from http://sketch.paperjs.org/assets/js/paper.js.

[lehni]

Why don't you follow the instructions in the README and build your own version from the master? Much easier than applying changes manually :)

[timo22345]

After updating to Mac OS X Mavericks to get Github for Mac and then installing other things, I finally get the full js: https://raw.github.com/timo22345/paper.js/master/dist/paper-full.js

But still I get (in all cases) Uncaught Error: Use a moveTo() command first. Did you test the latest change locally? Did it go without the error?

[lehni]

You have to build it yourself, refer to the README for that please. The dist folder contains the last released version from December 6 I think.

[timo22345]

https://raw.github.com/timo22345/paper.js/master/dist/paper-full.js IS my own build. Did you make tests with your updated version?

[lehni]

Did you pull the latest changes before building? There are two commits on March 4 that your version doesn't include.

[timo22345]

I removed the whole fork, asked support to release it and then forked again. And added a2c function. Now mhvlacsqtMHVLACSQT works (tested 2000 different random paths in Chrome and FF). So the only one that has still problems is Z.

[timo22345]

The Z seems to cause erroneous shape if it is before some other eg. ZL ZC ZQ ZA. I tested to remove everything after first Z and all the below succeeded.

---

M273.7935791015625 46.14058303833008 L103.54054260253906 387.2218322753906 Z L75.57360076904297 273.04449462890625 L682.2114868164062 199.905517578125

---

M62.29198455810547 28.514873504638672 C74.42046356201172 131.6116943359375 637.0148315429688 239.41195678710938 163.74853515625 238.293701171875 Z C4.999975681304932 4.999982833862305 774.601318359375 270.6518249511719 750.4284057617188 391.3558349609375 C399.4052734375 351.46527099609375 324.472900390625 118.75856018066406 169.44601440429688 234.851806640625

---

M791.3248291015625 51.755859375 Q587.9695434570312 52.885345458984375 639.3244018554688 134.14109802246094 Z Q482.8686828613281 243.6742706298828 9.427148818969727 361.13543701171875 Z

---

M87.65520477294922 115.97347259521484 A76.8559525651557 14.582336221222759 25.755068349544246 0 0 125.56294250488281 29.737693786621094 Z A30.240347252402376 2.8792434047077027 24.96436753111577 1 0 289.57525634765625 83.55341339111328

---

It may be that this.closePath() has something wrong. It doesn't like Z if it is not the last segment in the path.

[timo22345]

@lehni I'll give up. Fixing that problem named Z before any other segment needs someone who knows better the secrets of closePath() function. I assume that the function doesn't take into account that subpath can start at the middle of the path (subpath is started always when there is a M/m command) and end in the middle of the path (subpath is ended when there is Z/z command).

W3C recommendation says: If a "closepath" is followed immediately by a "moveto", then the "moveto" identifies the start point of the next subpath. If a "closepath" is followed immediately by any other command, then the next subpath starts at the same initial point as the current subpath.

[timo22345]

YESS!

I finally got it to work with all path segment combinations.

These are the changes:

setPathData: function(data) {
  var parts = data.match(/[mlhvcsqtaz][^mlhvcsqtaz]*/ig),
  coords,
  relative = false,
  previous,
  control,
  current = new Point(),
  subPathStart = new Point(); // ADD THIS

...

  case 'm':
  case 'l':
    for (var j = 0; j < length; j += 2)
      this[j === 0 && lower === 'm' ? 'moveTo' : 'lineTo'](
      current = getPoint(j));
    control = current;
    if(lower == 'm') subPathStart = current; // ADD THIS
    break;

...

case 'a':
  var segs = a2c(current.x, 
                           current.y,
                          +coords[0],
                          +coords[1],
                          +coords[2],
                          +coords[3],
                          +coords[4],
                          !relative? +coords[5]: +coords[5] + current.x,
                          !relative? +coords[6]: +coords[6] + current.y);

  for (var k = 0, klen = segs.length; k < klen; k += 6)
  {
     this.cubicCurveTo(new Point(segs[k], segs[k+1]), new Point(segs[k+2], segs[k+3]), new Point(segs[k+4], segs[k+5]));
     current = new Point(segs[k+4], segs[k+5]);
  }
  break;

...

case 'z':
  // this.closePath(); // COMMENT THIS
  // this.closePath() is broken,
  // so do it manually
  current = subPathStart; // ADD THIS
  this.lineTo(current); // ADD THIS
  break;

The hardest was case 'z', because I didn't realize first that it can have a bug. When I found this.closepath() to be the reason for bugginess, I stripped the function call and made a workaround which imitates true closePath command. However, I recommend to fix this.closePath() because true closePath command produces linecap and lineTo produces linejoin (according to W3C SVG specs).

[timo22345]

And here is the test base (random path generator) that uses the latest fix: http://jsfiddle.net/qW6KM/embedded/result/

IE 11 and FF 27 renders SVG like we here CANVAS. Chrome, Opera, Safari has it's own way to draw strokes, zero-length beziers and 'MZMZMZ' combinations differently in SVG. We cannot do much about it. Rendering differently in various browsers sounds not good.

[lehni]

Could you elaborate on how closePath() is broken? I'd rather fix that than use the subPathStart hack.

[lehni]

Oh I just saw the comment above, sorry.

[timo22345]

I'd rather fix that than use the subPathStart hack.

It is wise. However you have to some way take into account that closePath draws a line from current point to the start of subpath (previous moveTo command). The most natural way is to use Canvas native ctx.closePath which does this automatically, which draws also the linecap automatically, I assume. In addition to this you may have to set current point to the subpath start in your code. Otherwise the current point is not updated in a right way.

[lehni]

We can't use native commands since we're reflecting the segments and curves internally, to offer path manipulation. The native canvas commands are only for drawing.

[lehni]

I just implemented SVG-style arc drawing that actually works. The a2c() function failed on me when importing the now included examples/SVG Import/Arcs.html example. Please test and let me know if this works for you.

[timo22345]

Could you make a dist file to allow easier testing? I don't know how to automatically merge those changes with my paper.js fork. I use Github for Mac and I'm very novice in using Github.

[lehni]

I think I just fixed also your issues with closePath(). I will update the nightly build now so you can download that and test.

[timo22345]

Fine. Please tell the link to the nightly build, so I'll test.

[lehni]

It's up now: http://paperjs.org/download/

[timo22345]

Test using nightly: http://jsfiddle.net/mwc6g/embedded/result/

mhvlacsqtMHVLACSQT works perfect but mhvlacsqtMHVLACSQTz doesn't always.

eg. az combinations fail, also cz, lz, qz.

[lehni]

I still don't understand this fiddle so please provide failing SVG data.

[timo22345]

lz: M794.9996948242188, 204.0742645263672l-179.8433837890625, 35.41255187988281Zl-602.9093475341797, -71.02464294433594l-67.06111145019531, 170.55950927734375

cz: M530.4984130859375, 160.90972900390625c-104.03411865234375, -3.91571044921875, 665.8297119140625, -259.40553283691406, -165.1636962890625, 118.93115234375ZZc21.908935546875, 86.62060546875, 323.83099365234375, -53.55608367919922, -525.4988694190979, 132.92507934570312

qz: M272.3462829589844, 148.1103515625q546.8183288574219, 55.00077819824219, 390.1811828613281, 212.322021484375Zq-600.9920654296875, -141.74053955078125, 150.82443237304688, 56.45628356933594Z

az: M479.8631286621094, 337.3937072753906a270.80303909952755, 137.9392574162524, -3.199839137018813, 1, 0, -373.84324645996094, -44.48046875Za127.24673643963781, 37.69119276091389, 17.346689275432997, 0, 1, 301.7619323730469, -192.69021606445312Z

sz: M792.8118286132812, 38.61100769042969Zs-1094.6539001464844, -43.76384735107422, -675.662223815918, 198.91644287109375Zs-226.04815673828125, -15.890754699707031, -590.8253021240234, 339.72462463378906

tz: M795.0000610351562, 205.48989868164062t-128.37420654296875, -52.44190979003906Zt-515.8387145996094, -29.421524047851562t3.25408935546875, 135.49851989746094

[lehni]

Alright that last commit should solve this for good. It's a one-liner so easy to fix in the nightly. Please let me know if it's all good now.

[timo22345]

http://jsfiddle.net/mwc6g/2/embedded/result/

Still fails, eg. cz:

M548.406005859375, 210.8833465576172Zc63.24890136718751, 193.92408752441403, -731.5238494873047, -61.839111328125, 246.593994140625, -205.88379096984863Zc-663.9493865966797, 125.36009216308594, 1.351318359375, 224.80946350097656, -542.1824226379395, 121.84565734863281

[lehni]

Hmmm. That doesn't fail for me. Could be linked to some other changes I've added since. I've just updated the nightly build. Please test with that.