memononen / nanosvg

Simple stupid SVG parser
zlib License
1.68k stars 355 forks source link

Proper gradient support #34

Closed chrismile closed 9 years ago

chrismile commented 9 years ago

If I rasterize an object with a (linear) gradient that has a non-identity transform, it is rendered incorrectly (similar to the transform not being set at all). Here is an example:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
<defs>
<linearGradient inkscape:collect="always" id="linearGradient4416-4" x1="457.85709" y1="640.57642" x2="458.39285" y2="625.93359" gradientUnits="userSpaceOnUse" gradientTransform="matrix(1,0,0,0.95454547,86.600955,30.474306)">
    <stop style="stop-color:#494949;stop-opacity:0" offset="0" id="stop4412"/>
    <stop style="stop-color:#464646;stop-opacity:1" offset="1" id="stop4414"/>
</linearGradient>
</defs>
<g id="layer1">
<path transform="matrix(2.2560027,0,0,2.2560027,-422.41146,-1104.3522)" style="fill:url(#linearGradient4416-4);fill-opacity:1" id="path4408-7" d="M 542.851,635.456 m -18.2143,0 a 18.2143,10.0893 0 1 0 36.4286,0 a 18.2143,10.0893 0 1 0 -36.4286,0"/>
</g>
</svg>

NanoSVG rasterizer (my program is applying a blur to the image): lantern01_blur2

Qt & Inkscape rasterizer (with same blurring code): lantern01_blur2

Note: The object is not in the center. So when viewing the file in Inkscape, you have to zoom out a bit.

Are you still interested in working on gradients/NanoSVG? A lot of people would be really grateful if you'd fix these bugs, as NanoSVG is the best lightweight SVG rasterizer out there. There are also libraries like Qt and Cairo, but getting them to work on Android devices could be really hard.

Buggy gradients seem to be a problem for some months now: https://github.com/memononen/nanosvg/issues/26 (look especially on the linear gradient on the left of the picture) https://github.com/memononen/nanosvg/issues/27

I can also reproduce this bug: https://github.com/memononen/nanosvg/issues/21 NanoSVG isn't able to dereference "xlink:href" properly, as it doesn't remove the hash in front of the id.

memononen commented 9 years ago

Yes, I'm willing to improve gradient support, but currently I'm super slim on time (building a house).

Fixing the xlink:href should be quite easy, gradientUnits=userSpaceOnUse is a bit harder, but surely not impossible. These fixes should be done in the parser side. [edit] Your example svg snippet should just work, so that is actually a bug. I mixed the gradients units to the relative one (#21).

I'm happy to help you to figure out how to fix them.

chrismile commented 9 years ago

Yes, I can imagine that building a house is really time consuming. I'll see how much I can do to help fixing the gradients. "xlink:href" is mostly trivial. Currently, I only changed the code to remove the hash at the front. But of course there is still // TODO: use ref to fill in all unset values too. But still not so hard to code.

I can provide a patch as soon as the other things are working, too. Then there's userSpaceOnUse. If I understand it correctly, userSpaceOnUse means local shape coordinates and objectBoundingBox bounding box coordinates. But I don't understand what your code is doing in "nsvg__createGradient" so far.

nsvg__xformMultiply(grad->xform, attr->xform);
nsvg__xformMultiply(grad->xform, data->xform);

data->xform seems to be "gradientTransform", but what is "attr->xform"?

And which space are you computing in the following code? I suppose it is bounding box space -> gradient space?

// Calculate transform aligned to the line
dx = data->linear.x2 - data->linear.x1;
dy = data->linear.y2 - data->linear.y1;
grad->xform[0] = dy; grad->xform[1] = -dx;
grad->xform[2] = dx; grad->xform[3] = dy;
grad->xform[4] = data->linear.x1; grad->xform[5] = data->linear.y1;

What do you mean with

I mixed the gradients units to the relative one

in this context? Do you mean that you've mistaken the units in my example for relative coordinates at first?

Your example svg snippet should just work, so that is actually a bug.

That astonishes me. Why should it work when "gradientUnits=userSpaceOnUse" is not yet supported? Maybe I'm just missing something. It's really hard to understand code you haven't written yourself.

chrismile commented 9 years ago

OK, so it seems to work quite well now. I guess you meant to say that gradientUnits="objectBoundingBox" is not yet supported, and not "userSpaceOnUse". I've filed a pull request with some changes: https://github.com/memononen/nanosvg/pull/35

Fixed/added the following things:

So I guess you can close the following reports: https://github.com/memononen/nanosvg/issues/29 https://github.com/memononen/nanosvg/issues/27 https://github.com/memononen/nanosvg/issues/21

For https://github.com/memononen/nanosvg/issues/21 I believe there's still offsetting the radial gradient focus point missing. And then there's still inheritation of attributes... There are 3 TODOs left in code for this:

static NSVGgradient* nsvg__createGradient(NSVGparser* p, const char* id, const float* bounds, char* paintType)
[...]
// TODO: use ref to fill in all unset values too.
[...]

static char nsvg__parseLineCap(const char* str)
[...]
// TODO: handle inherit.
[...]

static char nsvg__parseLineJoin(const char* str)
[...]
// TODO: handle inherit.
[...]

I guess this could still be some work. Currently, NanoSVG stores attributes like a gradient's "spread" as chars. In functions like "nsvg__createGradient" there is no way to see whether a value is not set and just initialized to the standard value or it is actually set to the standard value in the file (and therefore mustn't be inherited). The SVG standard states

Any ‘linearGradient’ attributes which are defined on the referenced element which are not defined on this element are inherited by this element.

This could result in some elemental changes in the underlying attribute system. But for now, all features I need seems to work.

memononen commented 9 years ago

Good fixes, I added one minor nag about style. Otherwise it was good.

One option would be to use some value in the attributes which signals that it is not set (i.e. NaN, or -1 depending on value), and make sure a nice default is set in nsvg__addShape(). Another solution would be to use flags or booleans to define if a certain value is set.

chrismile commented 9 years ago

I'll see if I have some time in the following days to have a look at it. If the values are initialized with -1, the data type might need to change as the C/C++ standard doesn't define whether a char is signed or unsigned. Different compilers handle it in different ways.

memononen commented 8 years ago

Just wanted to let you know that I fixed number of issues with gradients and percent coords. If you have time, please test it and see if it works for you.

chrismile commented 8 years ago

Sorry for the late update. Yes, it works for my test SVGs. Do you currently have any plans to look further into https://github.com/memononen/nanosvg/issues/21, too?