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 Group.toSVG export missing opacity and visibility #5755

Merged
merged 9 commits into from
Jun 17, 2019
Merged

Fix Group.toSVG export missing opacity and visibility #5755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c500f34
ok ok
asturur Jun 15, 2019
2cf576d
the fix
asturur Jun 15, 2019
1049156
fixed some tests
asturur Jun 15, 2019
58bb156
better
asturur Jun 15, 2019
4c51bc0
fixed tests
asturur Jun 16, 2019
a84a118
fixed tests running together
asturur Jun 16, 2019
fc60e14
okish
asturur Jun 16, 2019
7d82a6f
adding a visual test
asturur Jun 17, 2019
06c3be2
smaller screenshot
asturur Jun 17, 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 .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ jobs:
- stage: Unit Tests
node_js: "8"
- stage: Visual Tests
env: LAUNCHER=Node CANFAIL=TRUE
env: LAUNCHER=Node
node_js: "10"
script: npm run build:fast && npm run test:visual
- stage: Visual Tests
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
"testem:ci": "testem ci"
},
"optionalDependencies": {
"canvas": "^2.5.0",
"canvas": "^2.6.0",
"jsdom": "15.1.0"
},
"devDependencies": {
Expand Down
25 changes: 13 additions & 12 deletions src/mixins/object.svg_export.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@
* @return {String} svg representation of an instance
*/
toSVG: function(reviver) {
return this._createBaseSVGMarkup(this._toSVG(), { reviver: reviver });
return this._createBaseSVGMarkup(this._toSVG(reviver), { reviver: reviver });
},

/**
Expand All @@ -181,7 +181,7 @@
* @return {String} svg representation of an instance
*/
toClipPathSVG: function(reviver) {
return '\t' + this._createBaseClipPathSVGMarkup(this._toSVG(), { reviver: reviver });
return '\t' + this._createBaseClipPathSVGMarkup(this._toSVG(reviver), { reviver: reviver });
},

/**
Expand All @@ -206,21 +206,22 @@
*/
_createBaseSVGMarkup: function(objectMarkup, options) {
options = options || {};
var noStyle = options.noStyle, withShadow = options.withShadow,
var noStyle = options.noStyle,
reviver = options.reviver,
styleInfo = noStyle ? '' : 'style="' + this.getSvgStyles() + '" ',
shadowInfo = withShadow ? 'style="' + this.getSvgFilter() + '" ' : '',
shadowInfo = options.withShadow ? 'style="' + this.getSvgFilter() + '" ' : '',
clipPath = this.clipPath,
vectorEffect = this.strokeUniform ? 'vector-effect="non-scaling-stroke" ' : '',
absoluteClipPath = this.clipPath && this.clipPath.absolutePositioned,
absoluteClipPath = clipPath && clipPath.absolutePositioned,
stroke = this.stroke, fill = this.fill, shadow = this.shadow,
commonPieces, markup = [], clipPathMarkup,
// insert commons in the markup, style and svgCommons
index = objectMarkup.indexOf('COMMON_PARTS'),
additionalTransform = options.additionalTransform;
if (clipPath) {
clipPath.clipPathId = 'CLIPPATH_' + fabric.Object.__uid++;
clipPathMarkup = '<clipPath id="' + clipPath.clipPathId + '" >\n' +
this.clipPath.toClipPathSVG(reviver) +
clipPath.toClipPathSVG(reviver) +
'</clipPath>\n';
}
if (absoluteClipPath) {
Expand All @@ -241,14 +242,14 @@
additionalTransform ? 'transform="' + additionalTransform + '" ' : '',
].join('');
objectMarkup[index] = commonPieces;
if (this.fill && this.fill.toLive) {
markup.push(this.fill.toSVG(this));
if (fill && fill.toLive) {
markup.push(fill.toSVG(this));
}
if (this.stroke && this.stroke.toLive) {
markup.push(this.stroke.toSVG(this));
if (stroke && stroke.toLive) {
markup.push(stroke.toSVG(this));
}
if (this.shadow) {
markup.push(this.shadow.toSVG(this));
if (shadow) {
markup.push(shadow.toSVG(this));
}
if (clipPath) {
markup.push(clipPathMarkup);
Expand Down
34 changes: 16 additions & 18 deletions src/shapes/group.class.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,31 +518,29 @@
* @param {Function} [reviver] Method for further parsing of svg representation.
* @return {String} svg representation of an instance
*/
toSVG: function(reviver) {
var svgString = [];
_toSVG: function(reviver) {
var svgString = ['<g ', 'COMMON_PARTS', ' >\n'];

for (var i = 0, len = this._objects.length; i < len; i++) {
svgString.push('\t', this._objects[i].toSVG(reviver));
svgString.push('\t\t', this._objects[i].toSVG(reviver));
}

return this._createBaseSVGMarkup(
this._toSVG(),
{ reviver: reviver, noStyle: true, withShadow: true });
svgString.push('</g>\n');
return svgString;
},

/**
* Returns svg representation of an instance
* @param {Function} [reviver] Method for further parsing of svg representation.
* @return {String} svg representation of an instance
* Returns styles-string for svg-export, specific version for group
* @return {String}
*/
_toSVG: function(reviver) {
var svgString = [];

for (var i = 0, len = this._objects.length; i < len; i++) {
svgString.push('\t', this._objects[i].toSVG(reviver));
}

return svgString;
getSvgStyles: function() {
var opacity = typeof this.opacity !== 'undefined' && this.opacity !== 1 ?
'opacity: ' + this.opacity + ';' : '',
visibility = this.visible ? '' : ' visibility: hidden;';
return [
opacity,
this.getSvgFilter(),
visibility
].join('');
},

/**
Expand Down
6 changes: 1 addition & 5 deletions test/lib/visualTestLoop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,6 @@
threshold: 0.095
};

function beforeEachHandler() {

}

return function testCallback(testObj) {
if (testObj.disabled) {
return;
Expand All @@ -123,7 +119,7 @@
var newModule = testObj.newModule;
if (newModule) {
QUnit.module(newModule, {
beforeEach: beforeEachHandler,
beforeEach: testObj.beforeEachHandler,
});
}
QUnit.test(testName, function(assert) {
Expand Down
5 changes: 2 additions & 3 deletions test/unit/canvas_static.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,16 +806,15 @@
canvas.renderOnAddRemove = false;
canvas.add(circle, rect, path1, tria, polygon, polyline, group, ellipse, image, pathGroup);

var reviverCount = 0,
len = canvas.size() + group.size() + pathGroup.size();
var reviverCount = 0;

function reviver(svg) {
reviverCount++;
return svg;
}

canvas.toSVG(null, reviver);
assert.equal(reviverCount, len);
assert.equal(reviverCount, 14);

canvas.renderOnAddRemove = true;
});
Expand Down
12 changes: 4 additions & 8 deletions test/unit/group.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,33 +447,29 @@
QUnit.test('toSVG', function(assert) {
var group = makeGroupWith2Objects();
assert.ok(typeof group.toSVG === 'function');

var expectedSVG = '<g transform=\"matrix(1 0 0 1 90 130)\" style=\"\" >\n\t<g transform=\"matrix(1 0 0 1 25 -25)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-15\" y=\"-5\" rx=\"0\" ry=\"0\" width=\"30\" height=\"10\" />\n</g>\n\t<g transform=\"matrix(1 0 0 1 -35 10)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-5\" y=\"-20\" rx=\"0\" ry=\"0\" width=\"10\" height=\"40\" />\n</g>\n</g>\n';
var expectedSVG = '<g transform=\"matrix(1 0 0 1 90 130)\" >\n<g style=\"\" >\n\t\t<g transform=\"matrix(1 0 0 1 25 -25)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-15\" y=\"-5\" rx=\"0\" ry=\"0\" width=\"30\" height=\"10\" />\n</g>\n\t\t<g transform=\"matrix(1 0 0 1 -35 10)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-5\" y=\"-20\" rx=\"0\" ry=\"0\" width=\"10\" height=\"40\" />\n</g>\n</g>\n</g>\n';
assert.equal(group.toSVG(), expectedSVG);
});

QUnit.test('toSVG with a clipPath', function(assert) {
var group = makeGroupWith2Objects();
assert.ok(typeof group.toSVG === 'function');
group.clipPath = new fabric.Rect({ width: 100, height: 100 });
var expectedSVG = '<g transform=\"matrix(1 0 0 1 90 130)\" style=\"\" clip-path=\"url(#CLIPPATH_0)\" >\n<clipPath id=\"CLIPPATH_0\" >\n\t<rect transform=\"matrix(1 0 0 1 50.5 50.5)\" x=\"-50\" y=\"-50\" rx=\"0\" ry=\"0\" width=\"100\" height=\"100\" />\n</clipPath>\n\t<g transform=\"matrix(1 0 0 1 25 -25)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-15\" y=\"-5\" rx=\"0\" ry=\"0\" width=\"30\" height=\"10\" />\n</g>\n\t<g transform=\"matrix(1 0 0 1 -35 10)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-5\" y=\"-20\" rx=\"0\" ry=\"0\" width=\"10\" height=\"40\" />\n</g>\n</g>\n';
var expectedSVG = '<g transform=\"matrix(1 0 0 1 90 130)\" clip-path=\"url(#CLIPPATH_0)\" >\n<clipPath id=\"CLIPPATH_0\" >\n\t<rect transform=\"matrix(1 0 0 1 50.5 50.5)\" x=\"-50\" y=\"-50\" rx=\"0\" ry=\"0\" width=\"100\" height=\"100\" />\n</clipPath>\n<g style=\"\" >\n\t\t<g transform=\"matrix(1 0 0 1 25 -25)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-15\" y=\"-5\" rx=\"0\" ry=\"0\" width=\"30\" height=\"10\" />\n</g>\n\t\t<g transform=\"matrix(1 0 0 1 -35 10)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-5\" y=\"-20\" rx=\"0\" ry=\"0\" width=\"10\" height=\"40\" />\n</g>\n</g>\n</g>\n';
assert.equal(group.toSVG(), expectedSVG);
});

QUnit.test('toSVG with a clipPath absolutePositioned', function(assert) {
var group = makeGroupWith2Objects();
assert.ok(typeof group.toSVG === 'function');
group.clipPath = new fabric.Rect({ width: 100, height: 100 });
group.clipPath.absolutePositioned = true;
var expectedSVG = '<g style=\"\" clip-path=\"url(#CLIPPATH_0)\" >\n<g transform=\"matrix(1 0 0 1 90 130)\" >\n<clipPath id=\"CLIPPATH_0\" >\n\t<rect transform=\"matrix(1 0 0 1 50.5 50.5)\" x=\"-50\" y=\"-50\" rx=\"0\" ry=\"0\" width=\"100\" height=\"100\" />\n</clipPath>\n\t<g transform=\"matrix(1 0 0 1 25 -25)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-15\" y=\"-5\" rx=\"0\" ry=\"0\" width=\"30\" height=\"10\" />\n</g>\n\t<g transform=\"matrix(1 0 0 1 -35 10)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-5\" y=\"-20\" rx=\"0\" ry=\"0\" width=\"10\" height=\"40\" />\n</g>\n</g>\n</g>\n';
var expectedSVG = '<g clip-path=\"url(#CLIPPATH_0)\" >\n<g transform=\"matrix(1 0 0 1 90 130)\" >\n<clipPath id=\"CLIPPATH_0\" >\n\t<rect transform=\"matrix(1 0 0 1 50.5 50.5)\" x=\"-50\" y=\"-50\" rx=\"0\" ry=\"0\" width=\"100\" height=\"100\" />\n</clipPath>\n<g style=\"\" >\n\t\t<g transform=\"matrix(1 0 0 1 25 -25)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-15\" y=\"-5\" rx=\"0\" ry=\"0\" width=\"30\" height=\"10\" />\n</g>\n\t\t<g transform=\"matrix(1 0 0 1 -35 10)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-5\" y=\"-20\" rx=\"0\" ry=\"0\" width=\"10\" height=\"40\" />\n</g>\n</g>\n</g>\n</g>\n';
assert.equal(group.toSVG(), expectedSVG);
});

QUnit.test('toSVG with a group as a clipPath', function(assert) {
var group = makeGroupWith2Objects();
assert.ok(typeof group.toSVG === 'function');
group.clipPath = makeGroupWith2Objects();
var expectedSVG = '<g transform=\"matrix(1 0 0 1 90 130)\" style=\"\" clip-path=\"url(#CLIPPATH_0)\" >\n<clipPath id=\"CLIPPATH_0\" >\n\t\t<rect transform=\"matrix(1 0 0 1 115 105)\" x=\"-15\" y=\"-5\" rx=\"0\" ry=\"0\" width=\"30\" height=\"10\" />\n\t\t<rect transform=\"matrix(1 0 0 1 55 140)\" x=\"-5\" y=\"-20\" rx=\"0\" ry=\"0\" width=\"10\" height=\"40\" />\n</clipPath>\n\t<g transform=\"matrix(1 0 0 1 25 -25)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-15\" y=\"-5\" rx=\"0\" ry=\"0\" width=\"30\" height=\"10\" />\n</g>\n\t<g transform=\"matrix(1 0 0 1 -35 10)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-5\" y=\"-20\" rx=\"0\" ry=\"0\" width=\"10\" height=\"40\" />\n</g>\n</g>\n';
var expectedSVG = '<g transform=\"matrix(1 0 0 1 90 130)\" clip-path=\"url(#CLIPPATH_0)\" >\n<clipPath id=\"CLIPPATH_0\" >\n\t\t<rect transform=\"matrix(1 0 0 1 115 105)\" x=\"-15\" y=\"-5\" rx=\"0\" ry=\"0\" width=\"30\" height=\"10\" />\n\t\t<rect transform=\"matrix(1 0 0 1 55 140)\" x=\"-5\" y=\"-20\" rx=\"0\" ry=\"0\" width=\"10\" height=\"40\" />\n</clipPath>\n<g style=\"\" >\n\t\t<g transform=\"matrix(1 0 0 1 25 -25)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-15\" y=\"-5\" rx=\"0\" ry=\"0\" width=\"30\" height=\"10\" />\n</g>\n\t\t<g transform=\"matrix(1 0 0 1 -35 10)\" >\n<rect style=\"stroke: none; stroke-width: 0; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(0,0,0); fill-rule: nonzero; opacity: 1;\" x=\"-5\" y=\"-20\" rx=\"0\" ry=\"0\" width=\"10\" height=\"40\" />\n</g>\n</g>\n</g>\n';
assert.equal(group.toSVG(), expectedSVG);
});

Expand Down
Loading