From 6a6b74aae1fd97d33f1479e1377b0040c3370ceb Mon Sep 17 00:00:00 2001 From: asturur Date: Tue, 3 Feb 2015 00:02:16 +0100 Subject: [PATCH 1/3] Fixes svg rendering rules --- src/shapes/circle.class.js | 2 +- src/shapes/polygon.class.js | 30 ++++++++++++++++++------------ src/shapes/polyline.class.js | 8 +++----- src/shapes/rect.class.js | 5 +++-- test/unit/polygon.js | 29 ++++++++++++++++++++--------- test/unit/polyline.js | 27 +++++++++++++++++++-------- test/unit/rect.js | 5 +++-- 7 files changed, 67 insertions(+), 39 deletions(-) diff --git a/src/shapes/circle.class.js b/src/shapes/circle.class.js index 997a863b..c9d7da56 100644 --- a/src/shapes/circle.class.js +++ b/src/shapes/circle.class.js @@ -228,7 +228,7 @@ * @private */ function isValidRadius(attributes) { - return (('radius' in attributes) && (attributes.radius > 0)); + return (('radius' in attributes) && (attributes.radius >= 0)); } /* _FROM_SVG_END_ */ diff --git a/src/shapes/polygon.class.js b/src/shapes/polygon.class.js index 003fc56a..50f85a43 100644 --- a/src/shapes/polygon.class.js +++ b/src/shapes/polygon.class.js @@ -57,7 +57,7 @@ */ initialize: function(points, options) { options = options || { }; - this.points = points; + this.points = points || [ ]; this.callSuper('initialize', options); this._calcDimensions(); if (!('top' in options)) { @@ -79,11 +79,11 @@ maxX = max(points, 'x'), maxY = max(points, 'y'); - this.width = (maxX - minX) || 1; - this.height = (maxY - minY) || 1; + this.width = (maxX - minX) || 0; + this.height = (maxY - minY) || 0; - this.minX = minX, - this.minY = minY; + this.minX = minX || 0, + this.minY = minY || 0; }, /** @@ -141,7 +141,9 @@ * @param {CanvasRenderingContext2D} ctx Context to render on */ _render: function(ctx) { - this.commonRender(ctx); + if(!this.commonRender(ctx)) { + return; + } this._renderFill(ctx); if (this.stroke || this.strokeDashArray) { ctx.closePath(); @@ -154,7 +156,14 @@ * @param {CanvasRenderingContext2D} ctx Context to render on */ commonRender: function(ctx) { - var point; + var point, len = this.points.length; + + if (!len || isNaN(this.points[len - 1].y)) { + // do not draw if no points or odd points + // NaN comes from parseFloat of a empty string in parser + return false; + } + ctx.beginPath(); if (this._applyPointOffset) { @@ -165,10 +174,11 @@ } ctx.moveTo(this.points[0].x, this.points[0].y); - for (var i = 0, len = this.points.length; i < len; i++) { + for (var i = 0; i < len; i++) { point = this.points[i]; ctx.lineTo(point.x, point.y); } + return true; }, /** @@ -216,10 +226,6 @@ var points = fabric.parsePointsAttribute(element.getAttribute('points')), parsedAttributes = fabric.parseAttributes(element, fabric.Polygon.ATTRIBUTE_NAMES); - if (points === null) { - return null; - } - return new fabric.Polygon(points, extend(parsedAttributes, options)); }; /* _FROM_SVG_END_ */ diff --git a/src/shapes/polyline.class.js b/src/shapes/polyline.class.js index f57b670d..84dd5c80 100644 --- a/src/shapes/polyline.class.js +++ b/src/shapes/polyline.class.js @@ -108,7 +108,9 @@ * @param {CanvasRenderingContext2D} ctx Context to render on */ _render: function(ctx) { - fabric.Polygon.prototype.commonRender.call(this, ctx); + if(!fabric.Polygon.prototype.commonRender.call(this, ctx)) { + return; + } this._renderFill(ctx); this._renderStroke(ctx); }, @@ -163,10 +165,6 @@ var points = fabric.parsePointsAttribute(element.getAttribute('points')), parsedAttributes = fabric.parseAttributes(element, fabric.Polyline.ATTRIBUTE_NAMES); - if (points === null) { - return null; - } - return new fabric.Polyline(points, fabric.util.object.extend(parsedAttributes, options)); }; /* _FROM_SVG_END_ */ diff --git a/src/shapes/rect.class.js b/src/shapes/rect.class.js index 195085cf..9e60b0b2 100644 --- a/src/shapes/rect.class.js +++ b/src/shapes/rect.class.js @@ -221,8 +221,9 @@ parsedAttributes.left = parsedAttributes.left || 0; parsedAttributes.top = parsedAttributes.top || 0; - - return new fabric.Rect(extend((options ? fabric.util.object.clone(options) : { }), parsedAttributes)); + var rect = new fabric.Rect(extend((options ? fabric.util.object.clone(options) : { }), parsedAttributes)); + rect.visible = rect.width > 0 && rect.height > 0; + return rect; }; /* _FROM_SVG_END_ */ diff --git a/test/unit/polygon.js b/test/unit/polygon.js index 787d1f0f..367a2864 100644 --- a/test/unit/polygon.js +++ b/test/unit/polygon.js @@ -37,6 +37,14 @@ 'globalCompositeOperation': 'source-over' }; + var REFERENCE_EMPTY_OBJECT = { + 'points': [], + 'width': 0, + 'height': 0, + 'top': 0, + 'left': 0 + }; + QUnit.module('fabric.Polygon'); test('constructor', function() { @@ -77,6 +85,18 @@ test('fromElement', function() { ok(typeof fabric.Polygon.fromElement == 'function'); + var empty_object = fabric.util.object.extend({}, REFERENCE_OBJECT); + empty_object = fabric.util.object.extend(empty_object, REFERENCE_EMPTY_OBJECT); + + var elPolygonWithoutPoints = fabric.document.createElement('polygon'); + + deepEqual(fabric.Polygon.fromElement(elPolygonWithoutPoints).toObject(), empty_object); + + var elPolygonWithEmptyPoints = fabric.document.createElement('polygon'); + elPolygonWithEmptyPoints.setAttribute('points', ''); + + deepEqual(fabric.Polygon.fromElement(elPolygonWithEmptyPoints).toObject(), empty_object); + var elPolygon = fabric.document.createElement('polygon'); elPolygon.setAttribute('points', '10,12 20,22'); @@ -130,15 +150,6 @@ deepEqual(polygonWithAttrs.get('transformMatrix'), [ 2, 0, 0, 2, -10, -20 ]); - var elPolygonWithoutPoints = fabric.document.createElement('polygon'); - - equal(fabric.Polygon.fromElement(elPolygonWithoutPoints), null); - - var elPolygonWithEmptyPoints = fabric.document.createElement('polygon'); - elPolygonWithEmptyPoints.setAttribute('points', ''); - - equal(fabric.Polygon.fromElement(elPolygonWithEmptyPoints), null); - equal(fabric.Polygon.fromElement(), null); }); })(); diff --git a/test/unit/polyline.js b/test/unit/polyline.js index 7ba51daf..4726d695 100644 --- a/test/unit/polyline.js +++ b/test/unit/polyline.js @@ -37,6 +37,14 @@ 'globalCompositeOperation': 'source-over' }; + var REFERENCE_EMPTY_OBJECT = { + 'points': [], + 'width': 0, + 'height': 0, + 'top': 0, + 'left': 0 + }; + QUnit.module('fabric.Polyline'); test('constructor', function() { @@ -76,6 +84,17 @@ test('fromElement', function() { ok(typeof fabric.Polyline.fromElement == 'function'); + var elPolylineWithoutPoints = fabric.document.createElement('polyline'); + var empty_object = fabric.util.object.extend({}, REFERENCE_OBJECT); + empty_object = fabric.util.object.extend(empty_object, REFERENCE_EMPTY_OBJECT); + + deepEqual(fabric.Polyline.fromElement(elPolylineWithoutPoints).toObject(), empty_object); + + var elPolylineWithEmptyPoints = fabric.document.createElement('polyline'); + elPolylineWithEmptyPoints.setAttribute('points', ''); + + deepEqual(fabric.Polyline.fromElement(elPolylineWithEmptyPoints).toObject(), empty_object); + var elPolyline = fabric.document.createElement('polyline'); elPolyline.setAttribute('points', '10,12 20,22'); @@ -119,14 +138,6 @@ deepEqual(polylineWithAttrs.get('transformMatrix'), [ 2, 0, 0, 2, -10, -20 ]); - var elPolylineWithoutPoints = fabric.document.createElement('polyline'); - equal(fabric.Polyline.fromElement(elPolylineWithoutPoints), null); - - var elPolylineWithEmptyPoints = fabric.document.createElement('polyline'); - elPolylineWithEmptyPoints.setAttribute('points', ''); - - equal(fabric.Polyline.fromElement(elPolylineWithEmptyPoints), null); - equal(fabric.Polyline.fromElement(), null); }); })(); diff --git a/test/unit/rect.js b/test/unit/rect.js index 67b76cb0..7e1f5bcc 100644 --- a/test/unit/rect.js +++ b/test/unit/rect.js @@ -71,9 +71,10 @@ var elRect = fabric.document.createElement('rect'); var rect = fabric.Rect.fromElement(elRect); - + var expectedObject = fabric.util.object.extend({ }, REFERENCE_RECT); + expectedObject.visible = false; ok(rect instanceof fabric.Rect); - deepEqual(rect.toObject(), REFERENCE_RECT); + deepEqual(rect.toObject(), expectedObject); }); test('fabric.Rect.fromElement with custom attributes', function() { From 1fb7a1b914e1d6cad02c063fbe6071cad2a0b7e3 Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Tue, 3 Feb 2015 14:28:23 +0100 Subject: [PATCH 2/3] Update polygon.class.js --- src/shapes/polygon.class.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shapes/polygon.class.js b/src/shapes/polygon.class.js index 50f85a43..7765fe0d 100644 --- a/src/shapes/polygon.class.js +++ b/src/shapes/polygon.class.js @@ -141,7 +141,7 @@ * @param {CanvasRenderingContext2D} ctx Context to render on */ _render: function(ctx) { - if(!this.commonRender(ctx)) { + if (!this.commonRender(ctx)) { return; } this._renderFill(ctx); From 69880b5104e33dce5d0fe184c7fc51100ef93b5c Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Tue, 3 Feb 2015 14:28:37 +0100 Subject: [PATCH 3/3] Update polyline.class.js --- src/shapes/polyline.class.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shapes/polyline.class.js b/src/shapes/polyline.class.js index 84dd5c80..c0bf798c 100644 --- a/src/shapes/polyline.class.js +++ b/src/shapes/polyline.class.js @@ -108,7 +108,7 @@ * @param {CanvasRenderingContext2D} ctx Context to render on */ _render: function(ctx) { - if(!fabric.Polygon.prototype.commonRender.call(this, ctx)) { + if (!fabric.Polygon.prototype.commonRender.call(this, ctx)) { return; } this._renderFill(ctx);