Skip to content

Transform gradients in strokes #143

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

Merged
adroitwhiz merged 4 commits into from
Jun 4, 2020
Merged

Transform gradients in strokes #143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
af8c980
Transform gradients in strokes
adroitwhiz May 26, 2020
bea1ac6
Use doc instead of windowRef.document
adroitwhiz May 27, 2020
06c0e4a
Reuse existing transformed gradients
adroitwhiz May 27, 2020
414c7d7
Move element categories into own file+correct them
adroitwhiz May 27, 2020
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
30 changes: 30 additions & 0 deletions src/element-categories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Elements that can have paint attributes (e.g. fill and stroke)
// Per the SVG spec, things like fill and stroke apply to shapes and text content elements
// https://www.w3.org/TR/SVG11/painting.html#FillProperty
// https://www.w3.org/TR/SVG11/painting.html#StrokeProperty
const PAINTABLE_ELEMENTS = new Set([
// Shape elements (https://www.w3.org/TR/SVG11/intro.html#TermShape)
'path', 'rect', 'circle', 'ellipse', 'line', 'polyline', 'polygon',
// Text content elements (https://www.w3.org/TR/SVG11/intro.html#TermTextContentElement)
// The actual tag names are `altGlyph` and `textPath`, but we're lowercasing the tag name in isContainerElement,
// so they should be lowercased here too.
'altglyph', 'textpath', 'text', 'tref', 'tspan'
]);

// "An element which can have graphics elements and other container elements as child elements."
// https://www.w3.org/TR/SVG11/intro.html#TermContainerElement
const CONTAINER_ELEMENTS = new Set([
'a', 'defs', 'g', 'marker', 'glyph', 'missing-glyph', 'pattern', 'svg', 'switch', 'symbol'
]);

const isPaintableElement = function (element) {
return element.tagName && PAINTABLE_ELEMENTS.has(element.tagName.toLowerCase());
};
const isContainerElement = function (element) {
return element.tagName && CONTAINER_ELEMENTS.has(element.tagName.toLowerCase());
};

module.exports = {
isPaintableElement,
isContainerElement
};
65 changes: 41 additions & 24 deletions src/transform-applier.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const Matrix = require('transformation-matrix');
const SvgElement = require('./svg-element');
const {isPaintableElement, isContainerElement} = require('./element-categories');
const log = require('./util/log');

/**
Expand Down Expand Up @@ -288,14 +289,6 @@ const _transformPath = function (pathString, transform) {
return result;
};

const GRAPHICS_ELEMENTS = ['circle', 'ellipse', 'image', 'line', 'path', 'polygon', 'polyline', 'rect', 'text', 'use'];
const CONTAINER_ELEMENTS = ['a', 'defs', 'g', 'marker', 'glyph', 'missing-glyph', 'pattern', 'svg', 'switch', 'symbol'];
const _isContainerElement = function (element) {
return element.tagName && CONTAINER_ELEMENTS.includes(element.tagName.toLowerCase());
};
const _isGraphicsElement = function (element) {
return element.tagName && GRAPHICS_ELEMENTS.includes(element.tagName.toLowerCase());
};
const _isPathWithTransformAndStroke = function (element, strokeWidth) {
if (!element.attributes) return false;
strokeWidth = element.attributes['stroke-width'] ?
Expand Down Expand Up @@ -372,6 +365,13 @@ const _createGradient = function (gradientId, svgTag, bbox, matrix) {
const newGradientId = `${gradientId}-${matrixString}`;
newGradient.setAttribute('id', newGradientId);

// This gradient already exists and was transformed before. Just reuse the already-transformed one.
if (svgTag.getElementById(newGradientId)) {
// This is the same code as in the end of the function, but I don't feel like wrapping the next 80 lines
// in an `if (!svgTag.getElementById(newGradientId))` block
return `url(#${newGradientId})`;
}

const scaleToBounds = getValue(newGradient, 'gradientUnits', true) !==
'userSpaceOnUse';
let origin;
Expand Down Expand Up @@ -505,14 +505,16 @@ const _parseUrl = (value, windowRef) => {
*/
const transformStrokeWidths = function (svgTag, windowRef, bboxForTesting) {
const inherited = Matrix.identity();
const applyTransforms = (element, matrix, strokeWidth, fill) => {
if (_isContainerElement(element)) {

const applyTransforms = (element, matrix, strokeWidth, fill, stroke) => {
if (isContainerElement(element)) {
// Push fills and stroke width down to leaves
if (element.attributes['stroke-width']) {
strokeWidth = element.attributes['stroke-width'].value;
}
if (element.attributes && element.attributes.fill) {
fill = element.attributes.fill.value;
if (element.attributes) {
if (element.attributes.fill) fill = element.attributes.fill.value;
if (element.attributes.stroke) stroke = element.attributes.stroke.value;
Copy link
Contributor Author

@adroitwhiz adroitwhiz May 26, 2020

Choose a reason for hiding this comment

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

Why are we checking if element.attributes exists here but not checking it in the previous if-block (if (element.attributes['stroke-width']))?

Copy link
Contributor

@fsih fsih May 27, 2020

Choose a reason for hiding this comment

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

It looks like element.attributes should exist if element.tagName exists, which is what we check for in _isContainerElement, except on Internet Explorer. So the if above would break on IE (but we don't support IE anyway)

}

// If any child nodes don't take attributes, leave the attributes
Expand All @@ -522,30 +524,34 @@ const transformStrokeWidths = function (svgTag, windowRef, bboxForTesting) {
element.childNodes[i],
Matrix.compose(matrix, _parseTransform(element)),
strokeWidth,
fill
fill,
stroke
);
}
element.removeAttribute('transform');
element.removeAttribute('stroke-width');
element.removeAttribute('fill');
element.removeAttribute('stroke');
} else if (_isPathWithTransformAndStroke(element, strokeWidth)) {
if (element.attributes['stroke-width']) {
strokeWidth = element.attributes['stroke-width'].value;
}
if (element.attributes.fill) {
fill = element.attributes.fill.value;
}
if (element.attributes.fill) fill = element.attributes.fill.value;
if (element.attributes.stroke) stroke = element.attributes.stroke.value;
matrix = Matrix.compose(matrix, _parseTransform(element));
if (Matrix.toString(matrix) === Matrix.toString(Matrix.identity())) {
element.removeAttribute('transform');
element.setAttribute('stroke-width', strokeWidth);
if (fill) element.setAttribute('fill', fill);
if (stroke) element.setAttribute('stroke', stroke);
return;
}

// Transform gradient
const gradientId = _parseUrl(fill, windowRef);
if (gradientId) {
const fillGradientId = _parseUrl(fill, windowRef);
const strokeGradientId = _parseUrl(stroke, windowRef);

if (fillGradientId || strokeGradientId) {
const doc = windowRef.document;
// Need path bounds to transform gradient
const svgSpot = doc.createElement('span');
Expand All @@ -555,8 +561,8 @@ const transformStrokeWidths = function (svgTag, windowRef, bboxForTesting) {
} else {
try {
doc.body.appendChild(svgSpot);
const svg = SvgElement.set(windowRef.document.createElementNS(SvgElement.svg, 'svg'));
const path = SvgElement.set(windowRef.document.createElementNS(SvgElement.svg, 'path'));
const svg = SvgElement.set(doc.createElementNS(SvgElement.svg, 'svg'));
const path = SvgElement.set(doc.createElementNS(SvgElement.svg, 'path'));
path.setAttribute('d', element.attributes.d.value);
svg.appendChild(path);
svgSpot.appendChild(svg);
Expand All @@ -568,8 +574,15 @@ const transformStrokeWidths = function (svgTag, windowRef, bboxForTesting) {
}
}

const newRef = _createGradient(gradientId, svgTag, bbox, matrix);
if (newRef) fill = newRef;
if (fillGradientId) {
const newFillRef = _createGradient(fillGradientId, svgTag, bbox, matrix);
if (newFillRef) fill = newFillRef;
}

if (strokeGradientId) {
const newStrokeRef = _createGradient(strokeGradientId, svgTag, bbox, matrix);
if (newStrokeRef) stroke = newStrokeRef;
}
}
Copy link
Contributor

@fsih fsih May 27, 2020

Choose a reason for hiding this comment

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

It seems like fillGradientId and strokeGradientId might be the same, and if they're the same, we should try not to make a duplicate definition (although I'm not sure if it hurts anything if we do have duplicates)


// Transform path data
Expand All @@ -580,14 +593,18 @@ const transformStrokeWidths = function (svgTag, windowRef, bboxForTesting) {
const matrixScale = _getScaleFactor(matrix);
element.setAttribute('stroke-width', _quadraticMean(matrixScale.x, matrixScale.y) * strokeWidth);
if (fill) element.setAttribute('fill', fill);
} else if (_isGraphicsElement(element)) {
// Push stroke width and fill down to leaves
if (stroke) element.setAttribute('stroke', stroke);
} else if (isPaintableElement(element)) {
// Push stroke width, fill, and stroke down to leaves
if (strokeWidth && !element.attributes['stroke-width']) {
element.setAttribute('stroke-width', strokeWidth);
}
if (fill && !element.attributes.fill) {
element.setAttribute('fill', fill);
}
if (stroke && !element.attributes.stroke) {
element.setAttribute('stroke', stroke);
}

// Push transform down to leaves
matrix = Matrix.compose(matrix, _parseTransform(element));
Expand Down