From 3230520d679a334cc1938dfbb5972783f63fd9b9 Mon Sep 17 00:00:00 2001 From: Flint O'Brien Date: Thu, 28 Apr 2016 12:11:54 -0400 Subject: [PATCH] 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); });