Add strokeDashOffset and svg corresponding attribute

[blobinabottle]

First PR here. Hope nothing wrong.

I added strokeDashOffset on all objects with stroke.
I tested successfully toSVG() with rect, circle, line.

[asturur]

Hey thanks!
We have to do a good amount of work here because of tests, but i can take part of the manual work.

As additional item you should remove dist folder from committing.
I usually do before committing:
git checkout master dist and restore the dist folder to the original state.
I keep a copy of npm run build:watch running while i develop so that it build automatically each file change.

We need to modify parser, i do not see the change in parser.js that will make possible to read the value and adding it to the object

Also we need new test and fix old tests.

[asturur]

look here for parser.js
https://github.com/fabricjs/fabric.js/blob/master/src/parser.js#L42

[asturur]

i wonder if node-canvas does support it.

[blobinabottle]

Sorry for the dist folder!
I'll check if I can test with node-canvas...

[asturur]

ok you are working on your master branch. That will make everything harder.
How are you using git? from command line or a client?

the last master good commit you should have is cdb6b513857881b33d689c75df4a2962c5ec4b17

so if you write git checkout cdb6b513857881b33d689c75df4a2962c5ec4b17 dist you will put back the right dist folder. In this way your PR would delete the dist folder for everyone.

[asturur]

there are so many tests to update, do not rush because you will get burned.
Are you able to run npm run test on your machine?

[blobinabottle]

aah easier to run locally :-) thanks

[blobinabottle]

I have several errors about imageSmoothing, is it fixable?

not ok 44 test/unit/canvas_static.js > Failed to load the test file with error:
TypeError: Cannot read property 'imageSmoothingEnabled' of null

[asturur]

i think your node-canvas installation failed.
On what OS are you doing this?

Before installing fabric in the develop folder with npm install you have to be sure you met the requirement for node-canvas to work.

https://github.com/Automattic/node-canvas/tree/v1.x

you can still avoid running them on node doing:

npm run testem

this will open something like this:

and from the suggested http address you can see what is going on in the browser, where you do not need to install node-canvas.

[asturur]

this is looking good. we need a small last effort:
a visual test and a svg import/export visual test.
I ll let you an example tomorrow that now is very late.

Thanks!

[asturur]

Ok so the idea would be to create an svg like one of those in the asset folder:
test/visual/assets
Most of the files are full and you do not want to loose time regenerating more than one preview.

create a new file called svg_stroke_9.svg in that folder. and copy inside the content from svg_stroke_2.svg. In this way you have some lines already setup and you do not need to reinvent the wheel.

in this array:
https://github.com/fabricjs/fabric.js/blob/master/test/visual/svg_import.js#L54
add the name of the new file you created.
(remember to do git add test/visual/asset/svg_stroke_9.svg before committing or the new file won't be pushed up)

Now in the file you created try to add some meaningfull examples that can cover also that other dash thing you talked about in the other issue ( starting with 0 )

Modify the file so that we have:
one line to have stroke-dashoffset and even number of dash
one line to have stroke-dashoffset and odd number of dash
one line to have stroke-dashoffset and even number of dash that start with 0
one line to have stroke-dashoffset and odd number of dash that start with 0
one line to have stroke-dashoffset and even number of dash that ends with 0
one line to have stroke-dashoffset and odd number of dash that ends with 0
one line without stroke-dashoffset and even number of dash that ends with 0
one line without stroke-dashoffset and odd number of dash that ends with 0
one line without stroke-dashoffset and even number of dash that start with 0
one line without stroke-dashoffset and odd number of dash that start with 0

As you see in the starting file some are declared in the style, other as property. You can do as you want. Regression should be catched anyway.

Then last step, you need to generate the golden image.
Open this file here in the browser:
/test/visual/goldenMaker.html
it will give you like a place to drag file or a button to upload them. Upload the the file you created and you will get back a png named in the same way that you have to add in the test/visual/golden folder and remind also to git add test/visual/asset/svg_stroke_9.png before pushing.

Running the tests with the original dist folder should fail this test, while with the new built fabric should pass.

[asturur]

to run the test and avoid node installation run npm run testem:visual
in the browser a passed test looks like:

while a failed one will be complaining about different pixels on the last bw image.

[asturur]

Suggested change

	'stroke-dashoffset': 'strokeDashOffset',
	// TODO: add support for percentage value when parsing from SVG
	'stroke-dashoffset': 'strokeDashOffset',

[asturur]

@blobinabottle i ll merge this as it is, is good enough. I ll add the additional tests.

[blobinabottle]

Ah sorry I planed to try doing the tests this weekend. I think it’s good I learn that :)

[asturur]

you can add test, in a separate PR, and they should have different results with built and non built fabric.js

[asturur]

If instruction i left are not clear, please ping me.