uxebu / bonsai

BonsaiJS is a graphics library and renderer
http://bonsaijs.org
Other
1.96k stars 189 forks source link

stroke-dasharray attribute on paths #153

Closed iamdustan closed 11 years ago

iamdustan commented 12 years ago

I’ve started work on #134 to implement the stroke-dasharray attribute. (docs: https://developer.mozilla.org/en-US/docs/SVG/Attribute/stroke-dasharray)

I believe everything should be set on the runner side of things, but I’m looking for some pointers on where to go in the renderer/svg/ files to implement the receiving side of things and what the easiest way to visually and jasmine-y test it.

iamdustan commented 12 years ago

To copy some things into this from a conversation with @basecode on IRC:

dasharray offset

Spec: http://www.w3.org/TR/SVG/painting.html#StrokeDashoffsetProperty

Canvas implementation

halida commented 11 years ago

stroke-dasharray is a must feature, hope it will be accepted soon.

iamdustan commented 11 years ago

@halida I started work on this a while back. I’ll pick it back up in the next couple weeks. I’ll try to remember to ping you whenever I start to work on it again.

halida commented 11 years ago

@iamdustan Cool!

iamdustan commented 11 years ago

Well look at that. Two months later I finish it. There are tests for the runner.

I was going to test the renderer side of things, but it seemed that the test file for that is in a bit of disarray: https://github.com/uxebu/bonsai/blob/master/test/renderer/svg_renderer_svg-spec.js#L59-L105

Instead I just added the example movie with 550522c

basecode commented 11 years ago

Thanks, @iamdustan! As you can see, I started reviewing it :) Since this change introduces a new morphable property, it would suggest to also add a morphing example. Maybe in the same file. What do you think?

iamdustan commented 11 years ago

Thanks for the review @basecode! I appreciate your bravery to enter into my pull requests. They always end up being a marathon run ;)

I assume “morphable” means capable of being animated? I will add and I see no reason not to put it in the same file.

iamdustan commented 11 years ago

@basecode code changes are up

iamdustan commented 11 years ago

@basecode I updated the demo movie with an animation of the strokeDashOffset, but the strokeDash wasn’t getting animated appropriately. undefined was getting passed through.

(moments later). Ah. I take src/runner/animation/translators/stroke-dash.js needs to be created?

basecode commented 11 years ago

@iamdustan re:animation. Yes, since strokeDash isn't numeric, it can't be "auto translated". After creating src/runner/animation/translators/stroke-dash.js you also need to announce it here.

iamdustan commented 11 years ago

@basecode see f4097ed for my first stab at a translator. There is an example included in everything looks to be working correctly.

iamdustan commented 11 years ago

@basecode I greatly decreased the complexity of the translator in 4e5a66e650263feafc28b781912a013b91ba027f to simply push all real numbers to the array in toNumeric and reset NaN’s to 0 in the toAttr call.

basecode commented 11 years ago

Looks fine to me. Thanks for that great contribution @iamdustan! One last thing. Please add a line to the CHANGELOG and we're done.

basecode commented 11 years ago

Additional info: The current implementation implicitly leaks SVG specific API.

"If an odd number of values is provided, then the list of values is repeated to yield an even number of values. Thus, 5,3,2 is equivalent to 5,3,2,5,3,2."

But since all other possible drawing APIs are probably following this spec as well I don't see a reason for solving that problem on the runner side right now.

Canvas API says: "If the number of elements in a is odd, then let a be the concatentation of two copies of a."

iamdustan commented 11 years ago

Changelog updated.

iamdustan commented 11 years ago

:thumbsup: We made it!