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.

Sign up for GitHub

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

Fix gradient parsing #5836

Merged
merged 19 commits into from
Aug 12, 2019
Merged

Fix gradient parsing #5836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d391c63
a test file
asturur Jul 22, 2019
f23bf1a
improvements
asturur Aug 10, 2019
001218d
improvements
asturur Aug 10, 2019
d029fc8
improvements
asturur Aug 10, 2019
fe3d5a8
fixed tests
asturur Aug 10, 2019
2cdc443
fix lint
asturur Aug 11, 2019
78f594b
fix lint
asturur Aug 11, 2019
46f60e4
changes to jsdoc
asturur Aug 11, 2019
69f9dff
more jsdocs and deprecations
asturur Aug 11, 2019
05aec06
some small extra test
asturur Aug 11, 2019
01fe62a
fixed typo
asturur Aug 11, 2019
5e2e845
more tests
asturur Aug 11, 2019
54f20de
fix some gradients export
asturur Aug 11, 2019
695c13a
added more code
asturur Aug 12, 2019
c2a697a
lint in tests
asturur Aug 12, 2019
7931ee6
added a visual test
asturur Aug 12, 2019
8843f53
added a visual test for svg export
asturur Aug 12, 2019
14db4d7
but i killed a test
asturur Aug 12, 2019
b8a8c32
added a simple UT
asturur Aug 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/elements_parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ fabric.ElementsParser = function(elements, callback, options, reviver, parsingOp
var gradientDef = this.extractPropertyDefinition(obj, property, 'gradientDefs');
if (gradientDef) {
var opacityAttr = el.getAttribute(property + '-opacity');
var gradient = fabric.Gradient.fromElement(gradientDef, obj, opacityAttr);
var gradient = fabric.Gradient.fromElement(gradientDef, obj, opacityAttr, this.options);
obj.set(property, gradient);
}
};
Expand Down
192 changes: 124 additions & 68 deletions src/gradient.class.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,18 +95,67 @@
*/
offsetY: 0,

/**
* A transform matrix to apply to the gradient before painting.
* Imported from svg gradients, is not applied with the current transform in the center.
* Before this transform is applied, the origin point is at the top left corner of the object
* plus the addition of offsetY and offsetX.
* @type Array[Number]
* @default null
*/
gradientTransform: null,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. painting
  2. behaves differently how? Describe the layout and the members of this matrix, please, enough to describe them for people coming along in the future.
/**
 * | x1 x2 0 0 
 * | ... |
 * ...
 * x1: a description of x1
 * ...
 */

Yes, I know, JavaDoc is probably insufficient for this (I've never seen a 2-D array defined in JavaDoc in detail), but some documentation of the members is better than none.

Even better would be to make this an instance of an explicit unit-testable matrix class. I realize that would be a significant refactor, but I'd be very surprised if you didn't have a need for it by now elsewhere in the code. I do see there's a fabric.ColorMatrix class, but that appears to be something else.


/**
* coordinates units for coords.
* If `pixels`, the number of coords are in the same unit of width / height.
* If set as `percentage` the coords are still a number, but 1 means 100% of width
* for the X and 100% of the height for the y. It can be bigger than 1 and negative.
* @type String pixels || percentage
* @default 'pixels'
*/
gradientUnits: 'pixels',

/**
* Gradient type
* @type String linear || radial
* @default 'pixels'
*/
type: 'linear',

/**
* Constructor
* @param {Object} [options] Options object with type, coords, gradientUnits and colorStops
* @param {Object} options Options object with type, coords, gradientUnits and colorStops
* @param {Object} [options.type] gradient type linear or radial
* @param {Object} [options.gradientUnits] gradient units
* @param {Object} [options.offsetX] SVG import compatibility
* @param {Object} [options.offsetY] SVG import compatibility
* @param {Array[Object]} options.colorStops contains the colorstops.
* @param {Object} options.coords contains the coords of the gradient
* @param {Number} [options.coords.x1] X coordiante of the first point for linear or of the focal point for radial
* @param {Number} [options.coords.y1] Y coordiante of the first point for linear or of the focal point for radial
* @param {Number} [options.coords.x2] X coordiante of the second point for linear or of the center point for radial
* @param {Number} [options.coords.y2] Y coordiante of the second point for linear or of the center point for radial
* @param {Number} [options.coords.r1] only for radial gradient, radius of the inner circle
* @param {Number} [options.coords.r2] only for radial gradient, radius of the external circle
* @return {fabric.Gradient} thisArg
*/
initialize: function(options) {
options || (options = { });
options.coords || (options.coords = { });

var coords = { };
var coords, _this = this;

this.id = fabric.Object.__uid++;
this.type = options.type || 'linear';
// sets everything, then coords and colorstops get sets again
Object.keys(options).forEach(function(option) {
_this[option] = options[option];
});

if (this.id) {
this.id += '_' + fabric.Object.__uid++;
}
else {
this.id = fabric.Object.__uid++;
}

coords = {
x1: options.coords.x1 || 0,
Expand All @@ -119,13 +168,9 @@
coords.r1 = options.coords.r1 || 0;
coords.r2 = options.coords.r2 || 0;
}

this.coords = coords;
this.colorStops = options.colorStops.slice();
if (options.gradientTransform) {
this.gradientTransform = options.gradientTransform;
}
this.offsetX = options.offsetX || this.offsetX;
this.offsetY = options.offsetY || this.offsetY;
},

/**
Expand Down Expand Up @@ -157,6 +202,7 @@
colorStops: this.colorStops,
offsetX: this.offsetX,
offsetY: this.offsetY,
gradientUnits: this.gradientUnits,
gradientTransform: this.gradientTransform ? this.gradientTransform.concat() : this.gradientTransform
};
fabric.util.populateWithProperties(this, object, propertiesToInclude);
Expand All @@ -175,23 +221,33 @@
markup, commonAttributes, colorStops = clone(this.colorStops, true),
needsSwap = coords.r1 > coords.r2,
transform = this.gradientTransform ? this.gradientTransform.concat() : fabric.iMatrix.concat(),
offsetX = object.width / 2 - this.offsetX, offsetY = object.height / 2 - this.offsetY,
withViewport = !!options.additionalTransform;
offsetX = -this.offsetX, offsetY = -this.offsetY,
withViewport = !!options.additionalTransform,
gradientUnits = this.gradientUnits === 'pixels' ? 'userSpaceOnUse' : 'objectBoundingBox';
// colorStops must be sorted ascending
colorStops.sort(function(a, b) {
return a.offset - b.offset;
});

if (gradientUnits === 'objectBoundingBox') {
offsetX /= object.width;
offsetY /= object.height;
}
else {
offsetX += object.width / 2;
offsetY += object.height / 2;
}
if (object.type === 'path') {
offsetX -= object.pathOffset.x;
offsetY -= object.pathOffset.y;
}


transform[4] -= offsetX;
transform[5] -= offsetY;

commonAttributes = 'id="SVGID_' + this.id +
'" gradientUnits="userSpaceOnUse"';
'" gradientUnits="' + gradientUnits + '"';
commonAttributes += ' gradientTransform="' + (withViewport ?
options.additionalTransform + ' ' : '') + fabric.util.matrixToSVG(transform) + '" ';

Expand Down Expand Up @@ -303,11 +359,17 @@
* @param {SVGGradientElement} el SVG gradient element
* @param {fabric.Object} instance
* @param {String} opacityAttr A fill-opacity or stroke-opacity attribute to multiply to each stop's opacity.
* @param {Object} svgOptions an object containing the size of the SVG in order to parse correctly graidents
* that uses gradientUnits as 'userSpaceOnUse' and percentages.
* @param {Object.number} viewBoxWidth width part of the viewBox attribute on svg
* @param {Object.number} viewBoxHeight height part of the viewBox attribute on svg
* @param {Object.number} width width part of the svg tag if viewBox is not specified
* @param {Object.number} height height part of the svg tag if viewBox is not specified
* @return {fabric.Gradient} Gradient instance
* @see http://www.w3.org/TR/SVG/pservers.html#LinearGradientElement
* @see http://www.w3.org/TR/SVG/pservers.html#RadialGradientElement
*/
fromElement: function(el, instance, opacityAttr) {
fromElement: function(el, instance, opacityAttr, svgOptions) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might as well add a javadoc line for svgOptions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, i m not done yet, svg export is now broken and needs to be adequate to the new logic.

/**
* @example:
*
Expand Down Expand Up @@ -349,104 +411,98 @@

var colorStopEls = el.getElementsByTagName('stop'),
type,
gradientUnits = el.getAttribute('gradientUnits') || 'objectBoundingBox',
gradientTransform = el.getAttribute('gradientTransform'),
gradientUnits = el.getAttribute('gradientUnits') === 'userSpaceOnUse' ?
'pixels' : 'percentage',
gradientTransform = el.getAttribute('gradientTransform') || '',
colorStops = [],
coords, ellipseMatrix, i;

coords, i, offsetX = 0, offsetY = 0,
transformMatrix;
if (el.nodeName === 'linearGradient' || el.nodeName === 'LINEARGRADIENT') {
type = 'linear';
coords = getLinearCoords(el);
}
else {
type = 'radial';
}

if (type === 'linear') {
coords = getLinearCoords(el);
}
else if (type === 'radial') {
coords = getRadialCoords(el);
}

for (i = colorStopEls.length; i--; ) {
colorStops.push(getColorStop(colorStopEls[i], multiplier));
}

ellipseMatrix = _convertPercentUnitsToValues(instance, coords, gradientUnits);
transformMatrix = fabric.parseTransformAttribute(gradientTransform);

__convertPercentUnitsToValues(instance, coords, svgOptions, gradientUnits);

if (gradientUnits === 'pixels') {
offsetX = -instance.left;
offsetY = -instance.top;
}

var gradient = new fabric.Gradient({
id: el.getAttribute('id'),
type: type,
coords: coords,
colorStops: colorStops,
offsetX: -instance.left,
offsetY: -instance.top
gradientUnits: gradientUnits,
gradientTransform: transformMatrix,
offsetX: offsetX,
offsetY: offsetY,
});

if (gradientTransform || ellipseMatrix !== '') {
gradient.gradientTransform = fabric.parseTransformAttribute((gradientTransform || '') + ellipseMatrix);
}

return gradient;
},
/* _FROM_SVG_END_ */

/**
* Returns {@link fabric.Gradient} instance from its object representation
* this function is uniquely used by Object.setGradient and is deprecated with it.
* @static
* @deprecated since 3.4.0
* @memberOf fabric.Gradient
* @param {Object} obj
* @param {Object} [options] Options object
*/
forObject: function(obj, options) {
options || (options = { });
_convertPercentUnitsToValues(obj, options.coords, 'userSpaceOnUse');
__convertPercentUnitsToValues(obj, options.coords, options.gradientUnits, {
// those values are to avoid errors. this function is uniquely used by
viewBoxWidth: 100,
viewBoxHeight: 100,
});
return new fabric.Gradient(options);
}
});

/**
* @private
*/
function _convertPercentUnitsToValues(object, options, gradientUnits) {
var propValue, addFactor = 0, multFactor = 1, ellipseMatrix = '';
for (var prop in options) {
if (options[prop] === 'Infinity') {
options[prop] = 1;
}
else if (options[prop] === '-Infinity') {
options[prop] = 0;
function __convertPercentUnitsToValues(instance, options, svgOptions, gradientUnits) {
var propValue, finalValue;
Object.keys(options).forEach(function(prop) {
propValue = options[prop];
if (propValue === 'Infinity') {
finalValue = 1;
}
propValue = parseFloat(options[prop], 10);
if (typeof options[prop] === 'string' && /^(\d+\.\d+)%|(\d+)%$/.test(options[prop])) {
multFactor = 0.01;
else if (propValue === '-Infinity') {
finalValue = 0;
}
else {
multFactor = 1;
}
if (prop === 'x1' || prop === 'x2' || prop === 'r2') {
multFactor *= gradientUnits === 'objectBoundingBox' ? object.width : 1;
addFactor = gradientUnits === 'objectBoundingBox' ? object.left || 0 : 0;
}
else if (prop === 'y1' || prop === 'y2') {
multFactor *= gradientUnits === 'objectBoundingBox' ? object.height : 1;
addFactor = gradientUnits === 'objectBoundingBox' ? object.top || 0 : 0;
}
options[prop] = propValue * multFactor + addFactor;
}
if (object.type === 'ellipse' &&
options.r2 !== null &&
gradientUnits === 'objectBoundingBox' &&
object.rx !== object.ry) {

var scaleFactor = object.ry / object.rx;
ellipseMatrix = ' scale(1, ' + scaleFactor + ')';
if (options.y1) {
options.y1 /= scaleFactor;
}
if (options.y2) {
options.y2 /= scaleFactor;
finalValue = parseFloat(options[prop], 10);
if (typeof propValue === 'string' && /^(\d+\.\d+)%|(\d+)%$/.test(propValue)) {
finalValue *= 0.01;
if (gradientUnits === 'pixels') {
// then we need to fix those percentages here in svg parsing
if (prop === 'x1' || prop === 'x2' || prop === 'r2') {
finalValue *= svgOptions.viewBoxWidth || svgOptions.width;
}
if (prop === 'y1' || prop === 'y2') {
finalValue *= svgOptions.viewBoxHeight || svgOptions.height;
}
}
}
}
}
return ellipseMatrix;
options[prop] = finalValue;
});
}
})();
Loading