Fabric fails to correctly import SVG gradients #5810

ajvincent · 2019-07-19T08:00:15Z

Version

2.4.0

Test Case

http://jsfiddle.net/ajvincent/b3xL0v2g/4/

Information about environment

Mozilla Firefox 68.0, Google Chrome 75.0.3770.142

Steps to reproduce

None.

Expected Behavior

The red, green and blue sides of the triangle should each have a gradient leading to yellow in the center.

Actual Behavior

The red, green, and blue sides of the triangle are solid colors.

Other notes

Possible duplicate of Shape gradients with percentage based coordination don't work correctly. #3665 .
The SVG source code is included in-line, which should make it relatively easy to compare to the generated canvas.
There are gradients that partially work in this image (red-to-green, green-to-blue, blue-to-red), but it's worth noting that the opacity isn't working.

ajvincent · 2019-07-20T07:49:00Z

http://victorblog.com/html5-canvas-gradient-creator/
This site has a pretty good example of what we should expect. The code is MIT-licensed, but considering that FabricJS does its work with matrices, it won't port over for us anyway.

ajvincent · 2019-07-20T08:39:59Z

I'm afraid I really stink at linear algebra. Not nearly enough practice in recent years.

I think the place to fix this is in src/gradient.class.js, fromElement, just after gradient.gradientTransform is established. The problem is that the coords object has no information regarding the height of the bounding box. This coords object is fully converted away from percentages in _convertPercentUnitsToValues.

Without the height, determining the correct rotation of the bounding box is impossible.

asturur · 2019-07-20T11:26:59Z

a suggestion:
http://jsfiddle.net/nkej2d7y/
When you have to show a difference in gradients or svg import/export, please show both of them in the fiddle.

ajvincent · 2019-07-20T20:13:55Z

I think my earlier assertion that fromElement is the right place to fix this is incorrect now, as it doesn't cover the case in #3665... which is essentially the same problem without the SVG.

asturur · 2019-07-21T08:03:53Z

ok i get the geometric problem.
In _convertPercentUnitsToValues we need to parse the gradientTransform and decompose it with fabric.util.qrDecomose.

Once we do that, we take away the angle property of the decomposition and we redo the matrix with the rest.

We use the angle to rotate x1, x2, y1, y2. Indeed a gradient rotated of 90 degree is swapping width with height basically. ( x1 - x2 with y1 - y2 ).

May break everything else, so needs to be done with an eye over:
http://fabricjs.com/test/svg_import/

asturur · 2019-07-21T08:06:30Z

Well i would say before i need to craft an example with many weird gradients that can fail.

ajvincent · 2019-07-21T16:25:45Z

I'm sorry, but I won't be in a good position to fix this one. Too many moving parts and not enough time.

asturur · 2019-07-21T17:26:25Z

no worries at all, i was planning to do it, is complicated and requires hostorical memory on that area. You provided a failing example and fixed the opacity. That is good

asturur · 2019-07-22T04:15:24Z

This is a small start with tests:

FABRIC

SVG

this is going to be hard

ajvincent · 2019-07-22T15:33:28Z

Here's what I think is happening:

The colors are mostly right, just that the gradients aren't steep enough. The actual final gradients we want are past the 0% to 100% range. This arises naturally from the Pythagorean theorem. For purposes of the gradient, you're calculating the a^2 part (the x value of the bounding box) but not the b^2 part (the y value).

So when the gradient is applied, the gradient box is computed for a shorter length than it should be. When you're past that length, you're effectively past the 100% point and you get locked into one color. This is why I was pointing out that the coords object doesn't have all the information it needs to fix this.

ajvincent · 2019-07-22T15:51:38Z

I think your work on this should be organized into three distinct parts, with unit tests at each part:

Populating the height of the gradient's bounding box
Computing the hypotenuse of the bounding box
Rotating the bounding box about its center

For the last part, I'd say simply follow the example from victorblog above: compute the new height and width with trigonometry, then adjust the bounding box's x1, y1, x2, y2 distances from cx and cy as being half the new width and height.

asturur · 2019-07-22T17:53:20Z

the problem is the logic in general.
Fabric uses the same code for radial and linear gradient, while it may be that we need to differentiate.

Canvas does not have a gradient transform, and i need to decide how the fabric gradientTransform relate to the SVG one ( but import has to work good anyway ).

The default gradient space reference is the object bounding box, when the transform has a rotation, the width and the height change meaning.
Now it could be that i have to calculate the bounding box of the rotated object and apply the old logic on that. That would solve for 90 degree rotation, unsure for middle ones.

All those examples needs to be duplicated with custom coordinates ( here using the defaults ) percentage based and non, objectBoundingBox or UserSpaceUse.

Then there is skew and flip that i did not expect at all.

I still need to build more examples combining more features to reverse engineer how they work. on the w3c website things are described, but they use bigger conceptual blocks that i do not know, so i have difficulties understanding how the SVG specs works

ajvincent · 2019-07-27T01:26:53Z

Since I'm dealing with SVG path elements, I hope to create a path element whose gradient is horizontal, and then both translate it and rotate it for Fabric to get the correct rendering where I currently have incorrect gradients now. I'm going to attempt this tonight.

ajvincent · 2019-07-27T02:00:10Z

Victory: with the opacity fix landed, I should be unblocked.

asturur · 2019-07-27T20:39:26Z

I'll try to collect more examples and work on much data as possible

<svg xmlns="http://www.w3.org/2000/svg" width="760" height="760"
         xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 380 380">
<defs>
<linearGradient id="three_stops_1">
    <stop offset="0%" style="stop-color: #ffcc00;"/>
    <stop offset="33.3%" style="stop-color: #cc6699"/>
    <stop offset="100%" style="stop-color: #66cc99;"/>
</linearGradient>
<linearGradient id="three_stops_2">
    <stop offset="0%" style="stop-color: #ffcc00;"/>
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 0.5"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 0.9"/>
</linearGradient>
<linearGradient id="three_stops_3" gradientTransform="rotate(45)">
    <stop offset="0%" style="stop-color: #ffcc00;"/>
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</linearGradient>
<linearGradient id="three_stops_4" gradientTransform="rotate(90)">
    <stop offset="0%" style="stop-color: #ffcc00;"/>
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</linearGradient>
<linearGradient id="three_stops_5" gradientTransform="rotate(90) translate(0.5, 0)">
    <stop offset="0%" style="stop-color: #ffcc00;"/>
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</linearGradient>
<linearGradient id="three_stops_6" gradientTransform="translate(0.5, 0) rotate(90)">
    <stop offset="0%" style="stop-color: #ffcc00;"/>
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</linearGradient>
<linearGradient id="three_stops_7" gradientTransform="translate(0, 0.5) rotate(90)">
    <stop offset="0%" style="stop-color: #ffcc00;"/>
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</linearGradient>
<linearGradient id="three_stops_8" gradientTransform="translate(0, 0.5) scale(2, 1)">
    <stop offset="0%" style="stop-color: #ffcc00;"/>
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</linearGradient>
<linearGradient id="three_stops_9" gradientTransform="translate(0, 0.5) scale(1, 2)">
    <stop offset="0%" style="stop-color: #ffcc00;"/>
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</linearGradient>
<linearGradient id="three_stops_10" gradientTransform="translate(0.5, 0.5) scale(-1, 2)">
    <stop offset="0%" style="stop-color: #ffcc00;"/>
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</linearGradient>
<linearGradient id="three_stops_11" gradientTransform="skewX(30)">
    <stop offset="0%" style="stop-color: #ffcc00;"/>
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</linearGradient>
<radialGradient id="radial_1" gradientUnits="objectBoundingBox" r="0.8">
    <stop offset="0%" style="stop-color: #ffcc00;" />
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</radialGradient>
<radialGradient id="radial_2" gradientUnits="objectBoundingBox" cx="0.5" cy="0.1" r="0.8">
    <stop offset="0%" style="stop-color: #ffcc00;" />
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</radialGradient>
<radialGradient id="radial_3" gradientTransform="rotate(45)" gradientUnits="objectBoundingBox" r="0.8">
    <stop offset="0%" style="stop-color: #ffcc00;" />
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</radialGradient>
<radialGradient id="radial_4" gradientTransform="rotate(90)" cx="0.5" cy="0.1" r="0.8">
    <stop offset="0%" style="stop-color: #ffcc00;" />
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</radialGradient>
<radialGradient id="radial_5" fx="0.5" fy="0.9" cx="0.5" cy="0.2" r="0.8">
    <stop offset="0%" style="stop-color: #ffcc00;" />
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</radialGradient>
<radialGradient id="radial_6" gradientTransform="rotate(30)" fx="0.4" fy="0.7" cx="0.5" cy="0.2" r="0.8">
    <stop offset="0%" style="stop-color: #ffcc00;" />
    <stop offset="33.3%" style="stop-color: #cc6699; stop-opacity: 1"/>
    <stop offset="100%" style="stop-color: #66cc99; stop-opacity: 1"/>
</radialGradient>
<polygon id="base" points="5,5 35,5 35,40 5,40 5,35 30,35 30,10 5,10" />
<rect width="35" height="80" id="base2" />
</defs>
  <use href="#base" style="fill: url(#three_stops_1); stroke: black;"/>
  <use href="#base"  transform="translate(45, 0)" style="fill: url(#three_stops_2); stroke: black;"/>
  <use href="#base"  fill-opacity="0.5" transform="translate(90, 0)" style="fill: url(#three_stops_2); stroke: black;"/>
  <use href="#base"  transform="translate(135, 0)" style="fill: url(#three_stops_3); stroke: black;"/>
  <polygon transform="translate(0, 45)" points="5,5 80,5 80,40 5,40 5,35 75,35 75,10 5,10" style="fill: url(#three_stops_4); stroke: black;"/>
  <use href="#base"  transform="translate(90, 45)" style="fill: url(#three_stops_5); stroke: black;"/>
  <use href="#base"  transform="translate(135, 45)" style="fill: url(#three_stops_6); stroke: black;"/>
  <use href="#base"  transform="translate(0, 90)" style="fill: url(#three_stops_7); stroke: black;"/>
  <use href="#base"  transform="translate(45, 90)"  style="fill: url(#three_stops_8); stroke: black;"/>
  <use href="#base"  transform="translate(90, 90)" style="fill: url(#three_stops_9); stroke: black;"/>
  <use href="#base"  transform="translate(135, 90)" style="fill: url(#three_stops_10); stroke: black;"/>
  <use href="#base"  transform="translate(0, 135)" style="fill: url(#three_stops_11); stroke: black;"/>
  <use href="#base2"  transform="translate(185, 5)" style="fill: url(#radial_1); stroke: black;"/>
  <use href="#base2"  transform="translate(235, 5)" style="fill: url(#radial_2); stroke: black;"/>
  <use href="#base2"  transform="translate(285, 5)" style="fill: url(#radial_3); stroke: black;"/>
  <use href="#base2"  transform="translate(335, 5)" style="fill: url(#radial_4); stroke: black;"/>
  <use href="#base2"  transform="translate(185, 95)" style="fill: url(#radial_5); stroke: black;"/>
  <use href="#base2"  transform="translate(235, 95)" style="fill: url(#radial_6); stroke: black;"/>
</svg>

asturur · 2019-08-07T12:15:06Z

spent LOT of time on this. Got crazy pulled out nothing.
There is something i do not undertand in the conversion from bounding box unit.

ajvincent · 2019-08-08T06:31:04Z

OK, it's become painfully obvious to me in an hour's experimentation that I don't understand transforms on linear gradients in SVG either, much less canvas. How I thought it worked is clearly wrong, including my bounding box suggestion, and this probably should be listed as unsupported for the foreseeable future.

Getting a mental model of this right is going to be a nightmare, and is way too much to ask of Fabric without a professional need for it that can't be worked around.

Honestly, I'm not even sure now that there is a bug in Fabric here. Certainly no one should be messing with gradient transforms they don't understand.

My sincerest apologies for sending you down the rabbit hole...

asturur · 2019-08-08T11:07:21Z

well just a particular combination of transform are parsed bad, most are ok.

asturur · 2019-08-10T05:03:19Z

ok so update:

SVG:

CANVAS:

So i deleted most of the code i had, and wrote code that parse correctly LINEAR gradients, randomly transformed, with unit type Object bounding box.

25% done.

asturur · 2019-08-10T05:24:30Z

50% done!!!!

asturur · 2019-08-10T05:40:51Z

this fix you svg too.

asturur · 2019-08-10T08:19:09Z

and the UserSpaceOnUse gradients works too:

I think we are good, now i have to see how tests pass.
We also got a new gradient feature!

ajvincent mentioned this issue Jul 20, 2019

Implement fill-opacity on Path objects with gradients. #5812

Merged

asturur added bug svg_parser labels Jul 20, 2019

asturur mentioned this issue Aug 10, 2019

Fix gradient parsing #5836

Merged

asturur closed this as completed in #5836 Aug 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fabric fails to correctly import SVG gradients #5810

Fabric fails to correctly import SVG gradients #5810

ajvincent commented Jul 19, 2019 •

edited

Loading

ajvincent commented Jul 20, 2019

ajvincent commented Jul 20, 2019

asturur commented Jul 20, 2019

ajvincent commented Jul 20, 2019

asturur commented Jul 21, 2019

asturur commented Jul 21, 2019

ajvincent commented Jul 21, 2019

asturur commented Jul 21, 2019

asturur commented Jul 22, 2019

ajvincent commented Jul 22, 2019

ajvincent commented Jul 22, 2019 •

edited

Loading

asturur commented Jul 22, 2019 •

edited

Loading

ajvincent commented Jul 27, 2019

ajvincent commented Jul 27, 2019

asturur commented Jul 27, 2019 •

edited

Loading

asturur commented Aug 7, 2019

ajvincent commented Aug 8, 2019 •

edited

Loading

asturur commented Aug 8, 2019

asturur commented Aug 10, 2019

asturur commented Aug 10, 2019

asturur commented Aug 10, 2019

asturur commented Aug 10, 2019

Fabric fails to correctly import SVG gradients #5810

Fabric fails to correctly import SVG gradients #5810

Comments

ajvincent commented Jul 19, 2019 • edited Loading

Version

Test Case

Information about environment

Steps to reproduce

Expected Behavior

Actual Behavior

Other notes

ajvincent commented Jul 20, 2019

ajvincent commented Jul 20, 2019

asturur commented Jul 20, 2019

ajvincent commented Jul 20, 2019

asturur commented Jul 21, 2019

asturur commented Jul 21, 2019

ajvincent commented Jul 21, 2019

asturur commented Jul 21, 2019

asturur commented Jul 22, 2019

ajvincent commented Jul 22, 2019

ajvincent commented Jul 22, 2019 • edited Loading

asturur commented Jul 22, 2019 • edited Loading

ajvincent commented Jul 27, 2019

ajvincent commented Jul 27, 2019

asturur commented Jul 27, 2019 • edited Loading

asturur commented Aug 7, 2019

ajvincent commented Aug 8, 2019 • edited Loading

asturur commented Aug 8, 2019

asturur commented Aug 10, 2019

asturur commented Aug 10, 2019

asturur commented Aug 10, 2019

asturur commented Aug 10, 2019

ajvincent commented Jul 19, 2019 •

edited

Loading

ajvincent commented Jul 22, 2019 •

edited

Loading

asturur commented Jul 22, 2019 •

edited

Loading

asturur commented Jul 27, 2019 •

edited

Loading

ajvincent commented Aug 8, 2019 •

edited

Loading