From af8c98038f762418ea1ae4349164badab92c8df0 Mon Sep 17 00:00:00 2001 From: adroitwhiz Date: Mon, 25 May 2020 20:40:14 -0400 Subject: [PATCH 1/4] Transform gradients in strokes --- src/transform-applier.js | 41 ++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/src/transform-applier.js b/src/transform-applier.js index 8ab1302e..73181e6e 100644 --- a/src/transform-applier.js +++ b/src/transform-applier.js @@ -505,14 +505,16 @@ const _parseUrl = (value, windowRef) => { */ const transformStrokeWidths = function (svgTag, windowRef, bboxForTesting) { const inherited = Matrix.identity(); - const applyTransforms = (element, matrix, strokeWidth, fill) => { + + 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; } // If any child nodes don't take attributes, leave the attributes @@ -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'); @@ -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; + } } // Transform path data @@ -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); + if (stroke) element.setAttribute('stroke', stroke); } else if (_isGraphicsElement(element)) { - // Push stroke width and fill down to leaves + // 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)); From bea1ac66ebb140cf0838b370ba10f2deeb00be71 Mon Sep 17 00:00:00 2001 From: adroitwhiz Date: Wed, 27 May 2020 16:12:02 -0400 Subject: [PATCH 2/4] Use doc instead of windowRef.document --- src/transform-applier.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/transform-applier.js b/src/transform-applier.js index 73181e6e..e706e19a 100644 --- a/src/transform-applier.js +++ b/src/transform-applier.js @@ -561,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); From 06c0e4a343050a7addcd252b2b54565545d7f25d Mon Sep 17 00:00:00 2001 From: adroitwhiz Date: Wed, 27 May 2020 16:28:43 -0400 Subject: [PATCH 3/4] Reuse existing transformed gradients --- src/transform-applier.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/transform-applier.js b/src/transform-applier.js index e706e19a..0dde3446 100644 --- a/src/transform-applier.js +++ b/src/transform-applier.js @@ -372,6 +372,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; From 414c7d7d7d17e35d390d82e565850c7f19771df5 Mon Sep 17 00:00:00 2001 From: adroitwhiz Date: Wed, 27 May 2020 16:58:39 -0400 Subject: [PATCH 4/4] Move element categories into own file+correct them Instead of checking if an element is a graphics element, check if it's "paintable" (fill/stroke applies to it). This includes several text-related elements, and excludes `use` and `image` elements. --- src/element-categories.js | 30 ++++++++++++++++++++++++++++++ src/transform-applier.js | 13 +++---------- 2 files changed, 33 insertions(+), 10 deletions(-) create mode 100644 src/element-categories.js diff --git a/src/element-categories.js b/src/element-categories.js new file mode 100644 index 00000000..f556682a --- /dev/null +++ b/src/element-categories.js @@ -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 +}; diff --git a/src/transform-applier.js b/src/transform-applier.js index 0dde3446..591cb0b5 100644 --- a/src/transform-applier.js +++ b/src/transform-applier.js @@ -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'); /** @@ -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'] ? @@ -514,7 +507,7 @@ const transformStrokeWidths = function (svgTag, windowRef, bboxForTesting) { const inherited = Matrix.identity(); const applyTransforms = (element, matrix, strokeWidth, fill, stroke) => { - if (_isContainerElement(element)) { + if (isContainerElement(element)) { // Push fills and stroke width down to leaves if (element.attributes['stroke-width']) { strokeWidth = element.attributes['stroke-width'].value; @@ -601,7 +594,7 @@ const transformStrokeWidths = function (svgTag, windowRef, bboxForTesting) { element.setAttribute('stroke-width', _quadraticMean(matrixScale.x, matrixScale.y) * strokeWidth); if (fill) element.setAttribute('fill', fill); if (stroke) element.setAttribute('stroke', stroke); - } else if (_isGraphicsElement(element)) { + } else if (isPaintableElement(element)) { // Push stroke width, fill, and stroke down to leaves if (strokeWidth && !element.attributes['stroke-width']) { element.setAttribute('stroke-width', strokeWidth);