Merge pull request #1958 from asturur/Fix-svg-tiny-test-suite

Fixes svg rendering rules

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_ */
 

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_ */

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_ */

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_ */
 

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);
   });
 })();

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);    
   });
 })();

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() {