2013base#39
Conversation
Some code refactored; text implementation fixed. Group elements fixed. Added OUTLINE_SHAPES option for debugging purposes.
|
Thanks. There's some good stuff here, but I'll have to cherrypick it - a few bits won't work. Good catch on fixes eg using a reserved keyword as a parameter (doh) But some bits I can't merge, will comment for each separately... |
There was a problem hiding this comment.
Good question. Experimenting with the examples in the official SVG Spec (which have PNG renders included, so we can check the "expected" output), it seems there's two problems that this solves:
- SVG Spec requires that we calculate the bounds for the text based on SVG rules. We have to do that using the transform --- I was surprised at this, it makes implementation much harder than I'd like, but SVG spec's approach to text is ... weird
- Apple requires that you define the text-size BEFORE you render the layer, because Apple's clipping is applied early in the rendering --- I was suprised by this, seems that Apple did some bad API design, and made it so that CATextLayer doesn't have an "automatic" bounds based on the text contents. This is annoying :(.
...put those two together, and the font-size has to be downscaled BEFORE we scale the resulting layer. Yes, its weird - but turn it off, and then try some of the spec examples, and you'll see Apple apply "incorrect" clipping.
There was a problem hiding this comment.
Well, OK, but the problem here is that text could not only be proportionally scaled, but all the valid SVG transforms can be applied, like rotation and skew.
I agree, that my implementation is wrong concerning SVG spec bounds calculation... But it lets us RENDER all transforms correctly, breaking layer bounds.
If you create a simple svg with transformed text, you'll easily understand what I mean.
Well, for example (sorry, I don't know, how to share files here):
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!-- Created with Inkscape (http://www.inkscape.org/) -->
<svg
xmlns:dc="http://purl.org/dc/elements/1.1/"
xmlns:cc="http://creativecommons.org/ns#"
xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
xmlns:svg="http://www.w3.org/2000/svg"
xmlns="http://www.w3.org/2000/svg"
version="1.1"
width="744.09448"
height="1052.3622"
id="svg3018">
<g
id="layer1">
<text
x="14.464138"
y="80.399139"
transform="matrix(1.4614298,0.61852241,0.50594217,0.8983918,0,0)"
id="text3030"
xml:space="preserve"
style="font-size:21.16327095px;font-style:normal;font-weight:normal;line-height:125%;letter-spacing:0px;word-spacing:0px;fill:#000000;fill-opacity:1;stroke:none;font-family:Verdana"><tspan
x="14.464138"
y="80.399139"
id="tspan3032">ROTATED AND SKEWED TEXT</tspan></text>
</g>
</svg>
There was a problem hiding this comment.
Yeah, I see what you mean.
Perhaps we need to pull your last few lines in this method (which do the transforms) up to this point, and apply them to the results of Apple's CoreText methods from the 2013base code ... and work out the actual box needed to send to CATextLayer / etc?
There was a problem hiding this comment.
We should try.
As far as I see, according to spec, we have to get bounds look somehow like the red box.
With my code, we get the green one.
There was a problem hiding this comment.
So ... I've spent a long time looking at this.
The short answer is: your original comment "why do we do this?" is because you've misunderstood how the SVG library works.
The library - from day 0 - was built to do this:
- work out the position and transofrmation of everything
- position everything correctly on screen. If something needs rotating or skewing, do that to the shape itself
- build CALayers that make things appear in those places. NEVER SET THE TRANSFORM FOR ANY LAYER
Advantages:
a. Users can use 3D transforms, or custom Affine transforms, and it all works
b. Render speed and hit-test speed is - theoretically - much faster because the transforms only have to be calculated once
Disadvantages;
c. If you take a layer that's invisible (e.g. elements are defined as invisible in SVG), and make it visible - it will appear to be in the wrong place.
d. We need the "layoutLayer" method to be called on EVERYTHING after making the layers - because we don't know the correct size of ANY layer until we've rendered ALL layers
There was a problem hiding this comment.
I don't know that the original version is "best" - but I've looked at the amount of code needed to change it (a lot!) and ... it would seem to make the library slower, so ... I'm going to undo my changes and leave it working the way it was previously.
|
I've commented on the commit itself, using GitHub's line-by-line. Some of it is notes to myself - when I get some time, I'll do a careful bit-by-bit merge and test each change. There's some questions in there too - if you get time, would be great to get your thoughts / help on some of those. |
|
For sure, I'll try to answer each question/comment. Where, by the way, can I find official examples? |
|
In the SVG spec itself, ( http://www.w3.org/TR/SVG/ ) they often embed The links resolve to plain SVG that you can save directly. Also, every time a user reports a "broken" SVG, we ask them to create a On 18 January 2013 14:08, Mark Trubnikov notifications@github.com wrote:
|
|
Oh, by the way, do you have any performance issues when zooming in and out? |
|
With SVGKFastImageView - no, that's why I made it :). I haven't tried it If FastImageView is slow, it suggests that CATextLayer is broken and On 18 January 2013 15:14, Mark Trubnikov notifications@github.com wrote:
|
|
Just a quick note to say: I'm swamped with work right now :(, haven't had time to finish merging this. But will get back to it ASAP when I can. |
There was a problem hiding this comment.
Accoridng to SVG spec, there's no X nor Y in SVGShapeElement - they only exist in SVGRectElement
(some of these SVG*Element classes are badly wrong in SVGKit right now, but I'm moving them all towards being spec-compliant)
|
FINALLY merged ... great work, @mtrubnikov ! Because GitHub refused to auto-merge, I had to split it up into small pieces. There is ONE PIECE I haven't done which should work fine - I'd prefer you to submit it as a separate commit (so it shows up under your username) -- the two UIColor-Expanded files. For the rest: "Some code refactored;" -- I had to skip the changes to layoutLayer because they triggered some crashes elsewhere. I don't know why, I'll have to revist this later :( "Text implementation fixed;" -- in the end, I used your approach of setting .affineTransform - but to make it work with ALL our text examples I had to do it differently (c.f. the code comments for detailed explanation of WTF I did and why) "Group elements and transforms done the right way (I hope so);" -- as discussed, there's too many problems with Apple's .affineTransform code for us to use it throughout SVGKit. However, inspired by your fixes, I re-wrote the whole of transforms to be MUCH easier to read, and to be MUCH closer to SVG Spec compliance (c.f. the new protocol "SVGTransformable" that's part of the SVG Spec) "Matrix transforms parsing fixed;" -- merged fine, but it had some problems with complex transforms, so I had to tweak it a little. Seems to work. "Added OUTLINE_SHAPES option for debugging purposes - it draws bounding boxes for all elements." -- this is a great idea, but it can be done in user-code (as a runtime thing). I've moved your code into the Demo-iOS project, AND I've merged it with some code I wrote for hit-testing. Now whenever you tap anywehre on screen, it outlines the smallest containing object. |
|
Sorry, I had no time even to answer you here recently. |
|
No worries - if you don't have time, I can commit them. But I prefer to keep the commit history accurate, if possible. |
Some code refactored;
Text implementation fixed;
Group elements and transforms done the right way (I hope so);
Matrix transforms parsing fixed;
Added OUTLINE_SHAPES option for debugging purposes - it draws bounding boxes for all elements.
May the force be with you. ;)