memononen / nanosvg

Simple stupid SVG parser
zlib License
1.71k stars 363 forks source link

Incorrect processing/rendering of linear gradients #75

Closed Rado-1 closed 7 years ago

Rado-1 commented 8 years ago

Linear gradient from this sample svg file is not rendered properly, Original picture looks in Inkscape or web browser like this: scr1 and it is rendered (in my NanoVG renderer but also in NanoSVG rasterizer) as: scr2

The same shift of gradient positions happens also on Y axis when rotated by 90 degrees.

X-Ryl669 commented 7 years ago

This patch should fix it.

diff --git a/src/nanosvg.h b/src/nanosvg.h
index ebfc584..05d7ce7 100644
--- a/src/nanosvg.h
+++ b/src/nanosvg.h
@@ -2704,6 +2704,8 @@ static void nsvg__scaleGradient(NSVGgradient* grad, float tx, float ty, float sx
        grad->xform[1] *= sx;
        grad->xform[2] *= sy;
        grad->xform[3] *= sy;
+       grad->xform[4] *= sy;
+       grad->xform[5] *= sy;
        grad->xform[4] += tx*sx;
        grad->xform[5] += ty*sx;
 }

The idea is that scaleGradient must scale an existing translation first before adding an a new translation. Please report if it works for you, and if so, I'll make a PR for it.

X-Ryl669 commented 7 years ago

@memononen Can you test this on your test suite to ensure it's working ? It's working for me on the given svg file + example svg in codebase and the solution seems logical

Rado-1 commented 7 years ago

I tested it on the original file, as well as on additional samples and linear gradients are working perfect with this fix now. After pushing it to git, you can close the issue. Thanks!

memononen commented 7 years ago

I apogolize that I do not have time to properly test this. I think there's one more bug in the scaling: ty*sx. Should probably use sy instead.

@X-Ryl669 Can I ask you to test if this works too?

float t[6];
nsvg__xformSetTranslation(t, tx, ty);
nsvg__xformMultiply (grad->xform, t);

nsvg__xformSetScale(t, sx, sy);
nsvg__xformMultiply (grad->xform, t);

It's a bit more code to execute, but I think it's more clear on intent.

Rado-1 commented 7 years ago

I did not observe some problems with ty sx on my quick tests, but changing to ty sy works fine as well. Also replacing it by the aforementioned code worked fine.

X-Ryl669 commented 7 years ago

Works for me too, so did a PR #92

memononen commented 7 years ago

Merged the fix, closing as fixed.