diff --git a/lib/path.js b/lib/path.js index 8e6b47221..814ee2e3b 100644 --- a/lib/path.js +++ b/lib/path.js @@ -1,6 +1,6 @@ 'use strict'; -const { removeLeadingZero } = require('./svgo/tools'); +const { removeLeadingZero, toFixed } = require('./svgo/tools'); /** * @typedef {import('./types').PathDataItem} PathDataItem @@ -250,8 +250,7 @@ exports.parsePathData = parsePathData; */ const roundAndStringify = (number, precision) => { if (precision != null) { - const ratio = 10 ** precision; - number = Math.round(number * ratio) / ratio; + number = toFixed(number, precision); } return { diff --git a/lib/svgo/tools.js b/lib/svgo/tools.js index 618ca70aa..2d5269ec8 100644 --- a/lib/svgo/tools.js +++ b/lib/svgo/tools.js @@ -229,3 +229,17 @@ const findReferences = (attribute, value) => { return results.map((body) => decodeURI(body)); }; exports.findReferences = findReferences; + +/** + * Does the same as {@link Number.toFixed} but without casting + * the return value to a string. + * + * @param {number} num + * @param {number} precision + * @returns {number} + */ +const toFixed = (num, precision) => { + const pow = 10 ** precision; + return Math.round(num * pow) / pow; +}; +exports.toFixed = toFixed; diff --git a/plugins/_transforms.js b/plugins/_transforms.js index 5606b6e7d..1ab6d6485 100644 --- a/plugins/_transforms.js +++ b/plugins/_transforms.js @@ -1,175 +1,182 @@ 'use strict'; -const regTransformTypes = /matrix|translate|scale|rotate|skewX|skewY/; -const regTransformSplit = - /\s*(matrix|translate|scale|rotate|skewX|skewY)\s*\(\s*(.+?)\s*\)[\s,]*/; -const regNumericValues = /[-+]?(?:\d*\.\d+|\d+\.?)(?:[eE][-+]?\d+)?/g; +const { toFixed } = require('../lib/svgo/tools'); /** * @typedef {{ name: string, data: number[] }} TransformItem + * @typedef {{ + * convertToShorts: boolean, + * floatPrecision: number, + * transformPrecision: number, + * matrixToTransform: boolean, + * shortTranslate: boolean, + * shortScale: boolean, + * shortRotate: boolean, + * removeUseless: boolean, + * collapseIntoOne: boolean, + * leadingZero: boolean, + * negativeExtraSpace: boolean, + * }} TransformParams */ +const transformTypes = new Set([ + 'matrix', + 'rotate', + 'scale', + 'skewX', + 'skewY', + 'translate', +]); + +const regTransformSplit = + /\s*(matrix|translate|scale|rotate|skewX|skewY)\s*\(\s*(.+?)\s*\)[\s,]*/; +const regNumericValues = /[-+]?(?:\d*\.\d+|\d+\.?)(?:[eE][-+]?\d+)?/g; + /** * Convert transform string to JS representation. * - * @type {(transformString: string) => TransformItem[]} + * @param {string} transformString + * @returns {TransformItem[]} Object representation of transform, or an empty array if it was malformed. */ exports.transform2js = (transformString) => { - // JS representation of the transform data - /** - * @type {TransformItem[]} - */ + /** @type {TransformItem[]} */ const transforms = []; - // current transform context - /** - * @type {?TransformItem} - */ - let current = null; + /** @type {?TransformItem} */ + let currentTransform = null; + // split value into ['', 'translate', '10 50', '', 'scale', '2', '', 'rotate', '-45', ''] for (const item of transformString.split(regTransformSplit)) { - var num; - if (item) { - // if item is a translate function - if (regTransformTypes.test(item)) { - // then collect it and change current context - current = { name: item, data: [] }; - transforms.push(current); - // else if item is data - } else { - // then split it into [10, 50] and collect as context.data - while ((num = regNumericValues.exec(item))) { - num = Number(num); - if (current != null) { - current.data.push(num); - } + if (!item) { + continue; + } + + if (transformTypes.has(item)) { + currentTransform = { name: item, data: [] }; + transforms.push(currentTransform); + } else { + let num; + // then split it into [10, 50] and collect as context.data + while ((num = regNumericValues.exec(item))) { + num = Number(num); + if (currentTransform != null) { + currentTransform.data.push(num); } } } } - // return empty array if broken transform (no data) - return current == null || current.data.length == 0 ? [] : transforms; + + return currentTransform == null || currentTransform.data.length == 0 + ? [] + : transforms; }; /** * Multiply transforms into one. * - * @type {(transforms: TransformItem[]) => TransformItem} + * @param {TransformItem[]} transforms + * @returns {TransformItem} */ exports.transformsMultiply = (transforms) => { - // convert transforms objects to the matrices const matrixData = transforms.map((transform) => { if (transform.name === 'matrix') { return transform.data; } return transformToMatrix(transform); }); - // multiply all matrices into one + const matrixTransform = { name: 'matrix', data: matrixData.length > 0 ? matrixData.reduce(multiplyTransformMatrices) : [], }; + return matrixTransform; }; /** - * math utilities in radians. + * Math utilities in radians. */ const mth = { /** - * @type {(deg: number) => number} + * @param {number} deg + * @returns {number} */ rad: (deg) => { return (deg * Math.PI) / 180; }, /** - * @type {(rad: number) => number} + * @param {number} rad + * @returns {number} */ deg: (rad) => { return (rad * 180) / Math.PI; }, /** - * @type {(deg: number) => number} + * @param {number} deg + * @returns {number} */ cos: (deg) => { return Math.cos(mth.rad(deg)); }, /** - * @type {(val: number, floatPrecision: number) => number} + * @param {number} val + * @param {number} floatPrecision + * @returns {number} */ acos: (val, floatPrecision) => { - return Number(mth.deg(Math.acos(val)).toFixed(floatPrecision)); + return toFixed(mth.deg(Math.acos(val)), floatPrecision); }, /** - * @type {(deg: number) => number} + * @param {number} deg + * @returns {number} */ sin: (deg) => { return Math.sin(mth.rad(deg)); }, /** - * @type {(val: number, floatPrecision: number) => number} + * @param {number} val + * @param {number} floatPrecision + * @returns {number} */ asin: (val, floatPrecision) => { - return Number(mth.deg(Math.asin(val)).toFixed(floatPrecision)); + return toFixed(mth.deg(Math.asin(val)), floatPrecision); }, /** - * @type {(deg: number) => number} + * @param {number} deg + * @returns {number} */ tan: (deg) => { return Math.tan(mth.rad(deg)); }, /** - * @type {(val: number, floatPrecision: number) => number} + * @param {number} val + * @param {number} floatPrecision + * @returns {number} */ atan: (val, floatPrecision) => { - return Number(mth.deg(Math.atan(val)).toFixed(floatPrecision)); + return toFixed(mth.deg(Math.atan(val)), floatPrecision); }, }; /** - * @typedef {{ - * convertToShorts: boolean, - * floatPrecision: number, - * transformPrecision: number, - * matrixToTransform: boolean, - * shortTranslate: boolean, - * shortScale: boolean, - * shortRotate: boolean, - * removeUseless: boolean, - * collapseIntoOne: boolean, - * leadingZero: boolean, - * negativeExtraSpace: boolean, - * }} TransformParams - */ - -/** - * Decompose matrix into simple transforms. See - * https://frederic-wang.fr/decomposition-of-2d-transform-matrices.html + * Decompose matrix into simple transforms. * - * @type {(transform: TransformItem, params: TransformParams) => TransformItem[]} + * @param {TransformItem} transform + * @param {TransformParams} params + * @returns {TransformItem[]} + * @see https://frederic-wang.fr/decomposition-of-2d-transform-matrices.html */ exports.matrixToTransform = (transform, params) => { - let floatPrecision = params.floatPrecision; - let data = transform.data; - let transforms = []; - let sx = Number( - Math.hypot(data[0], data[1]).toFixed(params.transformPrecision), - ); - let sy = Number( - ((data[0] * data[3] - data[1] * data[2]) / sx).toFixed( - params.transformPrecision, - ), - ); - let colsSum = data[0] * data[2] + data[1] * data[3]; - let rowsSum = data[0] * data[1] + data[2] * data[3]; - let scaleBefore = rowsSum != 0 || sx == sy; + const floatPrecision = params.floatPrecision; + const data = transform.data; + const transforms = []; // [..., ..., ..., ..., tx, ty] → translate(tx, ty) if (data[4] || data[5]) { @@ -179,6 +186,15 @@ exports.matrixToTransform = (transform, params) => { }); } + let sx = toFixed(Math.hypot(data[0], data[1]), params.transformPrecision); + let sy = toFixed( + (data[0] * data[3] - data[1] * data[2]) / sx, + params.transformPrecision, + ); + const colsSum = data[0] * data[2] + data[1] * data[3]; + const rowsSum = data[0] * data[1] + data[2] * data[3]; + const scaleBefore = rowsSum !== 0 || sx === sy; + // [sx, 0, tan(a)·sy, sy, 0, 0] → skewX(a)·scale(sx, sy) if (!data[1] && data[2]) { transforms.push({ @@ -197,19 +213,34 @@ exports.matrixToTransform = (transform, params) => { // [sx·cos(a), sx·sin(a), sy·-sin(a), sy·cos(a), x, y] → rotate(a[, cx, cy])·(scale or skewX) or // [sx·cos(a), sy·sin(a), sx·-sin(a), sy·cos(a), x, y] → scale(sx, sy)·rotate(a[, cx, cy]) (if !scaleBefore) - } else if (!colsSum || (sx == 1 && sy == 1) || !scaleBefore) { + } else if (!colsSum || (sx === 1 && sy === 1) || !scaleBefore) { if (!scaleBefore) { - sx = (data[0] < 0 ? -1 : 1) * Math.hypot(data[0], data[2]); - sy = (data[3] < 0 ? -1 : 1) * Math.hypot(data[1], data[3]); + sx = Math.hypot(data[0], data[2]); + sy = Math.hypot(data[1], data[3]); + + if (toFixed(data[0], params.transformPrecision) < 0) { + sx = -sx; + } + + if ( + data[3] < 0 || + (Math.sign(data[1]) === Math.sign(data[2]) && + toFixed(data[3], params.transformPrecision) === 0) + ) { + sy = -sy; + } + transforms.push({ name: 'scale', data: [sx, sy] }); } - var angle = Math.min(Math.max(-1, data[0] / sx), 1), - rotate = [ - mth.acos(angle, floatPrecision) * - ((scaleBefore ? 1 : sy) * data[1] < 0 ? -1 : 1), - ]; + const angle = Math.min(Math.max(-1, data[0] / sx), 1); + const rotate = [ + mth.acos(angle, floatPrecision) * + ((scaleBefore ? 1 : sy) * data[1] < 0 ? -1 : 1), + ]; - if (rotate[0]) transforms.push({ name: 'rotate', data: rotate }); + if (rotate[0]) { + transforms.push({ name: 'rotate', data: rotate }); + } if (rowsSum && colsSum) transforms.push({ @@ -220,15 +251,15 @@ exports.matrixToTransform = (transform, params) => { // rotate(a, cx, cy) can consume translate() within optional arguments cx, cy (rotation point) if (rotate[0] && (data[4] || data[5])) { transforms.shift(); - var cos = data[0] / sx, - sin = data[1] / (scaleBefore ? sx : sy), - x = data[4] * (scaleBefore ? 1 : sy), - y = data[5] * (scaleBefore ? 1 : sx), - denom = - (Math.pow(1 - cos, 2) + Math.pow(sin, 2)) * - (scaleBefore ? 1 : sx * sy); - rotate.push(((1 - cos) * x - sin * y) / denom); - rotate.push(((1 - cos) * y + sin * x) / denom); + const oneOverCos = 1 - data[0] / sx; + const sin = data[1] / (scaleBefore ? sx : sy); + const x = data[4] * (scaleBefore ? 1 : sy); + const y = data[5] * (scaleBefore ? 1 : sx); + const denom = (oneOverCos ** 2 + sin ** 2) * (scaleBefore ? 1 : sx * sy); + rotate.push( + (oneOverCos * x - sin * y) / denom, + (oneOverCos * y + sin * x) / denom, + ); } // Too many transformations, return original matrix if it isn't just a scale/translate @@ -236,11 +267,12 @@ exports.matrixToTransform = (transform, params) => { return [transform]; } - if ((scaleBefore && (sx != 1 || sy != 1)) || !transforms.length) + if ((scaleBefore && (sx != 1 || sy != 1)) || !transforms.length) { transforms.push({ name: 'scale', data: sx == sy ? [sx] : [sx, sy], }); + } return transforms; }; diff --git a/plugins/convertPathData.js b/plugins/convertPathData.js index e8d8350e1..54c006368 100644 --- a/plugins/convertPathData.js +++ b/plugins/convertPathData.js @@ -9,7 +9,7 @@ const { visit } = require('../lib/xast.js'); const { pathElems } = require('./_collections.js'); const { path2js, js2path } = require('./_path.js'); const { applyTransforms } = require('./applyTransforms.js'); -const { cleanupOutData } = require('../lib/svgo/tools'); +const { cleanupOutData, toFixed } = require('../lib/svgo/tools'); exports.name = 'convertPathData'; exports.description = @@ -482,7 +482,7 @@ function filters( // check if next curves are fitting the arc for ( var j = index; - (next = path[++j]) && 'cs'.includes(next.command); + (next = path[++j]) && (next.command === 'c' || next.command === 's'); ) { var nextData = next.args; @@ -1004,16 +1004,6 @@ function getIntersection(coords) { } } -/** - * Does the same as `Number.prototype.toFixed` but without casting - * the return value to a string. - * @type {(num: number, precision: number) => number} - */ -function toFixed(num, precision) { - const pow = 10 ** precision; - return Math.round(num * pow) / pow; -} - /** * Decrease accuracy of floating-point numbers * in path data keeping a specified number of decimals. diff --git a/plugins/convertTransform.js b/plugins/convertTransform.js index 730227ea3..6f3990789 100644 --- a/plugins/convertTransform.js +++ b/plugins/convertTransform.js @@ -1,10 +1,12 @@ 'use strict'; /** + * @typedef {import('../lib/types').XastChild} XastChild * @typedef {import('../lib/types').XastElement} XastElement + * @typedef {import('../lib/types').XastParent} XastParent */ -const { cleanupOutData } = require('../lib/svgo/tools.js'); +const { cleanupOutData, toFixed } = require('../lib/svgo/tools.js'); const { transform2js, transformsMultiply, @@ -59,15 +61,14 @@ exports.fn = (_root, params) => { return { element: { enter: (node) => { - // transform if (node.attributes.transform != null) { convertTransform(node, 'transform', newParams); } - // gradientTransform + if (node.attributes.gradientTransform != null) { convertTransform(node, 'gradientTransform', newParams); } - // patternTransform + if (node.attributes.patternTransform != null) { convertTransform(node, 'patternTransform', newParams); } @@ -98,9 +99,9 @@ exports.fn = (_root, params) => { */ /** - * Main function. - * - * @type {(item: XastElement, attrName: string, params: TransformParams) => void} + * @param {XastElement} item + * @param {string} attrName + * @param {TransformParams} params */ const convertTransform = (item, attrName, params) => { let data = transform2js(item.attributes[attrName]); @@ -145,7 +146,7 @@ const definePrecision = (data, { ...newParams }) => { matrixData.push(...item.data.slice(0, 4)); } } - let significantDigits = newParams.transformPrecision; + let numberOfDigits = newParams.transformPrecision; // Limit transform precision with matrix one. Calculating with larger precision doesn't add any value. if (matrixData.length) { newParams.transformPrecision = Math.min( @@ -153,7 +154,7 @@ const definePrecision = (data, { ...newParams }) => { Math.max.apply(Math, matrixData.map(floatDigits)) || newParams.transformPrecision, ); - significantDigits = Math.max.apply( + numberOfDigits = Math.max.apply( Math, matrixData.map( (n) => n.toString().replace(/\D+/g, '').length, // Number of digits in a number. 123.45 → 5 @@ -164,7 +165,7 @@ const definePrecision = (data, { ...newParams }) => { if (newParams.degPrecision == null) { newParams.degPrecision = Math.max( 0, - Math.min(newParams.floatPrecision, significantDigits - 2), + Math.min(newParams.floatPrecision, numberOfDigits - 2), ); } return newParams; @@ -219,11 +220,13 @@ const floatDigits = (n) => { /** * Convert transforms to the shorthand alternatives. * - * @type {(transforms: TransformItem[], params: TransformParams) => TransformItem[]} + * @param {TransformItem[]} transforms + * @param {TransformParams} params + * @returns {TransformItem[]} */ const convertToShorts = (transforms, params) => { for (var i = 0; i < transforms.length; i++) { - var transform = transforms[i]; + let transform = transforms[i]; // convert matrix to the short aliases if (params.matrixToTransform && transform.name === 'matrix') { @@ -267,8 +270,7 @@ const convertToShorts = (transforms, params) => { // translate(cx cy) rotate(a) translate(-cx -cy) → rotate(a cx cy) if ( params.shortRotate && - transforms[i - 2] && - transforms[i - 2].name === 'translate' && + transforms[i - 2]?.name === 'translate' && transforms[i - 1].name === 'rotate' && transforms[i].name === 'translate' && transforms[i - 2].data[0] === -transforms[i].data[0] && @@ -332,7 +334,9 @@ const removeUseless = (transforms) => { /** * Convert transforms JS representation to string. * - * @type {(transformJS: TransformItem[], params: TransformParams) => string} + * @param {TransformItem[]} transformJS + * @param {TransformParams} params + * @returns {string} */ const js2transform = (transformJS, params) => { const transformString = transformJS @@ -390,7 +394,9 @@ const round = (data) => { * in transforms keeping a specified number of decimals. * Smart rounds values like 2.349 to 2.35. * - * @type {(precision: number, data: number[]) => number[]} + * @param {number} precision + * @param {number[]} data + * @returns {number[]} */ const smartRound = (precision, data) => { for ( @@ -399,7 +405,7 @@ const smartRound = (precision, data) => { i--; ) { - if (Number(data[i].toFixed(precision)) !== data[i]) { + if (toFixed(data[i], precision) !== data[i]) { var rounded = +data[i].toFixed(precision - 1); data[i] = +Math.abs(rounded - data[i]).toFixed(precision + 1) >= tolerance @@ -407,5 +413,6 @@ const smartRound = (precision, data) => { : rounded; } } + return data; }; diff --git a/test/plugins/_transforms.test.js b/test/plugins/_transforms.test.js new file mode 100644 index 000000000..6f8d72e3b --- /dev/null +++ b/test/plugins/_transforms.test.js @@ -0,0 +1,64 @@ +'use strict'; + +const { matrixToTransform } = require('../../plugins/_transforms'); + +/** + * @typedef {import('../../plugins/_transforms').TransformParams} TransformParams + */ + +/** @type {TransformParams} */ +const params = { + floatPrecision: 3, + transformPrecision: 5, + matrixToTransform: true, + shortTranslate: true, + shortScale: true, + shortRotate: true, + removeUseless: true, + collapseIntoOne: true, + leadingZero: true, + negativeExtraSpace: false, + convertToShorts: true, +}; + +/** + * Some tests live here instead of in test SVGs because the output + * is longer, so SVGO doesn't actually use it. + */ +describe('should correctly simplify transforms', () => { + it('matrix(0, -1, 99, 0, 0, 0)', () => { + const matrix = { + name: 'matrix', + data: [0, -1, 99, 0, 0, 0], + }; + + expect(matrixToTransform(matrix, params)).toStrictEqual([ + { + name: 'scale', + data: [99, 1], + }, + { + name: 'rotate', + data: [-90], + }, + ]); + }); + + it('matrix(0, 1, 1, 0, 0, 0)', () => { + const matrix = { + name: 'matrix', + data: [0, 1, 1, 0, 0, 0], + }; + + expect(matrixToTransform(matrix, params)).toStrictEqual([ + { + name: 'scale', + data: [1, -1], + }, + { + name: 'rotate', + data: [-90], + }, + ]); + }); +}); diff --git a/test/plugins/convertTransform.05.svg b/test/plugins/convertTransform.05.svg new file mode 100644 index 000000000..c3a82a83e --- /dev/null +++ b/test/plugins/convertTransform.05.svg @@ -0,0 +1,13 @@ +Correctly optimize transform with same sign non-zero shears and. + +=== + + + + + +@@@ + + + +