Enforce that either stroke or fill is provided

src/ol/renderer/canvas/canvasrenderer.js

@@ -244,7 +244,8 @@ ol.renderer.canvas.Renderer.prototype.renderPolygonFeatures_ =
  * @private
  */
 ol.renderer.canvas.Renderer.renderCircle_ = function(circle) {
-  var size = circle.size + (2 * circle.strokeWidth) + 1,
+  var strokeWidth = circle.strokeWidth || 0,
+      size = circle.size + (2 * strokeWidth) + 1,
       mid = size / 2,
       canvas = /** @type {HTMLCanvasElement} */
           (goog.dom.createElement(goog.dom.TagName.CANVAS)),
@@ -260,11 +261,11 @@ ol.renderer.canvas.Renderer.renderCircle_ = function(circle) {
   context.globalAlpha = circle.opacity;
 
   if (fillStyle) {
-    context.fillStyle = circle.fillStyle;
+    context.fillStyle = fillStyle;
   }
   if (strokeStyle) {
-    context.lineWidth = circle.strokeWidth;
+    context.lineWidth = strokeWidth;
-    context.strokeStyle = circle.strokeStyle;
+    context.strokeStyle = strokeStyle;
   }
 
   context.beginPath();

src/ol/style/shape.js

@@ -19,9 +19,9 @@ ol.style.ShapeType = {
 /**
  * @typedef {{type: (ol.style.ShapeType),
  *            size: (number),
- *            fillStyle: (string),
+ *            fillStyle: (string|undefined),
- *            strokeStyle: (string),
+ *            strokeStyle: (string|undefined),
- *            strokeWidth: (number),
+ *            strokeWidth: (number|undefined),
  *            opacity: (number)}}
  */
 ol.style.ShapeLiteralOptions;
@@ -43,17 +43,30 @@ ol.style.ShapeLiteral = function(config) {
   /** @type {number} */
   this.size = config.size;
 
-  goog.asserts.assertString(config.fillStyle, 'fillStyle must be a string');
+  if (goog.isDef(config.fillStyle)) {
-  /** @type {string} */
+    goog.asserts.assertString(config.fillStyle, 'fillStyle must be a string');
+  }
+  /** @type {string|undefined} */
   this.fillStyle = config.fillStyle;
 
-  goog.asserts.assertString(config.strokeStyle, 'strokeStyle must be a string');
+  /** @type {string|undefined} */
-  /** @type {string} */
   this.strokeStyle = config.strokeStyle;
+  if (goog.isDef(this.strokeStyle)) {
+    goog.asserts.assertString(
+        this.strokeStyle, 'strokeStyle must be a string');
+  }
 
-  goog.asserts.assertNumber(config.strokeWidth, 'strokeWidth must be a number');
+  /** @type {number|undefined} */
-  /** @type {number} */
   this.strokeWidth = config.strokeWidth;
+  if (goog.isDef(this.strokeWidth)) {
+    goog.asserts.assertNumber(
+        this.strokeWidth, 'strokeWidth must be a number');
+  }
+
+  goog.asserts.assert(
+      goog.isDef(this.fillStyle) ||
+      (goog.isDef(this.strokeStyle) && goog.isDef(this.strokeWidth)),
+      'Either fillStyle or strokeStyle and strokeWidth must be set');
 
   goog.asserts.assertNumber(config.opacity, 'opacity must be a number');
   /** @type {number} */
@@ -88,8 +101,8 @@ ol.style.Shape = function(options) {
    * @type {ol.style.ShapeType}
    * @private
    */
-  this.type_ = /** @type {ol.style.ShapeType} */ goog.isDef(options.type) ?
+  this.type_ = /** @type {ol.style.ShapeType} */ (goog.isDef(options.type) ?
-      options.type : ol.style.ShapeDefaults.type;
+      options.type : ol.style.ShapeDefaults.type);
 
   /**
    * @type {ol.Expression}
@@ -104,28 +117,45 @@ ol.style.Shape = function(options) {
    * @type {ol.Expression}
    * @private
    */
-  this.fillStyle_ = !goog.isDef(options.fillStyle) ?
+  this.fillStyle_ = !goog.isDefAndNotNull(options.fillStyle) ?
-      new ol.ExpressionLiteral(ol.style.ShapeDefaults.fillStyle) :
+      null :
       (options.fillStyle instanceof ol.Expression) ?
           options.fillStyle : new ol.ExpressionLiteral(options.fillStyle);
 
-  /**
+  // stroke handling - if any stroke property is supplied, use defaults
-   * @type {ol.Expression}
+  var strokeStyle = null,
-   * @private
+      strokeWidth = null;
-   */
+
-  this.strokeStyle_ = !goog.isDef(options.strokeStyle) ?
+  if (goog.isDefAndNotNull(options.strokeStyle) ||
-      new ol.ExpressionLiteral(ol.style.ShapeDefaults.strokeStyle) :
+      goog.isDefAndNotNull(options.strokeWidth)) {
-      (options.strokeStyle instanceof ol.Expression) ?
+
-          options.strokeStyle : new ol.ExpressionLiteral(options.strokeStyle);
+    strokeStyle = !goog.isDefAndNotNull(options.strokeStyle) ?
+        new ol.ExpressionLiteral(ol.style.ShapeDefaults.strokeStyle) :
+        (options.strokeStyle instanceof ol.Expression) ?
+            options.strokeStyle : new ol.ExpressionLiteral(options.strokeStyle);
+
+    strokeWidth = !goog.isDef(options.strokeWidth) ?
+        new ol.ExpressionLiteral(ol.style.ShapeDefaults.strokeWidth) :
+        (options.strokeWidth instanceof ol.Expression) ?
+            options.strokeWidth : new ol.ExpressionLiteral(options.strokeWidth);
+  }
 
   /**
    * @type {ol.Expression}
    * @private
    */
-  this.strokeWidth_ = !goog.isDef(options.strokeWidth) ?
+  this.strokeStyle_ = strokeStyle;
-      new ol.ExpressionLiteral(ol.style.ShapeDefaults.strokeWidth) :
+
-      (options.strokeWidth instanceof ol.Expression) ?
+  /**
-          options.strokeWidth : new ol.ExpressionLiteral(options.strokeWidth);
+   * @type {ol.Expression}
+   * @private
+   */
+  this.strokeWidth_ = strokeWidth;
+
+  // one of stroke or fill can be null, both null is user error
+  goog.asserts.assert(!goog.isNull(this.fillStyle_) ||
+      !(goog.isNull(this.strokeStyle_) && goog.isNull(this.strokeWidth_)),
+      'Stroke or fill properties must be provided');
 
   /**
    * @type {ol.Expression}
@@ -153,14 +183,22 @@ ol.style.Shape.prototype.createLiteral = function(opt_feature) {
   var size = this.size_.evaluate(feature, attrs);
   goog.asserts.assertNumber(size, 'size must be a number');
 
-  var fillStyle = this.fillStyle_.evaluate(feature, attrs);
+  var fillStyle = goog.isNull(this.fillStyle_) ?
-  goog.asserts.assertString(fillStyle, 'fillStyle must be a string');
+      undefined : this.fillStyle_.evaluate(feature, attrs);
+  goog.asserts.assert(!goog.isDef(fillStyle) || goog.isString(fillStyle));
 
-  var strokeStyle = this.strokeStyle_.evaluate(feature, attrs);
+  var strokeStyle = goog.isNull(this.strokeStyle_) ?
-  goog.asserts.assertString(strokeStyle, 'strokeStyle must be a string');
+      undefined : this.strokeStyle_.evaluate(feature, attrs);
+  goog.asserts.assert(!goog.isDef(strokeStyle) || goog.isString(strokeStyle));
 
-  var strokeWidth = this.strokeWidth_.evaluate(feature, attrs);
+  var strokeWidth = goog.isNull(this.strokeWidth_) ?
-  goog.asserts.assertNumber(strokeWidth, 'strokeWidth must be a number');
+      undefined : this.strokeWidth_.evaluate(feature, attrs);
+  goog.asserts.assert(!goog.isDef(strokeWidth) || goog.isNumber(strokeWidth));
+
+  goog.asserts.assert(
+      goog.isDef(fillStyle) ||
+      (goog.isDef(strokeStyle) && goog.isDef(strokeWidth)),
+      'either fill style or strokeStyle and strokeWidth must be defined');
 
   var opacity = this.opacity_.evaluate(feature, attrs);
   goog.asserts.assertNumber(opacity, 'opacity must be a number');
@@ -168,9 +206,10 @@ ol.style.Shape.prototype.createLiteral = function(opt_feature) {
   return new ol.style.ShapeLiteral({
     type: this.type_,
     size: size,
-    fillStyle: fillStyle,
+    // TODO: check if typecast can be avoided here
-    strokeStyle: strokeStyle,
+    fillStyle: /** @type {string|undefined} */ (fillStyle),
-    strokeWidth: strokeWidth,
+    strokeStyle: /** @type {string|undefined} */ (strokeStyle),
+    strokeWidth: /** @type {number|undefined} */ (strokeWidth),
     opacity: opacity
   });
 };

test/spec/ol/style/shape.test.js

@@ -52,7 +52,7 @@ describe('ol.style.Shape', function() {
     it('accepts expressions', function() {
       var symbolizer = new ol.style.Shape({
         size: new ol.Expression('sizeAttr'),
-        strokeStyle: ol.Expression('color')
+        strokeStyle: new ol.Expression('color')
       });
       expect(symbolizer).toBeA(ol.style.Shape);
     });
@@ -64,7 +64,44 @@ describe('ol.style.Shape', function() {
     it('evaluates expressions with the given feature', function() {
       var symbolizer = new ol.style.Shape({
         size: new ol.Expression('sizeAttr'),
-        opacity: new ol.Expression('opacityAttr')
+        opacity: new ol.Expression('opacityAttr'),
+        fillStyle: '#BADA55'
+      });
+
+      var feature = new ol.Feature({
+        sizeAttr: 42,
+        opacityAttr: 0.4
+      });
+
+      var literal = symbolizer.createLiteral(feature);
+      expect(literal).toBeA(ol.style.ShapeLiteral);
+      expect(literal.size).toBe(42);
+      expect(literal.opacity).toBe(0.4);
+    });
+
+    it('can be called without a feature', function() {
+      var symbolizer = new ol.style.Shape({
+        size: 10,
+        opacity: 1,
+        fillStyle: '#BADA55',
+        strokeStyle: '#013',
+        strokeWidth: 2
+      });
+
+      var literal = symbolizer.createLiteral();
+      expect(literal).toBeA(ol.style.ShapeLiteral);
+      expect(literal.size).toBe(10);
+      expect(literal.opacity).toBe(1);
+      expect(literal.fillStyle).toBe('#BADA55');
+      expect(literal.strokeStyle).toBe('#013');
+      expect(literal.strokeWidth).toBe(2);
+    });
+
+    it('applies default type if none provided', function() {
+      var symbolizer = new ol.style.Shape({
+        size: new ol.Expression('sizeAttr'),
+        opacity: new ol.Expression('opacityAttr'),
+        fillStyle: '#BADA55'
       });
 
       var feature = new ol.Feature({