From 01ad9d7fdd108ad5a9d9efcae8bdc97644e7a9b6 Mon Sep 17 00:00:00 2001 From: Flint O'Brien Date: Fri, 22 Apr 2016 12:24:52 -0400 Subject: [PATCH 1/7] Refactor canvas.convertToPath() internals to svgutils in preparation for getBBox performance improvements. Two new functions in svgutils: convertToPath() and getBBoxOfElementAsPath(). Updated test/svgutils_test.html. --- editor/svgcanvas.js | 164 +-------------------------- editor/svgutils.js | 239 ++++++++++++++++++++++++++++++++++++++++ test/svgutils_test.html | 238 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 480 insertions(+), 161 deletions(-) diff --git a/editor/svgcanvas.js b/editor/svgcanvas.js index 67e9344e..a7c61afc 100644 --- a/editor/svgcanvas.js +++ b/editor/svgcanvas.js @@ -6576,17 +6576,15 @@ this.setSegType = function(new_type) { this.convertToPath = function(elem, getBBox) { if (elem == null) { var elems = selectedElements; - $.each(selectedElements, function(i, elem) { + $.each(elems, function(i, elem) { if (elem) {canvas.convertToPath(elem);} }); return; } - - if (!getBBox) { - var batchCmd = new svgedit.history.BatchCommand('Convert element to Path'); - } - - var attrs = getBBox?{}:{ + if( getBBox) { + return svgedit.utilities.getBBoxOfElementAsPath( elem, addSvgElementFromJson, pathActions) + } else { + var attrs = { 'fill': cur_shape.fill, 'fill-opacity': cur_shape.fill_opacity, 'stroke': cur_shape.stroke, @@ -6598,157 +6596,7 @@ this.convertToPath = function(elem, getBBox) { 'opacity': cur_shape.opacity, 'visibility':'hidden' }; - - // any attribute on the element not covered by the above - // TODO: make this list global so that we can properly maintain it - // TODO: what about @transform, @clip-rule, @fill-rule, etc? - $.each(['marker-start', 'marker-end', 'marker-mid', 'filter', 'clip-path'], function() { - if (elem.getAttribute(this)) { - attrs[this] = elem.getAttribute(this); - } - }); - - var path = addSvgElementFromJson({ - 'element': 'path', - 'attr': attrs - }); - - var eltrans = elem.getAttribute('transform'); - if (eltrans) { - path.setAttribute('transform', eltrans); - } - - var id = elem.id; - var parent = elem.parentNode; - if (elem.nextSibling) { - parent.insertBefore(path, elem); - } else { - parent.appendChild(path); - } - - var d = ''; - - var joinSegs = function(segs) { - $.each(segs, function(j, seg) { - var i; - var l = seg[0], pts = seg[1]; - d += l; - for (i = 0; i < pts.length; i+=2) { - d += (pts[i] +','+pts[i+1]) + ' '; - } - }); - }; - - // Possibly the cubed root of 6, but 1.81 works best - var num = 1.81; - var a, rx; - switch (elem.tagName) { - case 'ellipse': - case 'circle': - a = $(elem).attr(['rx', 'ry', 'cx', 'cy']); - var cx = a.cx, cy = a.cy; - rx = a.rx; - ry = a.ry; - if (elem.tagName == 'circle') { - rx = ry = $(elem).attr('r'); - } - - joinSegs([ - ['M',[(cx-rx),(cy)]], - ['C',[(cx-rx),(cy-ry/num), (cx-rx/num),(cy-ry), (cx),(cy-ry)]], - ['C',[(cx+rx/num),(cy-ry), (cx+rx),(cy-ry/num), (cx+rx),(cy)]], - ['C',[(cx+rx),(cy+ry/num), (cx+rx/num),(cy+ry), (cx),(cy+ry)]], - ['C',[(cx-rx/num),(cy+ry), (cx-rx),(cy+ry/num), (cx-rx),(cy)]], - ['Z',[]] - ]); - break; - case 'path': - d = elem.getAttribute('d'); - break; - case 'line': - a = $(elem).attr(['x1', 'y1', 'x2', 'y2']); - d = 'M'+a.x1+','+a.y1+'L'+a.x2+','+a.y2; - break; - case 'polyline': - case 'polygon': - d = 'M' + elem.getAttribute('points'); - break; - case 'rect': - var r = $(elem).attr(['rx', 'ry']); - rx = r.rx; - ry = r.ry; - var b = elem.getBBox(); - var x = b.x, y = b.y, w = b.width, h = b.height; - num = 4 - num; // Why? Because! - - if (!rx && !ry) { - // Regular rect - joinSegs([ - ['M',[x, y]], - ['L',[x+w, y]], - ['L',[x+w, y+h]], - ['L',[x, y+h]], - ['L',[x, y]], - ['Z',[]] - ]); - } else { - joinSegs([ - ['M',[x, y+ry]], - ['C',[x, y+ry/num, x+rx/num, y, x+rx, y]], - ['L',[x+w-rx, y]], - ['C',[x+w-rx/num, y, x+w, y+ry/num, x+w, y+ry]], - ['L',[x+w, y+h-ry]], - ['C',[x+w, y+h-ry/num, x+w-rx/num, y+h, x+w-rx, y+h]], - ['L',[x+rx, y+h]], - ['C',[x+rx/num, y+h, x, y+h-ry/num, x, y+h-ry]], - ['L',[x, y+ry]], - ['Z',[]] - ]); - } - break; - default: - path.parentNode.removeChild(path); - break; - } - - if (d) { - path.setAttribute('d', d); - } - - if (!getBBox) { - // Replace the current element with the converted one - - // Reorient if it has a matrix - if (eltrans) { - var tlist = svgedit.transformlist.getTransformList(path); - if (svgedit.math.hasMatrixTransform(tlist)) { - pathActions.resetOrientation(path); - } - } - - var nextSibling = elem.nextSibling; - batchCmd.addSubCommand(new svgedit.history.RemoveElementCommand(elem, nextSibling, parent)); - batchCmd.addSubCommand(new svgedit.history.InsertElementCommand(path)); - - clearSelection(); - elem.parentNode.removeChild(elem); - path.setAttribute('id', id); - path.removeAttribute('visibility'); - addToSelection([path], true); - - addCommandToHistory(batchCmd); - - } else { - // Get the correct BBox of the new path, then discard it - pathActions.resetOrientation(path); - var bb = false; - try { - bb = path.getBBox(); - } catch(e) { - // Firefox fails - } - path.parentNode.removeChild(path); - return bb; + return svgedit.utilities.convertToPath( elem, attrs, addSvgElementFromJson, pathActions, clearSelection, addToSelection, svgedit.history, addCommandToHistory) } }; diff --git a/editor/svgutils.js b/editor/svgutils.js index d3d9b011..adbc2fba 100644 --- a/editor/svgutils.js +++ b/editor/svgutils.js @@ -529,6 +529,245 @@ svgedit.utilities.getBBox = function(elem) { return ret; }; +svgedit.utilities.getPathDFromSegments = function( pathSegments) { + var d = ''; + + $.each(pathSegments, function(j, seg) { + var i; + var l = seg[0], pts = seg[1]; + d += l; + for (i = 0; i < pts.length; i+=2) { + d += (pts[i] +','+pts[i+1]) + ' '; + } + }); + + return d +} + +svgedit.utilities.getPathDFromElement = function( elem) { + + // Possibly the cubed root of 6, but 1.81 works best + var num = 1.81; + var d, a, rx, ry; + switch (elem.tagName) { + case 'ellipse': + case 'circle': + a = $(elem).attr(['rx', 'ry', 'cx', 'cy']); + var cx = a.cx, cy = a.cy; + rx = a.rx; + ry = a.ry; + if (elem.tagName == 'circle') { + rx = ry = $(elem).attr('r'); + } + + d = svgedit.utilities.getPathDFromSegments([ + ['M',[(cx-rx),(cy)]], + ['C',[(cx-rx),(cy-ry/num), (cx-rx/num),(cy-ry), (cx),(cy-ry)]], + ['C',[(cx+rx/num),(cy-ry), (cx+rx),(cy-ry/num), (cx+rx),(cy)]], + ['C',[(cx+rx),(cy+ry/num), (cx+rx/num),(cy+ry), (cx),(cy+ry)]], + ['C',[(cx-rx/num),(cy+ry), (cx-rx),(cy+ry/num), (cx-rx),(cy)]], + ['Z',[]] + ]); + break; + case 'path': + d = elem.getAttribute('d'); + break; + case 'line': + a = $(elem).attr(['x1', 'y1', 'x2', 'y2']); + d = 'M'+a.x1+','+a.y1+'L'+a.x2+','+a.y2; + break; + case 'polyline': + d = 'M' + elem.getAttribute('points'); + break; + case 'polygon': + d = 'M' + elem.getAttribute('points') + ' Z'; + break; + case 'rect': + var r = $(elem).attr(['rx', 'ry']); + rx = r.rx; + ry = r.ry; + var b = elem.getBBox(); + var x = b.x, y = b.y, w = b.width, h = b.height; + num = 4 - num; // Why? Because! + + if (!rx && !ry) { + // Regular rect + d = svgedit.utilities.getPathDFromSegments([ + ['M',[x, y]], + ['L',[x+w, y]], + ['L',[x+w, y+h]], + ['L',[x, y+h]], + ['L',[x, y]], + ['Z',[]] + ]); + } else { + d = svgedit.utilities.getPathDFromSegments([ + ['M',[x, y+ry]], + ['C',[x, y+ry/num, x+rx/num, y, x+rx, y]], + ['L',[x+w-rx, y]], + ['C',[x+w-rx/num, y, x+w, y+ry/num, x+w, y+ry]], + ['L',[x+w, y+h-ry]], + ['C',[x+w, y+h-ry/num, x+w-rx/num, y+h, x+w-rx, y+h]], + ['L',[x+rx, y+h]], + ['C',[x+rx/num, y+h, x, y+h-ry/num, x, y+h-ry]], + ['L',[x, y+ry]], + ['Z',[]] + ]); + } + break; + default: + break; + } + + return d; + +}; + +svgedit.utilities.addAttributesForConvertToPath = function( elem, attrs) { + // TODO: make this list global so that we can properly maintain it + // TODO: what about @transform, @clip-rule, @fill-rule, etc? + $.each(['marker-start', 'marker-end', 'marker-mid', 'filter', 'clip-path'], function() { + if (elem.getAttribute(this)) { + attrs[this] = elem.getAttribute(this); + } + }); +}; + +// Function: getBBoxOfElementAsPath +// Get the BBox of an element-as-path +// +// Parameters: +// elem - The DOM element to be probed +// addSvgElementFromJson - Function to add the path element to the current layer. See canvas.addSvgElementFromJso +// pathActions - If a transform exists, pathActions.resetOrientation() is used. See: canvas.pathActions. +// +// Returns: +// The resulting path's bounding box object. +svgedit.utilities.getBBoxOfElementAsPath = function(elem, addSvgElementFromJson, pathActions) { + + var attrs = {} + + // any attribute on the element not covered by the above + svgedit.utilities.addAttributesForConvertToPath( elem, attrs) + + var path = addSvgElementFromJson({ + 'element': 'path', + 'attr': attrs + }); + + var eltrans = elem.getAttribute('transform'); + if (eltrans) { + path.setAttribute('transform', eltrans); + } + + var id = elem.id; + var parent = elem.parentNode; + if (elem.nextSibling) { + parent.insertBefore(path, elem); + } else { + parent.appendChild(path); + } + + var d = svgedit.utilities.getPathDFromElement( elem); + if( d) + path.setAttribute('d', d); + else + path.parentNode.removeChild(path); + + // Get the correct BBox of the new path, then discard it + pathActions.resetOrientation(path); + var bb = false; + try { + bb = path.getBBox(); + } catch(e) { + // Firefox fails + } + path.parentNode.removeChild(path); + return bb; +} + +// Function: getBBoxOfElementAsPath +// Convert selected element to a path. +// +// Parameters: +// elem - The DOM element to be converted +// attrs - Apply attributes to new path. see canvas.convertToPath +// addSvgElementFromJson - Function to add the path element to the current layer. See canvas.addSvgElementFromJso +// pathActions - If a transform exists, pathActions.resetOrientation() is used. See: canvas.pathActions. +// clearSelection - see canvas.clearSelection +// addToSelection - see canvas.addToSelection +// history - see svgedit.history +// addCommandToHistory - see canvas.addCommandToHistory +// +// Returns: +// The resulting path's bounding box object. +svgedit.utilities.convertToPath = function(elem, attrs, addSvgElementFromJson, pathActions, clearSelection, addToSelection, history, addCommandToHistory) { + + var batchCmd = new history.BatchCommand('Convert element to Path'); + + // any attribute on the element not covered by the above + // TODO: make this list global so that we can properly maintain it + // TODO: what about @transform, @clip-rule, @fill-rule, etc? + $.each(['marker-start', 'marker-end', 'marker-mid', 'filter', 'clip-path'], function() { + if (elem.getAttribute(this)) { + attrs[this] = elem.getAttribute(this); + } + }); + + var path = addSvgElementFromJson({ + 'element': 'path', + 'attr': attrs + }); + + var eltrans = elem.getAttribute('transform'); + if (eltrans) { + path.setAttribute('transform', eltrans); + } + + var id = elem.id; + var parent = elem.parentNode; + if (elem.nextSibling) { + parent.insertBefore(path, elem); + } else { + parent.appendChild(path); + } + + var d = svgedit.utilities.getPathDFromElement( elem); + if( d) { + path.setAttribute('d', d); + + // Replace the current element with the converted one + + // Reorient if it has a matrix + if (eltrans) { + var tlist = svgedit.transformlist.getTransformList(path); + if (svgedit.math.hasMatrixTransform(tlist)) { + pathActions.resetOrientation(path); + } + } + + var nextSibling = elem.nextSibling; + batchCmd.addSubCommand(new history.RemoveElementCommand(elem, nextSibling, parent)); + batchCmd.addSubCommand(new history.InsertElementCommand(path)); + + clearSelection(); + elem.parentNode.removeChild(elem); + path.setAttribute('id', id); + path.removeAttribute('visibility'); + addToSelection([path], true); + + addCommandToHistory(batchCmd); + + return path; + } else { + // the elem.tagName was not recognized, so no "d" attribute. Remove it, so we've haven't changed anything. + path.parentNode.removeChild(path); + return null; + } + +}; + + // Function: svgedit.utilities.getRotationAngle // Get the rotation angle of the given/selected DOM element // diff --git a/test/svgutils_test.html b/test/svgutils_test.html index d81e8aa9..c5cb6d54 100644 --- a/test/svgutils_test.html +++ b/test/svgutils_test.html @@ -10,6 +10,7 @@ + @@ -131,5 +362,6 @@

    +
    From 86c3818886d75c757eb19609f71269a6a4f16e1c Mon Sep 17 00:00:00 2001 From: Flint O'Brien Date: Fri, 22 Apr 2016 13:36:10 -0400 Subject: [PATCH 2/7] Updated comment svgutils convertToPath. --- editor/svgutils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor/svgutils.js b/editor/svgutils.js index adbc2fba..a29a0d58 100644 --- a/editor/svgutils.js +++ b/editor/svgutils.js @@ -686,7 +686,7 @@ svgedit.utilities.getBBoxOfElementAsPath = function(elem, addSvgElementFromJson, return bb; } -// Function: getBBoxOfElementAsPath +// Function: convertToPath // Convert selected element to a path. // // Parameters: From 0c9ff4d1acea5b6efaaeff127de74036a98268be Mon Sep 17 00:00:00 2001 From: Flint O'Brien Date: Fri, 22 Apr 2016 13:39:20 -0400 Subject: [PATCH 3/7] Updated comment svgutils convertToPath... again. --- editor/svgutils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor/svgutils.js b/editor/svgutils.js index a29a0d58..718609f5 100644 --- a/editor/svgutils.js +++ b/editor/svgutils.js @@ -700,7 +700,7 @@ svgedit.utilities.getBBoxOfElementAsPath = function(elem, addSvgElementFromJson, // addCommandToHistory - see canvas.addCommandToHistory // // Returns: -// The resulting path's bounding box object. +// The converted path element or null if the DOM element was not recognized. svgedit.utilities.convertToPath = function(elem, attrs, addSvgElementFromJson, pathActions, clearSelection, addToSelection, history, addCommandToHistory) { var batchCmd = new history.BatchCommand('Convert element to Path'); From 12a393505d482107d55e7cf98203f17cafaa601d Mon Sep 17 00:00:00 2001 From: Flint O'Brien Date: Sun, 24 Apr 2016 16:43:20 -0400 Subject: [PATCH 4/7] Refactoring and performance improvements for getStrokedBBox. canvas.getStrokedBBox internals refactored to svgutils. getStrokedBBox/getCheckedBBox renamed to svgedit.utilities.getBBoxWithTransform Removed duplicate calls to native getBBox. Refactored slow transformed BBox from temporary DOM append/remove to matrix calculations. Lots of tests. Added qunit/qunit-assert-close.js. --- editor/svgcanvas.js | 135 +------- editor/svgutils.js | 165 ++++++++- test/all_tests.html | 1 + test/qunit/qunit-assert-close.js | 106 ++++++ test/svgutils_bbox_test.html | 511 ++++++++++++++++++++++++++++ test/svgutils_performance_test.html | 247 ++++++++++++++ 6 files changed, 1022 insertions(+), 143 deletions(-) create mode 100644 test/qunit/qunit-assert-close.js create mode 100644 test/svgutils_bbox_test.html create mode 100644 test/svgutils_performance_test.html diff --git a/editor/svgcanvas.js b/editor/svgcanvas.js index a7c61afc..1511bc30 100644 --- a/editor/svgcanvas.js +++ b/editor/svgcanvas.js @@ -601,140 +601,9 @@ var getIntersectionList = this.getIntersectionList = function(rect) { // // Returns: // A single bounding box object -getStrokedBBox = this.getStrokedBBox = function(elems) { +var getStrokedBBox = this.getStrokedBBox = function(elems) { if (!elems) {elems = getVisibleElements();} - if (!elems.length) {return false;} - // Make sure the expected BBox is returned if the element is a group - var getCheckedBBox = function(elem) { - // TODO: Fix issue with rotated groups. Currently they work - // fine in FF, but not in other browsers (same problem mentioned - // in Issue 339 comment #2). - - var bb = svgedit.utilities.getBBox(elem); - if (!bb) { - return null; - } - var angle = svgedit.utilities.getRotationAngle(elem); - - if ((angle && angle % 90) || - svgedit.math.hasMatrixTransform(svgedit.transformlist.getTransformList(elem))) { - // Accurate way to get BBox of rotated element in Firefox: - // Put element in group and get its BBox - var good_bb = false; - // Get the BBox from the raw path for these elements - var elemNames = ['ellipse', 'path', 'line', 'polyline', 'polygon']; - if (elemNames.indexOf(elem.tagName) >= 0) { - bb = good_bb = canvas.convertToPath(elem, true); - } else if (elem.tagName == 'rect') { - // Look for radius - var rx = elem.getAttribute('rx'); - var ry = elem.getAttribute('ry'); - if (rx || ry) { - bb = good_bb = canvas.convertToPath(elem, true); - } - } - - if (!good_bb) { - // Must use clone else FF freaks out - var clone = elem.cloneNode(true); - var g = document.createElementNS(NS.SVG, 'g'); - var parent = elem.parentNode; - parent.appendChild(g); - g.appendChild(clone); - bb = svgedit.utilities.bboxToObj(g.getBBox()); - parent.removeChild(g); - } - - // Old method: Works by giving the rotated BBox, - // this is (unfortunately) what Opera and Safari do - // natively when getting the BBox of the parent group -// var angle = angle * Math.PI / 180.0; -// var rminx = Number.MAX_VALUE, rminy = Number.MAX_VALUE, -// rmaxx = Number.MIN_VALUE, rmaxy = Number.MIN_VALUE; -// var cx = round(bb.x + bb.width/2), -// cy = round(bb.y + bb.height/2); -// var pts = [ [bb.x - cx, bb.y - cy], -// [bb.x + bb.width - cx, bb.y - cy], -// [bb.x + bb.width - cx, bb.y + bb.height - cy], -// [bb.x - cx, bb.y + bb.height - cy] ]; -// var j = 4; -// while (j--) { -// var x = pts[j][0], -// y = pts[j][1], -// r = Math.sqrt( x*x + y*y ); -// var theta = Math.atan2(y,x) + angle; -// x = round(r * Math.cos(theta) + cx); -// y = round(r * Math.sin(theta) + cy); -// -// // now set the bbox for the shape after it's been rotated -// if (x < rminx) rminx = x; -// if (y < rminy) rminy = y; -// if (x > rmaxx) rmaxx = x; -// if (y > rmaxy) rmaxy = y; -// } -// -// bb.x = rminx; -// bb.y = rminy; -// bb.width = rmaxx - rminx; -// bb.height = rmaxy - rminy; - } - return bb; - }; - - var full_bb; - $.each(elems, function() { - if (full_bb) {return;} - if (!this.parentNode) {return;} - full_bb = getCheckedBBox(this); - }); - - // This shouldn't ever happen... - if (full_bb == null) {return null;} - - // full_bb doesn't include the stoke, so this does no good! -// if (elems.length == 1) return full_bb; - - var max_x = full_bb.x + full_bb.width; - var max_y = full_bb.y + full_bb.height; - var min_x = full_bb.x; - var min_y = full_bb.y; - - // FIXME: same re-creation problem with this function as getCheckedBBox() above - var getOffset = function(elem) { - var sw = elem.getAttribute('stroke-width'); - var offset = 0; - if (elem.getAttribute('stroke') != 'none' && !isNaN(sw)) { - offset += sw/2; - } - return offset; - }; - var bboxes = []; - $.each(elems, function(i, elem) { - var cur_bb = getCheckedBBox(elem); - if (cur_bb) { - var offset = getOffset(elem); - min_x = Math.min(min_x, cur_bb.x - offset); - min_y = Math.min(min_y, cur_bb.y - offset); - bboxes.push(cur_bb); - } - }); - - full_bb.x = min_x; - full_bb.y = min_y; - - $.each(elems, function(i, elem) { - var cur_bb = bboxes[i]; - // ensure that elem is really an element node - if (cur_bb && elem.nodeType == 1) { - var offset = getOffset(elem); - max_x = Math.max(max_x, cur_bb.x + cur_bb.width + offset); - max_y = Math.max(max_y, cur_bb.y + cur_bb.height + offset); - } - }); - - full_bb.width = max_x - min_x; - full_bb.height = max_y - min_y; - return full_bb; + return svgedit.utilities.getStrokedBBox( elems, addSvgElementFromJson, pathActions) }; // Function: getVisibleElements diff --git a/editor/svgutils.js b/editor/svgutils.js index 718609f5..31e0961c 100644 --- a/editor/svgutils.js +++ b/editor/svgutils.js @@ -767,6 +767,160 @@ svgedit.utilities.convertToPath = function(elem, attrs, addSvgElementFromJson, p }; +// Function: getBBoxWithTransform +// Get bounding box that includes any transforms. +// +// Parameters: +// elem - The DOM element to be converted +// addSvgElementFromJson - Function to add the path element to the current layer. See canvas.addSvgElementFromJson +// pathActions - If a transform exists, pathActions.resetOrientation() is used. See: canvas.pathActions. +// +// Returns: +// A single bounding box object +svgedit.utilities.getBBoxWithTransform = function(elem, addSvgElementFromJson, pathActions) { + // TODO: Fix issue with rotated groups. Currently they work + // fine in FF, but not in other browsers (same problem mentioned + // in Issue 339 comment #2). + + var bb = svgedit.utilities.getBBox(elem); + + if (!bb) { + return null; + } + + var tlist = svgedit.transformlist.getTransformList(elem) + var angle = svgedit.utilities.getRotationAngleFromTransformList(tlist); + + if (angle || svgedit.math.hasMatrixTransform(tlist)) { + + var good_bb = false; + // Get the BBox from the raw path for these elements + // TODO: why ellipse and not circle + var elemNames = ['ellipse', 'path', 'line', 'polyline', 'polygon']; + if (elemNames.indexOf(elem.tagName) >= 0) { + bb = good_bb = svgedit.utilities.getBBoxOfElementAsPath(elem, addSvgElementFromJson, pathActions); + } else if (elem.tagName == 'rect') { + // Look for radius + var rx = elem.getAttribute('rx'); + var ry = elem.getAttribute('ry'); + if (rx || ry) { + bb = good_bb = svgedit.utilities.getBBoxOfElementAsPath(elem, addSvgElementFromJson, pathActions); + } + } + + if (!good_bb) { + + var matrix = svgedit.math.transformListToTransform( tlist).matrix; + bb = svgedit.math.transformBox(bb.x, bb.y, bb.width, bb.height, matrix).aabox; + + // Old technique that was exceedingly slow with large documents. + // + // Accurate way to get BBox of rotated element in Firefox: + // Put element in group and get its BBox + // + // Must use clone else FF freaks out + //var clone = elem.cloneNode(true); + //var g = document.createElementNS(NS.SVG, 'g'); + //var parent = elem.parentNode; + //parent.appendChild(g); + //g.appendChild(clone); + //var bb2 = svgedit.utilities.bboxToObj(g.getBBox()); + //parent.removeChild(g); + } + + } + return bb; +}; + +// TODO: This is problematic with large stroke-width and, for example, a single horizontal line. The calculated BBox extends way beyond left and right sides. +function getStrokeOffsetForBBox(elem) { + var sw = elem.getAttribute('stroke-width'); + return (!isNaN(sw) && elem.getAttribute('stroke') != 'none') ? sw/2 : 0; +}; + +// Function: getStrokedBBox +// Get the bounding box for one or more stroked and/or transformed elements +// +// Parameters: +// elems - Array with DOM elements to check +// addSvgElementFromJson - Function to add the path element to the current layer. See canvas.addSvgElementFromJson +// pathActions - If a transform exists, pathActions.resetOrientation() is used. See: canvas.pathActions. +// +// Returns: +// A single bounding box object +svgedit.utilities.getStrokedBBox = function(elems, addSvgElementFromJson, pathActions) { + if (!elems || !elems.length) {return false;} + + var full_bb; + $.each(elems, function() { + if (full_bb) {return;} + if (!this.parentNode) {return;} + full_bb = svgedit.utilities.getBBoxWithTransform(this, addSvgElementFromJson, pathActions); + }); + + // This shouldn't ever happen... + if (full_bb === undefined) {return null;} + + // full_bb doesn't include the stoke, so this does no good! + // if (elems.length == 1) return full_bb; + + var max_x = full_bb.x + full_bb.width; + var max_y = full_bb.y + full_bb.height; + var min_x = full_bb.x; + var min_y = full_bb.y; + + // If only one elem, don't call the potentially slow getBBoxWithTransform method again. + if( elems.length === 1) { + var offset = getStrokeOffsetForBBox(elems[0]); + min_x -= offset; + min_y -= offset; + max_x += offset; + max_y += offset; + } else { + $.each(elems, function(i, elem) { + var cur_bb = svgedit.utilities.getBBoxWithTransform(elem, addSvgElementFromJson, pathActions); + if (cur_bb) { + var offset = getStrokeOffsetForBBox(elem); + min_x = Math.min(min_x, cur_bb.x - offset); + min_y = Math.min(min_y, cur_bb.y - offset); + // TODO: The old code had this test for max, but not min. I suspect this test should be for both min and max + if (elem.nodeType == 1) { + max_x = Math.max(max_x, cur_bb.x + cur_bb.width + offset); + max_y = Math.max(max_y, cur_bb.y + cur_bb.height + offset); + } + } + }); + } + + full_bb.x = min_x; + full_bb.y = min_y; + full_bb.width = max_x - min_x; + full_bb.height = max_y - min_y; + return full_bb; +}; + + +// Function: svgedit.utilities.getRotationAngleFromTransformList +// Get the rotation angle of the given transform list. +// +// Parameters: +// tlist - List of transforms +// to_rad - Boolean that when true returns the value in radians rather than degrees +// +// Returns: +// Float with the angle in degrees or radians +svgedit.utilities.getRotationAngleFromTransformList = function(tlist, to_rad) { + if(!tlist) {return 0;} // elements have no tlist + var N = tlist.numberOfItems; + var i; + for (i = 0; i < N; ++i) { + var xform = tlist.getItem(i); + if (xform.type == 4) { + return to_rad ? xform.angle * Math.PI / 180.0 : xform.angle; + } + } + return 0.0; +}; // Function: svgedit.utilities.getRotationAngle // Get the rotation angle of the given/selected DOM element @@ -781,16 +935,7 @@ svgedit.utilities.getRotationAngle = function(elem, to_rad) { var selected = elem || editorContext_.getSelectedElements()[0]; // find the rotation transform (if any) and set it var tlist = svgedit.transformlist.getTransformList(selected); - if(!tlist) {return 0;} // elements have no tlist - var N = tlist.numberOfItems; - var i; - for (i = 0; i < N; ++i) { - var xform = tlist.getItem(i); - if (xform.type == 4) { - return to_rad ? xform.angle * Math.PI / 180.0 : xform.angle; - } - } - return 0.0; + return svgedit.utilities.getRotationAngleFromTransformList(tlist, to_rad) }; // Function getRefElem diff --git a/test/all_tests.html b/test/all_tests.html index c595e0e7..270a1a6e 100644 --- a/test/all_tests.html +++ b/test/all_tests.html @@ -12,6 +12,7 @@ + diff --git a/test/qunit/qunit-assert-close.js b/test/qunit/qunit-assert-close.js new file mode 100644 index 00000000..4b71e3df --- /dev/null +++ b/test/qunit/qunit-assert-close.js @@ -0,0 +1,106 @@ +/** + * Checks that the first two arguments are equal, or are numbers close enough to be considered equal + * based on a specified maximum allowable difference. + * + * @example assert.close(3.141, Math.PI, 0.001); + * + * @param Number actual + * @param Number expected + * @param Number maxDifference (the maximum inclusive difference allowed between the actual and expected numbers) + * @param String message (optional) + */ +function close(actual, expected, maxDifference, message) { + var actualDiff = (actual === expected) ? 0 : Math.abs(actual - expected), + result = actualDiff <= maxDifference; + message = message || (actual + " should be within " + maxDifference + " (inclusive) of " + expected + (result ? "" : ". Actual: " + actualDiff)); + QUnit.push(result, actual, expected, message); +} + + +/** + * Checks that the first two arguments are equal, or are numbers close enough to be considered equal + * based on a specified maximum allowable difference percentage. + * + * @example assert.close.percent(155, 150, 3.4); // Difference is ~3.33% + * + * @param Number actual + * @param Number expected + * @param Number maxPercentDifference (the maximum inclusive difference percentage allowed between the actual and expected numbers) + * @param String message (optional) + */ +close.percent = function closePercent(actual, expected, maxPercentDifference, message) { + var actualDiff, result; + if (actual === expected) { + actualDiff = 0; + result = actualDiff <= maxPercentDifference; + } + else if (actual !== 0 && expected !== 0 && expected !== Infinity && expected !== -Infinity) { + actualDiff = Math.abs(100 * (actual - expected) / expected); + result = actualDiff <= maxPercentDifference; + } + else { + // Dividing by zero (0)! Should return `false` unless the max percentage was `Infinity` + actualDiff = Infinity; + result = maxPercentDifference === Infinity; + } + message = message || (actual + " should be within " + maxPercentDifference + "% (inclusive) of " + expected + (result ? "" : ". Actual: " + actualDiff + "%")); + + QUnit.push(result, actual, expected, message); +}; + + +/** + * Checks that the first two arguments are numbers with differences greater than the specified + * minimum difference. + * + * @example assert.notClose(3.1, Math.PI, 0.001); + * + * @param Number actual + * @param Number expected + * @param Number minDifference (the minimum exclusive difference allowed between the actual and expected numbers) + * @param String message (optional) + */ +function notClose(actual, expected, minDifference, message) { + var actualDiff = Math.abs(actual - expected), + result = actualDiff > minDifference; + message = message || (actual + " should not be within " + minDifference + " (exclusive) of " + expected + (result ? "" : ". Actual: " + actualDiff)); + QUnit.push(result, actual, expected, message); +} + + +/** + * Checks that the first two arguments are numbers with differences greater than the specified + * minimum difference percentage. + * + * @example assert.notClose.percent(156, 150, 3.5); // Difference is 4.0% + * + * @param Number actual + * @param Number expected + * @param Number minPercentDifference (the minimum exclusive difference percentage allowed between the actual and expected numbers) + * @param String message (optional) + */ +notClose.percent = function notClosePercent(actual, expected, minPercentDifference, message) { + var actualDiff, result; + if (actual === expected) { + actualDiff = 0; + result = actualDiff > minPercentDifference; + } + else if (actual !== 0 && expected !== 0 && expected !== Infinity && expected !== -Infinity) { + actualDiff = Math.abs(100 * (actual - expected) / expected); + result = actualDiff > minPercentDifference; + } + else { + // Dividing by zero (0)! Should only return `true` if the min percentage was `Infinity` + actualDiff = Infinity; + result = minPercentDifference !== Infinity; + } + message = message || (actual + " should not be within " + minPercentDifference + "% (exclusive) of " + expected + (result ? "" : ". Actual: " + actualDiff + "%")); + + QUnit.push(result, actual, expected, message); +}; + + +//QUnit.extend(QUnit.assert, { +// close: close, +// notClose: notClose +//}); \ No newline at end of file diff --git a/test/svgutils_bbox_test.html b/test/svgutils_bbox_test.html new file mode 100644 index 00000000..ef12703c --- /dev/null +++ b/test/svgutils_bbox_test.html @@ -0,0 +1,511 @@ + + + + + Unit Tests for svgutils.js BBox functions + + + + + + + + + + + + + + + + +

    Unit Tests for svgutils.js BBox functions

    +

    +

    +
      +
      + + diff --git a/test/svgutils_performance_test.html b/test/svgutils_performance_test.html new file mode 100644 index 00000000..79c6eb8c --- /dev/null +++ b/test/svgutils_performance_test.html @@ -0,0 +1,247 @@ + + + + + + + Performance Unit Tests for svgutils.js + + + + + + + + + + + + + + + + +

      Performance Unit Tests for svgutils.js

      +

      +

      +
        + +
        +
        + + + + + + + + +
        + + + + + + + + + Layer 1 + + + + + + + + + + + + Some text + + + + Layer 2 + + + + +
        +
        +
        + + + From 17c3e0fa28c4d1f23f47751392a8938e352b3927 Mon Sep 17 00:00:00 2001 From: Flint O'Brien Date: Sun, 24 Apr 2016 16:56:32 -0400 Subject: [PATCH 5/7] Performance improvement for select.js and canvas.addToSelection canvas.addToSelection was calling getBBox, throwing it away, then calling selectorManager.requestSelector() which called getBBox again. Now passing-in an optional bbox to selector functions. --- editor/select.js | 25 ++++++++++++++++--------- editor/svgcanvas.js | 6 ++++-- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/editor/select.js b/editor/select.js index 4ac342ef..088c4ad7 100644 --- a/editor/select.js +++ b/editor/select.js @@ -33,7 +33,8 @@ var gripRadius = svgedit.browser.isTouch() ? 10 : 4; // Parameters: // id - integer to internally indentify the selector // elem - DOM element associated with this selector -svgedit.select.Selector = function(id, elem) { +// bbox - Optional bbox to use for initialization (prevents duplicate getBBox call). +svgedit.select.Selector = function(id, elem, bbox) { // this is the selector's unique number this.id = id; @@ -77,7 +78,7 @@ svgedit.select.Selector = function(id, elem) { 'w' : null }; - this.reset(this.selectedElement); + this.reset(this.selectedElement, bbox); }; @@ -86,10 +87,11 @@ svgedit.select.Selector = function(id, elem) { // // Parameters: // e - DOM element associated with this selector -svgedit.select.Selector.prototype.reset = function(e) { +// bbox - Optional bbox to use for reset (prevents duplicate getBBox call). +svgedit.select.Selector.prototype.reset = function(e, bbox) { this.locked = true; this.selectedElement = e; - this.resize(); + this.resize(bbox); this.selectorGroup.setAttribute('display', 'inline'); }; @@ -136,7 +138,8 @@ svgedit.select.Selector.prototype.showGrips = function(show) { // Function: svgedit.select.Selector.resize // Updates the selector to match the element's size -svgedit.select.Selector.prototype.resize = function() { +// bbox - Optional bbox to use for resize (prevents duplicate getBBox call). +svgedit.select.Selector.prototype.resize = function(bbox) { var selectedBox = this.selectorRect, mgr = selectorManager_, selectedGrips = mgr.selectorGrips, @@ -162,7 +165,10 @@ svgedit.select.Selector.prototype.resize = function() { m.e *= current_zoom; m.f *= current_zoom; - var bbox = svgedit.utilities.getBBox(selected); + if (!bbox) + bbox = svgedit.utilities.getBBox(selected); + // TODO: svgedit.utilities.getBBox (previous line) already knows to call getStrokedBBox when tagName === 'g'. Remove this? + // TODO: svgedit.utilities.getBBox doesn't exclude 'gsvg' and calls getStrokedBBox for any 'g'. Should getBBox be updated? if (tagName === 'g' && !$.data(selected, 'gsvg')) { // The bbox for a group does not include stroke vals, so we // get the bbox based on its children. @@ -413,7 +419,8 @@ svgedit.select.SelectorManager.prototype.initGroup = function() { // // Parameters: // elem - DOM element to get the selector for -svgedit.select.SelectorManager.prototype.requestSelector = function(elem) { +// bbox - Optional bbox to use for reset (prevents duplicate getBBox call). +svgedit.select.SelectorManager.prototype.requestSelector = function(elem, bbox) { if (elem == null) {return null;} var i, N = this.selectors.length; @@ -425,13 +432,13 @@ svgedit.select.SelectorManager.prototype.requestSelector = function(elem) { for (i = 0; i < N; ++i) { if (this.selectors[i] && !this.selectors[i].locked) { this.selectors[i].locked = true; - this.selectors[i].reset(elem); + this.selectors[i].reset(elem, bbox); this.selectorMap[elem.id] = this.selectors[i]; return this.selectors[i]; } } // if we reached here, no available selectors were found, we create one - this.selectors[N] = new svgedit.select.Selector(N, elem); + this.selectors[N] = new svgedit.select.Selector(N, elem, bbox); this.selectorParentGroup.appendChild(this.selectors[N].selectorGroup); this.selectorMap[elem.id] = this.selectors[N]; return this.selectors[N]; diff --git a/editor/svgcanvas.js b/editor/svgcanvas.js index 1511bc30..37062ace 100644 --- a/editor/svgcanvas.js +++ b/editor/svgcanvas.js @@ -940,7 +940,9 @@ var addToSelection = this.addToSelection = function(elemsToAdd, showGrips) { var i = elemsToAdd.length; while (i--) { var elem = elemsToAdd[i]; - if (!elem || !svgedit.utilities.getBBox(elem)) {continue;} + if (!elem) {continue;} + var bbox = svgedit.utilities.getBBox(elem); + if (!bbox) {continue;} if (elem.tagName === 'a' && elem.childNodes.length === 1) { // Make "a" element's child be the selected element @@ -955,7 +957,7 @@ var addToSelection = this.addToSelection = function(elemsToAdd, showGrips) { // only the first selectedBBoxes element is ever used in the codebase these days // if (j == 0) selectedBBoxes[0] = svgedit.utilities.getBBox(elem); j++; - var sel = selectorManager.requestSelector(elem); + var sel = selectorManager.requestSelector(elem, bbox); if (selectedElements.length > 1) { sel.showGrips(false); From 7db3b22c5897f06078c6a8f5925eec852ac4d547 Mon Sep 17 00:00:00 2001 From: Flint O'Brien Date: Tue, 26 Apr 2016 16:01:39 -0400 Subject: [PATCH 6/7] Updates from pull request code review. Refactored getExtraAttributesForConvertToPath. Updated all formatting requests. Refactored and renamed addAttributesForConvertToPath to getExtraAttributesForConvertToPath. Now called from getBBoxOfElementAsPath and convertToPath. --- editor/select.js | 3 +- editor/svgcanvas.js | 32 +++++++++-------- editor/svgutils.js | 86 +++++++++++++++++++++++++++------------------ 3 files changed, 70 insertions(+), 51 deletions(-) diff --git a/editor/select.js b/editor/select.js index 088c4ad7..3b2e35de 100644 --- a/editor/select.js +++ b/editor/select.js @@ -165,8 +165,9 @@ svgedit.select.Selector.prototype.resize = function(bbox) { m.e *= current_zoom; m.f *= current_zoom; - if (!bbox) + if (!bbox) { bbox = svgedit.utilities.getBBox(selected); + } // TODO: svgedit.utilities.getBBox (previous line) already knows to call getStrokedBBox when tagName === 'g'. Remove this? // TODO: svgedit.utilities.getBBox doesn't exclude 'gsvg' and calls getStrokedBBox for any 'g'. Should getBBox be updated? if (tagName === 'g' && !$.data(selected, 'gsvg')) { diff --git a/editor/svgcanvas.js b/editor/svgcanvas.js index 37062ace..9137f6ce 100644 --- a/editor/svgcanvas.js +++ b/editor/svgcanvas.js @@ -603,7 +603,7 @@ var getIntersectionList = this.getIntersectionList = function(rect) { // A single bounding box object var getStrokedBBox = this.getStrokedBBox = function(elems) { if (!elems) {elems = getVisibleElements();} - return svgedit.utilities.getStrokedBBox( elems, addSvgElementFromJson, pathActions) + return svgedit.utilities.getStrokedBBox(elems, addSvgElementFromJson, pathActions) }; // Function: getVisibleElements @@ -6452,22 +6452,24 @@ this.convertToPath = function(elem, getBBox) { }); return; } - if( getBBox) { - return svgedit.utilities.getBBoxOfElementAsPath( elem, addSvgElementFromJson, pathActions) + if (getBBox) { + return svgedit.utilities.getBBoxOfElementAsPath(elem, addSvgElementFromJson, pathActions) } else { + // TODO: Why is this applying attributes from cur_shape, then inside utilities.convertToPath it's pulling addition attributes from elem? + // TODO: If convertToPath is called with one elem, cur_shape and elem are probably the same; but calling with multiple is a bug or cool feature. var attrs = { - 'fill': cur_shape.fill, - 'fill-opacity': cur_shape.fill_opacity, - 'stroke': cur_shape.stroke, - 'stroke-width': cur_shape.stroke_width, - 'stroke-dasharray': cur_shape.stroke_dasharray, - 'stroke-linejoin': cur_shape.stroke_linejoin, - 'stroke-linecap': cur_shape.stroke_linecap, - 'stroke-opacity': cur_shape.stroke_opacity, - 'opacity': cur_shape.opacity, - 'visibility':'hidden' - }; - return svgedit.utilities.convertToPath( elem, attrs, addSvgElementFromJson, pathActions, clearSelection, addToSelection, svgedit.history, addCommandToHistory) + 'fill': cur_shape.fill, + 'fill-opacity': cur_shape.fill_opacity, + 'stroke': cur_shape.stroke, + 'stroke-width': cur_shape.stroke_width, + 'stroke-dasharray': cur_shape.stroke_dasharray, + 'stroke-linejoin': cur_shape.stroke_linejoin, + 'stroke-linecap': cur_shape.stroke_linecap, + 'stroke-opacity': cur_shape.stroke_opacity, + 'opacity': cur_shape.opacity, + 'visibility':'hidden' + }; + return svgedit.utilities.convertToPath(elem, attrs, addSvgElementFromJson, pathActions, clearSelection, addToSelection, svgedit.history, addCommandToHistory); } }; diff --git a/editor/svgutils.js b/editor/svgutils.js index 31e0961c..b7cf2c92 100644 --- a/editor/svgutils.js +++ b/editor/svgutils.js @@ -529,22 +529,39 @@ svgedit.utilities.getBBox = function(elem) { return ret; }; -svgedit.utilities.getPathDFromSegments = function( pathSegments) { +// Function: getPathDFromSegments +// Create a path 'd' attribute from path segments. +// Each segment is an array of the form: [singleChar, [x,y, x,y, ...]] +// +// Parameters: +// pathSegments - An array of path segments to be converted +// +// Returns: +// The converted path d attribute. +svgedit.utilities.getPathDFromSegments = function(pathSegments) { var d = ''; $.each(pathSegments, function(j, seg) { var i; - var l = seg[0], pts = seg[1]; - d += l; + var pts = seg[1]; + d += seg[0]; for (i = 0; i < pts.length; i+=2) { d += (pts[i] +','+pts[i+1]) + ' '; } }); - return d -} + return d; +}; -svgedit.utilities.getPathDFromElement = function( elem) { +// Function: getPathDFromElement +// Make a path 'd' attribute from a simple SVG element shape. +// +// Parameters: +// elem - The element to be converted +// +// Returns: +// The path d attribute or undefined if the element type is unknown. +svgedit.utilities.getPathDFromElement = function(elem) { // Possibly the cubed root of 6, but 1.81 works best var num = 1.81; @@ -623,14 +640,25 @@ svgedit.utilities.getPathDFromElement = function( elem) { }; -svgedit.utilities.addAttributesForConvertToPath = function( elem, attrs) { +// Function: getExtraAttributesForConvertToPath +// Get a set of attributes from an element that is useful for convertToPath. +// +// Parameters: +// elem - The element to be probed +// +// Returns: +// An object with attributes. +svgedit.utilities.getExtraAttributesForConvertToPath = function(elem) { + var attrs = {} ; // TODO: make this list global so that we can properly maintain it // TODO: what about @transform, @clip-rule, @fill-rule, etc? $.each(['marker-start', 'marker-end', 'marker-mid', 'filter', 'clip-path'], function() { - if (elem.getAttribute(this)) { - attrs[this] = elem.getAttribute(this); + var a = elem.getAttribute(this); + if (a) { + attrs[this] = a; } }); + return attrs; }; // Function: getBBoxOfElementAsPath @@ -638,21 +666,16 @@ svgedit.utilities.addAttributesForConvertToPath = function( elem, attrs) { // // Parameters: // elem - The DOM element to be probed -// addSvgElementFromJson - Function to add the path element to the current layer. See canvas.addSvgElementFromJso +// addSvgElementFromJson - Function to add the path element to the current layer. See canvas.addSvgElementFromJson // pathActions - If a transform exists, pathActions.resetOrientation() is used. See: canvas.pathActions. // // Returns: // The resulting path's bounding box object. svgedit.utilities.getBBoxOfElementAsPath = function(elem, addSvgElementFromJson, pathActions) { - var attrs = {} - - // any attribute on the element not covered by the above - svgedit.utilities.addAttributesForConvertToPath( elem, attrs) - var path = addSvgElementFromJson({ 'element': 'path', - 'attr': attrs + 'attr': svgedit.utilities.getExtraAttributesForConvertToPath(elem) }); var eltrans = elem.getAttribute('transform'); @@ -660,7 +683,6 @@ svgedit.utilities.getBBoxOfElementAsPath = function(elem, addSvgElementFromJson, path.setAttribute('transform', eltrans); } - var id = elem.id; var parent = elem.parentNode; if (elem.nextSibling) { parent.insertBefore(path, elem); @@ -668,8 +690,8 @@ svgedit.utilities.getBBoxOfElementAsPath = function(elem, addSvgElementFromJson, parent.appendChild(path); } - var d = svgedit.utilities.getPathDFromElement( elem); - if( d) + var d = svgedit.utilities.getPathDFromElement(elem); + if (d) path.setAttribute('d', d); else path.parentNode.removeChild(path); @@ -684,7 +706,7 @@ svgedit.utilities.getBBoxOfElementAsPath = function(elem, addSvgElementFromJson, } path.parentNode.removeChild(path); return bb; -} +}; // Function: convertToPath // Convert selected element to a path. @@ -692,7 +714,7 @@ svgedit.utilities.getBBoxOfElementAsPath = function(elem, addSvgElementFromJson, // Parameters: // elem - The DOM element to be converted // attrs - Apply attributes to new path. see canvas.convertToPath -// addSvgElementFromJson - Function to add the path element to the current layer. See canvas.addSvgElementFromJso +// addSvgElementFromJson - Function to add the path element to the current layer. See canvas.addSvgElementFromJson // pathActions - If a transform exists, pathActions.resetOrientation() is used. See: canvas.pathActions. // clearSelection - see canvas.clearSelection // addToSelection - see canvas.addToSelection @@ -705,14 +727,8 @@ svgedit.utilities.convertToPath = function(elem, attrs, addSvgElementFromJson, p var batchCmd = new history.BatchCommand('Convert element to Path'); - // any attribute on the element not covered by the above - // TODO: make this list global so that we can properly maintain it - // TODO: what about @transform, @clip-rule, @fill-rule, etc? - $.each(['marker-start', 'marker-end', 'marker-mid', 'filter', 'clip-path'], function() { - if (elem.getAttribute(this)) { - attrs[this] = elem.getAttribute(this); - } - }); + // Any attribute on the element not covered by the passed-in attributes + attrs = $.extend({}, attrs, svgedit.utilities.getExtraAttributesForConvertToPath(elem)); var path = addSvgElementFromJson({ 'element': 'path', @@ -732,8 +748,8 @@ svgedit.utilities.convertToPath = function(elem, attrs, addSvgElementFromJson, p parent.appendChild(path); } - var d = svgedit.utilities.getPathDFromElement( elem); - if( d) { + var d = svgedit.utilities.getPathDFromElement(elem); + if (d) { path.setAttribute('d', d); // Replace the current element with the converted one @@ -788,7 +804,7 @@ svgedit.utilities.getBBoxWithTransform = function(elem, addSvgElementFromJson, p return null; } - var tlist = svgedit.transformlist.getTransformList(elem) + var tlist = svgedit.transformlist.getTransformList(elem); var angle = svgedit.utilities.getRotationAngleFromTransformList(tlist); if (angle || svgedit.math.hasMatrixTransform(tlist)) { @@ -810,7 +826,7 @@ svgedit.utilities.getBBoxWithTransform = function(elem, addSvgElementFromJson, p if (!good_bb) { - var matrix = svgedit.math.transformListToTransform( tlist).matrix; + var matrix = svgedit.math.transformListToTransform(tlist).matrix; bb = svgedit.math.transformBox(bb.x, bb.y, bb.width, bb.height, matrix).aabox; // Old technique that was exceedingly slow with large documents. @@ -870,7 +886,7 @@ svgedit.utilities.getStrokedBBox = function(elems, addSvgElementFromJson, pathAc var min_y = full_bb.y; // If only one elem, don't call the potentially slow getBBoxWithTransform method again. - if( elems.length === 1) { + if (elems.length === 1) { var offset = getStrokeOffsetForBBox(elems[0]); min_x -= offset; min_y -= offset; @@ -910,7 +926,7 @@ svgedit.utilities.getStrokedBBox = function(elems, addSvgElementFromJson, pathAc // Returns: // Float with the angle in degrees or radians svgedit.utilities.getRotationAngleFromTransformList = function(tlist, to_rad) { - if(!tlist) {return 0;} // elements have no tlist + if (!tlist) {return 0;} // elements have no tlist var N = tlist.numberOfItems; var i; for (i = 0; i < N; ++i) { From 3230520d679a334cc1938dfbb5972783f63fd9b9 Mon Sep 17 00:00:00 2001 From: Flint O'Brien Date: Thu, 28 Apr 2016 12:11:54 -0400 Subject: [PATCH 7/7] Optimized getBBoxWithTransform when rotation multiple of 90 This feature was in previous release, but broken. This update uses standard, faster bounding box calculation for simple shapes when angle is multiple of 90. Updated tests. Fixed two tests that were broken in Safari. --- editor/svgutils.js | 54 ++++++++++++++++++++++++++++-------- test/svgutils_bbox_test.html | 32 +++++++++++++++------ 2 files changed, 66 insertions(+), 20 deletions(-) diff --git a/editor/svgutils.js b/editor/svgutils.js index b7cf2c92..3fef5302 100644 --- a/editor/svgutils.js +++ b/editor/svgutils.js @@ -783,6 +783,35 @@ svgedit.utilities.convertToPath = function(elem, attrs, addSvgElementFromJson, p }; +// Function: bBoxCanBeOptimizedOverNativeGetBBox +// Can the bbox be optimized over the native getBBox? The optimized bbox is the same as the native getBBox when +// the rotation angle is a multiple of 90 degrees and there are no complex transforms. +// Getting an optimized bbox can be dramatically slower, so we want to make sure it's worth it. +// +// The best example for this is a circle rotate 45 degrees. The circle doesn't get wider or taller when rotated +// about it's center. +// +// The standard, unoptimized technique gets the native bbox of the circle, rotates the box 45 degrees, uses +// that width and height, and applies any transforms to get the final bbox. This means the calculated bbox +// is much wider than the original circle. If the angle had been 0, 90, 180, etc. both techniques render the +// same bbox. +// +// The optimization is not needed if the rotation is a multiple 90 degrees. The default technique is to call +// getBBox then apply the angle and any transforms. +// +// Parameters: +// angle - The rotation angle in degrees +// hasMatrixTransform - True if there is a matrix transform +// +// Returns: +// True if the bbox can be optimized. +function bBoxCanBeOptimizedOverNativeGetBBox(angle, hasMatrixTransform) { + var angleModulo90 = angle % 90; + var closeTo90 = angleModulo90 < -89.99 || angleModulo90 > 89.99; + var closeTo0 = angleModulo90 > -0.001 && angleModulo90 < 0.001; + return hasMatrixTransform || ! (closeTo0 || closeTo90); +} + // Function: getBBoxWithTransform // Get bounding box that includes any transforms. // @@ -806,21 +835,24 @@ svgedit.utilities.getBBoxWithTransform = function(elem, addSvgElementFromJson, p var tlist = svgedit.transformlist.getTransformList(elem); var angle = svgedit.utilities.getRotationAngleFromTransformList(tlist); + var hasMatrixTransform = svgedit.math.hasMatrixTransform(tlist); - if (angle || svgedit.math.hasMatrixTransform(tlist)) { + if (angle || hasMatrixTransform) { var good_bb = false; - // Get the BBox from the raw path for these elements - // TODO: why ellipse and not circle - var elemNames = ['ellipse', 'path', 'line', 'polyline', 'polygon']; - if (elemNames.indexOf(elem.tagName) >= 0) { - bb = good_bb = svgedit.utilities.getBBoxOfElementAsPath(elem, addSvgElementFromJson, pathActions); - } else if (elem.tagName == 'rect') { - // Look for radius - var rx = elem.getAttribute('rx'); - var ry = elem.getAttribute('ry'); - if (rx || ry) { + if (bBoxCanBeOptimizedOverNativeGetBBox(angle, hasMatrixTransform)) { + // Get the BBox from the raw path for these elements + // TODO: why ellipse and not circle + var elemNames = ['ellipse', 'path', 'line', 'polyline', 'polygon']; + if (elemNames.indexOf(elem.tagName) >= 0) { bb = good_bb = svgedit.utilities.getBBoxOfElementAsPath(elem, addSvgElementFromJson, pathActions); + } else if (elem.tagName == 'rect') { + // Look for radius + var rx = elem.getAttribute('rx'); + var ry = elem.getAttribute('ry'); + if (rx || ry) { + bb = good_bb = svgedit.utilities.getBBoxOfElementAsPath(elem, addSvgElementFromJson, pathActions); + } } } diff --git a/test/svgutils_bbox_test.html b/test/svgutils_bbox_test.html index ef12703c..2ae7927e 100644 --- a/test/svgutils_bbox_test.html +++ b/test/svgutils_bbox_test.html @@ -32,9 +32,11 @@ } return elem; } + var mockAddSvgElementFromJsonCallCount = 0; function mockAddSvgElementFromJson( json) { var elem = mockCreateSVGElement( json) svgroot.appendChild( elem) + mockAddSvgElementFromJsonCallCount++; return elem } var mockPathActions = { @@ -82,7 +84,8 @@ setup: function() { // We're reusing ID's so we need to do this for transforms. svgedit.transformlist.resetListMap(); - svgedit.path.init(null) + svgedit.path.init(null); + mockAddSvgElementFromJsonCallCount = 0; }, teardown: function() { } @@ -109,6 +112,7 @@ svgroot.appendChild( elem) bbox = getBBoxWithTransform(elem, mockAddSvgElementFromJson, mockPathActions) deepEqual(bbox, {"x": 0, "y": 1, "width": 2, "height": 2 }); + equal( mockAddSvgElementFromJsonCallCount, 0); svgroot.removeChild( elem); elem = mockCreateSVGElement({ @@ -118,6 +122,7 @@ svgroot.appendChild( elem); bbox = getBBoxWithTransform( elem, mockAddSvgElementFromJson, mockPathActions) deepEqual( bbox, { "x": 0, "y": 1, "width": 5, "height": 10}); + equal( mockAddSvgElementFromJsonCallCount, 0); svgroot.removeChild( elem); elem = mockCreateSVGElement({ @@ -127,6 +132,7 @@ svgroot.appendChild( elem); bbox = getBBoxWithTransform( elem, mockAddSvgElementFromJson, mockPathActions) deepEqual( bbox, { "x": 0, "y": 1, "width": 5, "height": 5}); + equal( mockAddSvgElementFromJsonCallCount, 0); svgroot.removeChild( elem); elem = mockCreateSVGElement({ @@ -141,6 +147,7 @@ svgroot.appendChild( g); bbox = getBBoxWithTransform( elem, mockAddSvgElementFromJson, mockPathActions) deepEqual( bbox, { "x": 0, "y": 1, "width": 5, "height": 10}); + equal( mockAddSvgElementFromJsonCallCount, 0); svgroot.removeChild( g); }); @@ -172,6 +179,7 @@ close( bbox.y, 15, EPSILON); close( bbox.width, 20, EPSILON); close( bbox.height, 10, EPSILON); + equal( mockAddSvgElementFromJsonCallCount, 1); svgroot.removeChild( elem); var rect = {x: 10, y: 10, width: 10, height: 20}; @@ -182,12 +190,14 @@ 'attr': { 'id': 'rect2', 'x': rect.x, 'y': rect.y, 'width': rect.width, 'height': rect.height, 'transform': 'rotate(' + angle + ' ' + origin.x + ',' + origin.y + ')'} }); svgroot.appendChild( elem); + mockAddSvgElementFromJsonCallCount = 0; bbox = getBBoxWithTransform( elem, mockAddSvgElementFromJson, mockPathActions); var r2 = rotateRect( rect, angle, origin); close( bbox.x, r2.x, EPSILON, 'rect2 x is ' + r2.x); close( bbox.y, r2.y, EPSILON, 'rect2 y is ' + r2.y); close( bbox.width, r2.width, EPSILON, 'rect2 width is' + r2.width); close( bbox.height, r2.height, EPSILON, 'rect2 height is ' + r2.height); + equal( mockAddSvgElementFromJsonCallCount, 0); svgroot.removeChild( elem); @@ -202,11 +212,13 @@ }); g.appendChild( elem); svgroot.appendChild( g); + mockAddSvgElementFromJsonCallCount = 0; bbox = getBBoxWithTransform( g, mockAddSvgElementFromJson, mockPathActions); close( bbox.x, r2.x, EPSILON, 'rect2 x is ' + r2.x); close( bbox.y, r2.y, EPSILON, 'rect2 y is ' + r2.y); close( bbox.width, r2.width, EPSILON, 'rect2 width is' + r2.width); close( bbox.height, r2.height, EPSILON, 'rect2 height is ' + r2.height); + equal( mockAddSvgElementFromJsonCallCount, 0); svgroot.removeChild( g); @@ -215,12 +227,14 @@ 'attr': { 'id': 'ellipse1', 'cx': '100', 'cy': '100', 'rx': '50', 'ry': '50', 'transform': 'rotate(45 100,100)'} }); svgroot.appendChild( elem); + mockAddSvgElementFromJsonCallCount = 0; bbox = getBBoxWithTransform( elem, mockAddSvgElementFromJson, mockPathActions); // TODO: the BBox algorithm is using the bezier control points to calculate the bounding box. Should be 50, 50, 100, 100. - close( bbox.x, 45.111, EPSILON); - close( bbox.y, 45.111, EPSILON); - close( bbox.width, 109.777, EPSILON); - close( bbox.height, 109.777, EPSILON); + ok( bbox.x > 45 && bbox.x <= 50); + ok( bbox.y > 45 && bbox.y <= 50); + ok( bbox.width >= 100 && bbox.width < 110); + ok( bbox.height >= 100 && bbox.height < 110); + equal( mockAddSvgElementFromJsonCallCount, 1); svgroot.removeChild( elem); }); @@ -311,10 +325,10 @@ svgroot.appendChild( elem); bbox = getBBoxWithTransform( elem, mockAddSvgElementFromJson, mockPathActions) // TODO: the BBox algorithm is using the bezier control points to calculate the bounding box. Should be 50, 50, 100, 100. - close( bbox.x, 45.111 + tx, EPSILON); - close( bbox.y, 45.111 + ty, EPSILON); - close( bbox.width, 109.777, EPSILON); - close( bbox.height, 109.777, EPSILON); + ok( bbox.x > 45 + tx && bbox.x <= 50 + tx); + ok( bbox.y > 45 + ty && bbox.y <= 50 + ty); + ok( bbox.width >= 100 && bbox.width < 110); + ok( bbox.height >= 100 && bbox.height < 110); svgroot.removeChild( elem); });