-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Infer width and height from viewBox #237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 2.x
Are you sure you want to change the base?
Infer width and height from viewBox #237
Conversation
if not specified
…0-infer-width-and-height-from-viewbox
…er-width-and-height-from-viewbox Conflicts: Source/DOM classes/Unported or Partial DOM/SVGSVGElement.m
|
Off the top of my head, you cannot take width and height from viewbox - I'm checking now in spec, but this stuff is subtle. On 13 November 2014 17:06:12 Chetan Padia notifications@github.com wrote:
|
|
A good svg to test with is the world map - it has a negative view box origin (which is horrible, but legal!). Also, do you have some test SVGs you can release as public domain and include in thedemo project to demonstrate your changes working? Make sure no one breaks this in future... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldnt this be part of the existing method?
|
Going through this in detail, I like what you're trying to do, but I think there's a cleaner way.
This way, we fix "relative" width / height everywhere, AND we keep the information about original viewport (which your patch currently discards), AND - I believe - the example SVG's you have will all be fixed. ...AND, as a bonus, other percentages for width/hegith on the root SVG will all "just work". |
|
Looking at this in more detail there are two problems here:
It is trivial to create test cases:
Both of these cases are perfectly valid SVG files as far as popular editors are concerned. I've been trying to work through this and I can't quite work out how to structure it. Your instructions seem clear enough, but there are a few things that don't quite make sense to me.
I believe all we need to do is:
Any suggestions of how to proceed? |
|
I think therrs a lot more than two problems. One fix can fix those AND several others. Eg i believe we crash anywhere there is a % width or height. Eg when svg width is a percentage, there are several cases where we should be calculating actual numbers - using the viewbox is actively wrong. I'll have a look again later today, but I think we can retain percentage info easily enough. |
|
I've made a personal branch / fork to play around with this: https://github.com/adamgit/SVGKit/tree/FixPercentLengths I've made a commit that does:
As far as I can tell, this solves most of the above problem. What's missing:
It may also be that we need to edit the "transform" code - if Percentage SVGlengths are legal inside a transform, then we should use SVGLengthFromLengthInSVGTree there too. |
|
(in case it's not clear - I'm suggesting you fork that fork, see if you can easily apply the fixes on top. If so, you can PR to my fork, and I can then merge and PR back into SVGKit master. Or ditto if anyone else wants to fix this) Because: this is quite a complex fix, impacting a bunch of files, and I'm not sure we'll get it right first time. I don't want to mess around too much in SVGKit upstream with this. |
Some SVG editors and exporters do not include the
<svg>"width" and "height" attributes.e.g.
SVGKit currently will not render these files. We found that Adobe Illustrator exports files like this if you check the "Responsive" option when exporting.
Additionally, some editors and exporters set percentages as their "width" and "height" values to indicate a relative size at which to display the image to a browser. e.g.
SVGKit currently crashes when rendering these files. We've found that Inskscape saves files like this when saving as "SVG Plain".
This change ensures that:
widths andheights are ignoredwidthandheightattributes are present, we simply grab the values from theviewBoxattribute.