Skip to content
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

Proper gradient support #34

Closed
chrismile opened this issue May 26, 2015 · 7 comments
Closed

Proper gradient support #34

chrismile opened this issue May 26, 2015 · 7 comments

Comments

@chrismile
Copy link
Contributor

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:
#26 (look especially on the linear gradient on the left of the picture)
#27

I can also reproduce this bug: #21
NanoSVG isn't able to dereference "xlink:href" properly, as it doesn't remove the hash in front of the id.

@memononen
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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: #35

Fixed/added the following things:

So I guess you can close the following reports:
#29
#27
#21

For #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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

Sorry for the late update.
Yes, it works for my test SVGs.
Do you currently have any plans to look further into #21, too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants