From ca981e383d7f1a9fafa2c76d2729f7acb192f83f Mon Sep 17 00:00:00 2001 From: nebulon42 Date: Wed, 5 Jul 2017 14:14:29 +0200 Subject: [PATCH] do not output a symbolizer when a symbolizer rule with 'none' is present, output empty symbolizer when a symbolizer rule with 'auto' is present, fixes #462, #18 --- CHANGELOG.md | 3 +++ docs/language_elements.rst | 39 ++++++++++++++++++++++++++--- lib/carto/tree/definition.js | 33 +++++++++++++++++++++--- lib/carto/tree/reference.js | 12 +++++++++ lib/carto/tree/ruleset.js | 2 +- package.json | 2 +- test/errorhandling/issue_462.mss | 6 +++++ test/errorhandling/issue_462.result | 8 ++++++ test/rendering-mss/issue_462.mss | 10 ++++++++ test/rendering-mss/issue_462.xml | 10 ++++++++ test/rendering-mss/issue_462a.mss | 9 +++++++ test/rendering-mss/issue_462a.xml | 13 ++++++++++ test/rendering-mss/issue_462b.mss | 15 +++++++++++ test/rendering-mss/issue_462b.xml | 10 ++++++++ 14 files changed, 162 insertions(+), 10 deletions(-) create mode 100644 test/errorhandling/issue_462.mss create mode 100644 test/errorhandling/issue_462.result create mode 100644 test/rendering-mss/issue_462.mss create mode 100644 test/rendering-mss/issue_462.xml create mode 100644 test/rendering-mss/issue_462a.mss create mode 100644 test/rendering-mss/issue_462a.xml create mode 100644 test/rendering-mss/issue_462b.mss create mode 100644 test/rendering-mss/issue_462b.xml diff --git a/CHANGELOG.md b/CHANGELOG.md index cdc259aa9..21b69dc02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ For Mapnik XML all character data as tag content is now prefixed with CDATA. ([#377](https://github.com/mapbox/carto/issues/377)) * carto now accepts custom references for validating rules (part of [#413](https://github.com/mapbox/carto/issues/413)) * The JavaScript API has been documented ([#479](https://github.com/mapbox/carto/issues/479)) +* New symbolizer rules (work on the whole symbolizer) enable control of symbolizer serialization. Write e.g. `line: none` to suppress output +of the line symbolizer for this definition. Write e.g. `marker: auto` to output a markers symbolizer with default values. Symbolizer rules +are considered an advanced features and are never inherited to other definitions ([#477](https://github.com/mapbox/carto/pull/477)). ### Breaking changes diff --git a/docs/language_elements.rst b/docs/language_elements.rst index b486e624b..d8943169c 100644 --- a/docs/language_elements.rst +++ b/docs/language_elements.rst @@ -166,9 +166,40 @@ Mapnik >= 3.0.0 supports variables of the form ``@var``. These can be used from For this to have any effect you have to pass the variables to Mapnik at render time in a hashmap of the form ``variable_name:variable_value``. -Resetting properties -==================== +Controlling output of symbolizers and symbolizer attributes +=========================================================== -You can reset properties by specifying them with the keyword ``none``. They will then be removed from the symbolizer thus using their default value -or not using them at all. This does not work or makes sense for all properties like e.g. not for ``text-face-name``. For an overview over properties +You can control symbolizer output by using rules that work on the whole symbolizer. E.g. ``line`` works on the +line symbolizer. By using the keywords ``none`` or ``auto`` you can either suppress the symbolizer or output it with +default values. The keyword ``auto`` does not work on shield and text symbolizers because they have attributes without +default values. Here is an example how this works:: + + #layer { + line: none; + line-width: 2; + [feature = 'redfeature'] { + line-color: red; + } + [feature = 'bluefeature'] { + line-color: blue; + } + } + +Without ``line: none`` carto would output a line symbolizer with default values for all features +other than redfeature and bluefeature, that is a black line with width 1. In contrast, you can +quickly output a symbolizer with default value by using ``auto``:: + + #layer { + [feature = 'quickfeature'] { + marker: auto; + } + } + +This outputs a default markers symbolizer for all quickfeature features. + +You can also control the output of individual symbolizer properties by specifying them with the keyword ``none`` e.g. ``line-color: none``. +They will then be removed from the symbolizer thus using their default value +or not using them at all. This does not work or makes sense for all properties like e.g. not for ``text-face-name`` as it does not have a default value. For an overview over properties where this works or makes sense see `this list `_. +In this case the use of ``none`` and ``auto`` is equivalent. In both cases the default +value will be used as Mapnik uses the default value automatically when the property is not present. diff --git a/lib/carto/tree/definition.js b/lib/carto/tree/definition.js index cbc65f05b..a845baeef 100644 --- a/lib/carto/tree/definition.js +++ b/lib/carto/tree/definition.js @@ -9,7 +9,7 @@ var assert = require('assert'), // } // // The selector can have filters -tree.Definition = function Definition(selector, rules) { +tree.Definition = function Definition(env, selector, rules) { this.elements = selector.elements; assert.ok(selector.filters instanceof tree.Filterset); this.rules = rules; @@ -24,6 +24,7 @@ tree.Definition = function Definition(selector, rules) { this.attachment = selector.attachment || '__default__'; this.specificity = selector.specificity(); this.matchCount = 0; + this.ref = env.ref; // special handling for Map selector if (_.isArray(this.elements) && this.elements.length > 0 && @@ -47,15 +48,18 @@ tree.Definition.prototype.clone = function(filters) { clone.ruleIndex = _.clone(this.ruleIndex); clone.filters = filters ? filters : this.filters.clone(); clone.attachment = this.attachment; + clone.ref = this.ref; return clone; }; tree.Definition.prototype.addRules = function(rules) { var added = 0; - // Add only unique rules. for (var i = 0; i < rules.length; i++) { - if (!this.ruleIndex[rules[i].id]) { + // only add rule if not for whole symbolizer (default) + // and if unique + if (this.ref.selectorName(rules[i].name) !== 'default' && + !this.ruleIndex[rules[i].id]) { this.rules.push(rules[i]); this.ruleIndex[rules[i].id] = true; added++; @@ -118,6 +122,7 @@ tree.Definition.prototype.symbolizersToObject = function(env, symbolizers, zoom) var sym_count = 0; for (var i = 0; i < sym_order.length; i++) { + var doNotSerialize = false; var attributes = symbolizers[sym_order[i]]; var symbolizer = sym_order[i].split('/').pop(); @@ -159,8 +164,24 @@ tree.Definition.prototype.symbolizersToObject = function(env, symbolizers, zoom) tagcontent = attributes[j].ev(env).toObject(env, true); } else { var attr = attributes[j].ev(env); + // if we have a rule for the whole symbolizer (default) with keyword none + // don't output the rule + if (env.ref.selectorName(attr.name) === 'default' && + attr.value.value[0].is === 'keyword') { + if (attr.value.value[0].value === 'none') { + doNotSerialize = true; + // still call toObject for validation + attr.toObject(env); + } + else if (attr.value.value[0].value === 'auto') { + // still call toObject for validation + attr.toObject(env); + } + } - if (!(attr.value.value[0].is === 'keyword' && attr.value.value[0].value === 'none')) { + // only output attributes that don't use the keywords none and auto + if (!(attr.value.value[0].is === 'keyword' && + (attr.value.value[0].value === 'none' || attr.value.value[0].value === 'auto'))) { symbolizerAttr.push(attr.toObject(env)); } else { @@ -174,6 +195,10 @@ tree.Definition.prototype.symbolizersToObject = function(env, symbolizers, zoom) } }); + if (doNotSerialize) { + continue; + } + if (symbolizerAttr.length) { var attrObj = {}; diff --git a/lib/carto/tree/reference.js b/lib/carto/tree/reference.js index ac43e3ad6..2fa03d19f 100644 --- a/lib/carto/tree/reference.js +++ b/lib/carto/tree/reference.js @@ -161,6 +161,18 @@ function generateRequiredProperties(data) { } tree.Reference.prototype.requiredProperties = function (symbolizer_name, rules) { + var that = this, + doNotSerialize = false; + + _.forEach(rules, function (rule) { + if (that.selectorName(rule.name) === 'default') { + doNotSerialize = true; + } + }); + if (doNotSerialize) { + return null; + } + var req = this.required_cache[symbolizer_name]; for (var i in req) { if (!(req[i] in rules)) { diff --git a/lib/carto/tree/ruleset.js b/lib/carto/tree/ruleset.js index 33f4f4316..654ebab9a 100644 --- a/lib/carto/tree/ruleset.js +++ b/lib/carto/tree/ruleset.js @@ -169,7 +169,7 @@ tree.Ruleset.prototype = { if (index !== false) { selectors[i].index = index; } - result.push(new tree.Definition(selectors[i], rules.slice())); + result.push(new tree.Definition(env, selectors[i], rules.slice())); } return result; diff --git a/package.json b/package.json index a1361c9a3..ac1415a19 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "hsluv": "~0.0.1", "js-yaml": "~3.8.4", "lodash": "~4.17.2", - "mapnik-reference": "~8.7.0", + "mapnik-reference": "~8.8.1", "semver": "~5.3.0", "yargs": "~8.0.1" }, diff --git a/test/errorhandling/issue_462.mss b/test/errorhandling/issue_462.mss new file mode 100644 index 000000000..36fe49b86 --- /dev/null +++ b/test/errorhandling/issue_462.mss @@ -0,0 +1,6 @@ +#test { + shield: auto; + text: auto; + line-pattern: auto; + polygon-pattern: auto; +} diff --git a/test/errorhandling/issue_462.result b/test/errorhandling/issue_462.result new file mode 100644 index 000000000..511a6c0e2 --- /dev/null +++ b/test/errorhandling/issue_462.result @@ -0,0 +1,8 @@ +Warning: issue_462.mss:2:4 shield is unstable. It may change in the future. +Error: issue_462.mss:2:4 Invalid value for shield, the type keyword (options: none) is expected. auto (of type keyword) was given. +Warning: issue_462.mss:3:4 text is unstable. It may change in the future. +Error: issue_462.mss:3:4 Invalid value for text, the type keyword (options: none) is expected. auto (of type keyword) was given. +Warning: issue_462.mss:4:4 line-pattern is unstable. It may change in the future. +Error: issue_462.mss:4:4 Invalid value for line-pattern, the type keyword (options: none) is expected. auto (of type keyword) was given. +Warning: issue_462.mss:5:4 polygon-pattern is unstable. It may change in the future. +Error: issue_462.mss:5:4 Invalid value for polygon-pattern, the type keyword (options: none) is expected. auto (of type keyword) was given. \ No newline at end of file diff --git a/test/rendering-mss/issue_462.mss b/test/rendering-mss/issue_462.mss new file mode 100644 index 000000000..3f505572e --- /dev/null +++ b/test/rendering-mss/issue_462.mss @@ -0,0 +1,10 @@ +#foo { + line: none; + line-width: 2; + [feature = 'bar'] { + line-color: red; + } + [feature = 'baz'] { + line-color: blue; + } +} diff --git a/test/rendering-mss/issue_462.xml b/test/rendering-mss/issue_462.xml new file mode 100644 index 000000000..a57e2115c --- /dev/null +++ b/test/rendering-mss/issue_462.xml @@ -0,0 +1,10 @@ + diff --git a/test/rendering-mss/issue_462a.mss b/test/rendering-mss/issue_462a.mss new file mode 100644 index 000000000..d7c4bc30e --- /dev/null +++ b/test/rendering-mss/issue_462a.mss @@ -0,0 +1,9 @@ +#foo { + line: auto; + [feature = 'bar'] { + line-color: red; + } + [feature = 'baz'] { + line-color: blue; + } +} diff --git a/test/rendering-mss/issue_462a.xml b/test/rendering-mss/issue_462a.xml new file mode 100644 index 000000000..65b321c3a --- /dev/null +++ b/test/rendering-mss/issue_462a.xml @@ -0,0 +1,13 @@ + diff --git a/test/rendering-mss/issue_462b.mss b/test/rendering-mss/issue_462b.mss new file mode 100644 index 000000000..cb8065d57 --- /dev/null +++ b/test/rendering-mss/issue_462b.mss @@ -0,0 +1,15 @@ +#foo { + shield: none; + [feature = 'bar'] { + shield-name: "[refs]"; + shield-face-name: "Arial"; + shield-file: url('test.svg'); + shield-fill: red; + } + [feature = 'baz'] { + shield-name: "[refs]"; + shield-face-name: "Arial"; + shield-file: url('test.svg'); + shield-fill: blue; + } +} diff --git a/test/rendering-mss/issue_462b.xml b/test/rendering-mss/issue_462b.xml new file mode 100644 index 000000000..dc8da4b94 --- /dev/null +++ b/test/rendering-mss/issue_462b.xml @@ -0,0 +1,10 @@ +