openfl / svg

Provides SVG parsing and rendering for OpenFL and Haxe
MIT License
68 stars 30 forks source link

Stroke thickness off by sqrt(2)/2 #35

Closed jcward closed 8 years ago

jcward commented 8 years ago

With a simple test SVG, I'm finding that the stroke thickness is off by a factor of sqrt(2)/2 (and, less critically, the default joint type seems to be rounded instead of miter.)

Here's the test SVG:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg width="256" height="256" version="1.1">
  <rect x="0" y="0" width="256" height="256" fill="#FFFFFF"/>
  <rect x="32" y="32" width="192" height="192" style="fill:#0F0;stroke:#000;stroke-width:32;stroke-opacity:0.5"/>
  <circle cx="128" cy="128" r="32" fill="#0000FF" stroke="#000000" stroke-opacity="0.5" stroke-width="32" />
</svg>

And this is OpenFL on the left vs eog/gimp/etc on the right:

image

I can hack the extra scaling factor into SVGRenderer.hx and it fixes the issue, however, I'm not sure where this extra factor should go in the lib:

image

ashes999 commented 8 years ago

@jgranick can you please comment on this?

jcward commented 8 years ago

@jgranick @ashes999 interesting, the scale computation on line 137 is:

var scale = Math.sqrt(m.a*m.a + m.d*m.d);

It's taking the hypotenuse length of the x/y scale. But for an identity matrix, this results in 1.414 (aka, sqrt(2).) so I believe we need to divide out the sqrt(2) like so:

var scale = Math.sqrt(m.a*m.a + m.d*m.d) / 1.41421356237;

I'm going to do a quick scaling test to see how eog/gimp/etc scale strokes.

ashes999 commented 8 years ago

@jcward that bit of code was last changed in this commit, please take a look at the commit (and/or the instigating issue) and double-check that you're not re-breaking something that was fixed.

jcward commented 8 years ago

That commit was correct, scaling is the .a and .d components as noted in the Flash matrix docs:

image image

I'm adding a testcase and creating a PR now...

ashes999 commented 8 years ago

Thanks for spending so much time on this, and for adding test cases to all your PRs. We really need to build that up to get this library more stable.

jcward commented 8 years ago

@ashes999 no problem, I want this library to work well, and testcases is how we'll get there without regressions! :)

ashes999 commented 8 years ago

@jcward if you fix one (and only one) SVG issue, I would like to suggest you pick this one. I think this is what's causing all of our existing test images to be rendered with a non-zero difference. (This also prevents us from using smaller test images, because the number of pixels changed becomes a larger percentage of the overall image...)

jcward commented 8 years ago

@ashes999 well, I don't know if you'll get 100% image match, but yes, even small strokes are slightly off. There is a fix in PR #36.

ashes999 commented 8 years ago

Ugh, it's been sitting in PR for a while. I don't know much about the code-base, and I could accept, but I'd rather someone more experienced takes a look at your changes.

Sorry, I know that's not very useful.

jcward commented 8 years ago

Yeah, no worries, probably best to get their feedback. :)