PocketSVG v2

[fjolnir]

TODO:

• Update CONTRIBS.
• Resolve merge conflicts.
• Add back Podfile
• Rename instances of SVGPathSerializing back to PocketSVG.
• Support loading via NSURL
• Anything else?

I'm on holiday at the moment, I'll get this merged once I get back.

[arielelkin]

TODO after #52 is merged:

• document scaleLineWidth. @fjolnir: can you help me clarify what this does?
• Improve API in SVGEngine so that its functions can be made public. @fjolnir: let me know what you think of PocketSVG v2 #47 (comment)
• Think about SVGLayer. From an API point of view, I don't see what advantages it has over SVGImageView over a CAShapeLayer with a SVGBezierPath as its path. So I'd suggest either exposing its _shapeLayers or not exposing SVGLayer altogether.
• update Readme
• update podspec

[fjolnir]

scaleLineWidth specifies wether or not line thickness should be scaled when scaling paths (it's often not desirable)

SVGLayer is the main workhorse of the project. SVGImageView is just a thin wrapper around it. a CAShapeLayer can only render a single path, SVGLayer renders multiple, with different attributes. I don't see any reason to expose _shapeLayers..

[arielelkin]

scaleLineWidth specifies wether or not line thickness should be scaled when scaling paths (it's often not desirable)

Cool, that's in: 9bfe4ad

[arielelkin]

SVGLayer is the main workhorse of the project. SVGImageView is just a thin wrapper around it. a CAShapeLayer can only render a single path, SVGLayer renders multiple, with different attributes. I don't see any reason to expose _shapeLayers..

Because then SVGLayer doesn't offer any extra functionality compared to SVGImageView.

The only reason a user would use SVGLayer instead of SVGImageView is if they need to work with CALayer.

And in that case, I don't see why it would benefit the user to use SVGLayer rather than build one or many CAShapeLayer from SVGBezierPath.

If the user wants a CALayer from their SVG, then they should be able to benefit from all the CAShapeLayer API – in comparison, SVGLayer unnecessarily constrains them.

Which is why I believe SVGLayer should either not be exposed at all or offer advantages over CAShapeLayer.

Do you see my point?

[fjolnir]

SVGLayer is a layer, SVGImageView is a view. They have different use cases (As an anecdote: In the app I work on, we use SVGLayers and it would be a big pain in the ass if it wasn't available).

Reading paths into a CAShapeLayer would be a pain. You'd have to handle transforms & attributes yourself.

Everything in there is there for a reason.

[mindbrix]

Would it make more sense to add the SVGLayer helpers as a category on CAShapeLayer?

Maintaining a separate array of sublayers seems to go against the grain of QuartzCore. Also, resetting all the CGPaths during layout will be CPU intensive.

Have you considered calculating the scale using the determinant of the current transform?

CGFloat scale = sqrt(abs(t.a * t.d - t.b * t.c));

[arielelkin]

Hey there @mindbrix, thanks for dropping by.

Would it make more sense to add the SVGLayer helpers as a category on CAShapeLayer?

I see, but I don't think that would let users comfortably take advantage of SVGs malleability, as methods for customising the SVGs' shapes' stroke color, fill color, etc. would have to be in an initialiser, or appear as separate methods on everything that's a CAShapeLayer whether or not it's rendering SVG.

Maintaining a separate array of sublayers seems to go against the grain of QuartzCore.

It does feel awkward. But then again what exactly would that grain be? QuartzCore classes don't know how to process or represent third-party file formats (hence our library here (and yours)).

We need to do a bit of trailblazing here. So I think it would be good to give users control over the individual _shapeLayers in a SVGLayer (and it would be another competitive advantage over SVGKImage).

@fjolnir: I can see the value in exposing SVGLayer, at least as a convenience. What do you think of my reasons for exposing its _shapeLayers? (it could be a copy or readonly property).

[mindbrix]

Hi @arielelkin. Surely all that is needed is:

+(CAShapeLayer *)layerFromSVGAtURL:(NSURL *)url;

This can initialise all the layer's properties (path, fill color, etc.) from the SVG, and they can then be subsequently updated (with animation) in the usual way.

https://developer.apple.com/library/ios/documentation/GraphicsImaging/Reference/CAShapeLayer_class/

For stroke width scale invariance, you could store the SVG line width in an associated property, then on layout set the layer's lineWidth property to the SVG line width divided by the scale of the layer's affineTransform property (see above).

[fjolnir]

A CAShapeLayer can only have a single set of attributes, each path in an SVG has a separate one. So I don't see a way to render an SVG using a single CAShapeLayer.

Exposing _shapeLayers is not feasible because the sublayers do not share the lifetime of the super layer necessarily, it would also lock us into the current implementation. If one needs an extra level of control, then one should probably use CGPathsFromSVGString directly and render the paths manually. Or while being careful, reading svgLayer.sublayers since that array is exactly equivalent to _shapeLayers in the current implementation.

Maintaining a separate array of sublayers seems to go against the grain of QuartzCore.

This statement could use some sources (If you mean the actual _shapeLayers NSArray, as opposed to just sublayers in general; the reason for it is to be robust in the case where a user of the library inserts his own sublayers, which is not unheard of).

Also, resetting all the CGPaths during layout will be CPU intensive.

It's not free; I welcome PRs that improve performance.

[arielelkin]

Exposing _shapeLayers is not feasible because the sublayers do not share the lifetime of the super layer necessarily, it would also lock us into the current implementation. If one needs an extra level of control, then one should probably use CGPathsFromSVGString directly and render the paths manually.

OK, then let's keep SVGLayer as it is. I'll make sure the docs recommend the use of CGPathsFromSVGString if more fine-grained control is needed.

@fjolnir: let me know if you have any comments on #52 and feel free to merge it.

[fjolnir]

This is a misleading name. Rendering happens multiple times after the image is loaded (every time layer frame changes etc) and is handled by the shape layers. This renaming does not make things more understandable.

[arielelkin]

I see.

But I think that having this as a method is not very consistent with the rest of the API. Currently, if I want to load an SVG into an SVGLayer or SVGImageView then I have to call a method if I know the name of the file in the bundle, otherwise I need to update a variable if I have its raw XML. You should be able to load an SVG in a consistent way regardless of the form it takes (file path or XML).

I think we should offer these three different initialisers for SVGLayer and SVGImageView:

- (instancetype)initWithData:(NSData *)data
- (instancetype)initWithSVGSource:(NSString *)svgSource
- (instancetype)initWithContentsOfFile:(NSString *)path
- (instancetype)initWithContentsOfURL:(NSURL *)url;

So that the layer/imageview is always properly initialised.

To be consistent with the svgSource property, the loadSVGNamed can be replaced by this property:

@property(nonatomic, copy) NSString *svgName;

Any thoughts?

[fjolnir]

the only point of the loadSVGNamed: method is to allow live update of files as they're edited, while in debug mode. But that can be moved down to SVGImageView.

I think the best way is just to set an array of SVGPath* objects on the layer. (that class didn't exist when I wrote SVGLayer)

SVGView can have those more user friendly init methods (A'la UIImageView).

initWithContentsOfFile is redundant when you have initWithContentsOfURL though :)

[arielelkin]

the only point of the loadSVGNamed: method is to allow live update of files as they're edited, while in debug mode. But that can be moved down to SVGImageView.

Ok, but we can have the same functionality by having an svgName property.

initWithContentsOfFile is redundant when you have initWithContentsOfURL though :)

good point.

[arielelkin]

New PR with all this: #54