trello-archive/victor

svg size not detected; assumed to be 1x1

Closed this issue · 5 comments

stephentalley commented

I've got a valid svg file:

<?xml version="1.0" encoding="utf-8"?>
<!-- Generator: Adobe Illustrator 18.1.1, SVG Export Plug-In . SVG Version: 6.00 Build 0)  -->
<svg
  xmlns:xlink="http://www.w3.org/1999/xlink"
  id="Layer_1"
  enable-background="new 0 0 24 24"
  version="1.1"
  viewBox="0 0 24 24"
  x="0px"
  xml:space="preserve"
  xmlns="http://www.w3.org/2000/svg"
  y="0px"
>
  <path
    d="M18,20H6c-1.1,0-2-0.9-2-2V6c0-1.1,0.9-2,2-2h12c1.1,0,2,0.9,2,2v12C20,19.1,19.1,20,18,20z"
    fill="#686868"
  />
</svg>

The generated pngs are:

build/generated/res/debug/svg/drawable-mdpi/test.png 1x1
build/generated/res/debug/svg/drawable-hdpi/test.png 2x2
build/generated/res/debug/svg/drawable-xhdpi/test.png 2x2
build/generated/res/debug/svg/drawable-xxhdpi/test.png 3x3
build/generated/res/debug/svg/drawable-xxxhdpi/test.png 4x4

Victor apparently thinks this is a 1x1 svg, though Adobe Illustrator and Inkscape recognize it as 24x24.

Manually adding width and height attributes to the file fixes it:

<?xml version="1.0" encoding="utf-8"?>
<!-- Generator: Adobe Illustrator 18.1.1, SVG Export Plug-In . SVG Version: 6.00 Build 0)  -->
<svg
  xmlns:xlink="http://www.w3.org/1999/xlink"
  id="Layer_1"
  enable-background="new 0 0 24 24"
  height="24"
  version="1.1"
  viewBox="0 0 24 24"
  width="24"
  x="0px"
  xml:space="preserve"
  xmlns="http://www.w3.org/2000/svg"
  y="0px"
>
  <path
    d="M18,20H6c-1.1,0-2-0.9-2-2V6c0-1.1,0.9-2,2-2h12c1.1,0,2,0.9,2,2v12C20,19.1,19.1,20,18,20z"
    fill="#686868"
  />
</svg>

I thought this might be related to not specifying svgDpi in build.gradle, but adding:

victor {
    // Any assets defined in relative terms needs a base DPI specified
    svgDpi = 72
}

...doesn't change the result.

intrepidclass commented

Hi,

See issue 20 and also the sample project for how to specify the width in a way that Victor currently recognizes.

#20
https://github.com/trello/victor/blob/master/sample/src/main/svg/ic_laptop.svg

stephentalley commented

Thanks ciskeboekelo. Adding width/height attributes does work, but is slightly tedious and seems unnecessary given that every other tool (Adobe Illustrator, Inkscape, ImageMagick, etc.) seems to properly determine the size. I was hoping this issue could prompt a fix to make these changes unnecessary.

intrepidclass commented

@stephentalley Ah yes, I see now you already knew the workaround, sorry.

@dlew I'm wondering if this bit in SVGResource.groovy isn't the cause of this:

    width = UnitProcessor.svgHorizontalLengthToUserSpace(svgElement.width.baseVal.valueAsString, '', context)
    height = UnitProcessor.svgVerticalLengthToUserSpace(svgElement.height.baseVal.valueAsString, '', context)

Now I'm new at SVG, at Batik, at Groovy, at building Gradle plugins, so forgive me if I say something stupid, but it seems that that bit of code reading the width/heigth directly from the svg tag (which may not be specified), whereas this:

    width = context.viewportWidth;
    height = context.viewportHeight;

would seem to get the viewport dimensions regardless of whether the width is specified in the svg element. I would have tested it, but I currently don't have the time to figure out how to builld and test the victor gradle plugin myself.

dlew commented

Due to the oddities of Gradle, I find the easiest way to develop is to move the plugin to the buildSrc directory. (I know there are complicated setups you can use where it publishes the plugin automatically before building, but bleh.)

I'll look into this soon. I can't remember exactly why but I think there's a reason I was using UnitProcessor - probably it has to do with evaluating the value with DPI applied.

dlew commented

I've spent the last hour reading up on SVG sizing. More specifically, what happens if you don't define width or height because that's the crux of this issue.

The key information is provided by the specs here (emphasis mine):

SVG needs to specify how to calculate some intrinsic sizing properties to enable inclusion within other languages. The intrinsic width and height of the viewport of SVG content must be determined from the 'width' and 'height' attributes. If either of these are not specified, the lacuna value of '100%' must be used. Note: the 'width' and 'height' attributes are not the same as the CSS width and height properties. Specifically, percentage values do not provide an intrinsic width or height, and do not indicate a percentage of the containing block. Rather, they indicate the portion of the viewport that is actually covered by image data.

Since neither width nor height is provided, it assumes 100% for both. Percentage values do not determine intrinsic width/height.

The reason it works in other contexts:

  • In HTML, a percentage width/height determines its size based on other, extrinsic values provided by the webpage.
  • In SVG editors, you don't need an intrinsic width/height since it's vector graphics. You can edit in entirely abstract units; it just doesn't have any intrinsic size.

In the context of Victor, there are no extrinsic values that can be relied upon to determine the width/height. The SVG must have an intrinsic width/height.

Ultimately, I've decided this is not an issue for Victor to solve.

The only possible solution would be to allow the definition of a default width/height if neither is specified (as a way of providing extrinsic values). However, this seems a poor solution vs. being more explicit with your own SVGs.

It's been one of those fun bugs that has expanded my knowledge. Thanks!