From 511f8ecc828b0cb43f4e4d114331808c206ab274 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 18 May 2019 15:44:29 -0400 Subject: [PATCH 1/3] Allow user to adjust the threshold for the unsquare building warning --- css/80_app.css | 8 ++++ modules/core/validator.js | 55 ++++++++++++++++++++++- modules/ui/issues.js | 70 ++++++++++++++++++++++++++--- modules/validations/unsquare_way.js | 14 ++++-- test/spec/core/validator.js | 2 +- 5 files changed, 136 insertions(+), 13 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 75209416c..d76c9a5e0 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -3397,6 +3397,14 @@ button.autofix.action.active { color: #05ac10; } +input.square-degrees-input { + padding: 2px; + height: unset; + text-align: center; + background: rgba(0,0,0,0); + color: currentColor; +} + /* Entity Issues List */ .entity-issues .issue-container .issue { diff --git a/modules/core/validator.js b/modules/core/validator.js index cfffcd680..376f7d5e2 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -1,6 +1,7 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; import { coreDifference } from './difference'; +import { geoExtent } from '../geo/extent'; import { modeSelect } from '../modes/select'; import { utilArrayGroupBy, utilRebind } from '../util'; import { t } from '../util/locale'; @@ -69,6 +70,56 @@ export function coreValidator(context) { dispatch.call('validated'); }; + + // when the user changes the squaring thereshold, rerun this on all buildings + validator.changeSquareThreshold = function() { + var checkUnsquareWay = _rules.unsquare_way; + if (typeof checkUnsquareWay !== 'function') return; + + // uncache existing + Object.values(_issuesByIssueID) + .filter(function(issue) { return issue.type === 'unsquare_way'; }) + .forEach(function(issue) { + var entityId = issue.entityIds[0]; // always 1 entity for unsquare way + if (_issuesByEntityID[entityId]) { + _issuesByEntityID[entityId].delete(issue.id); + } + delete _issuesByIssueID[issue.id]; + }); + + var buildings = context.intersects(geoExtent([-180,-90],[180, 90])) // everywhere + .filter(function(entity) { + return entity.type === 'way' && entity.tags.building && entity.tags.building !== 'no'; + }); + + // rerun for all buildings + buildings.forEach(function(entity) { + var detected = checkUnsquareWay(entity, context); + if (detected.length !== 1) return; + + var issue = detected[0]; + var ignoreFix = new validationIssueFix({ + title: t('issues.fix.ignore_issue.title'), + icon: 'iD-icon-close', + onClick: function() { + ignoreIssue(this.issue.id); + } + }); + ignoreFix.type = 'ignore'; + ignoreFix.issue = issue; + issue.fixes.push(ignoreFix); + + if (!_issuesByEntityID[entity.id]) { + _issuesByEntityID[entity.id] = new Set(); + } + _issuesByEntityID[entity.id].add(issue.id); + _issuesByIssueID[issue.id] = issue; + }); + + dispatch.call('validated'); + }; + + // options = { // what: 'all', // 'all' or 'edited' // where: 'all', // 'all' or 'visible' @@ -89,13 +140,13 @@ export function coreValidator(context) { if (!opts.includeIgnored && _ignoredIssueIDs[issue.id]) return false; // Sanity check: This issue may be for an entity that not longer exists. - // If we detect this, uncache and return false so it is not incluced.. + // If we detect this, uncache and return false so it is not included.. var entityIds = issue.entityIds || []; for (var i = 0; i < entityIds.length; i++) { var entityId = entityIds[i]; if (!context.hasEntity(entityId)) { delete _issuesByEntityID[entityId]; - delete _issuesByIssueID[entityId]; + delete _issuesByIssueID[issue.id]; return false; } } diff --git a/modules/ui/issues.js b/modules/ui/issues.js index 65efe66c0..4cccfdc54 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -10,11 +10,16 @@ import { geoSphericalDistance } from '../geo'; import { svgIcon } from '../svg/icon'; import { uiDisclosure } from './disclosure'; import { uiTooltipHtml } from './tooltipHtml'; -import { utilHighlightEntities } from '../util'; +import { utilGetSetValue, utilHighlightEntities, utilNoAuto } from '../util'; export function uiIssues(context) { var key = t('issues.key'); + + var MINSQUARE = 0; + var MAXSQUARE = 20; + var DEFAULTSQUARE = 6.5; // see also unsquare_way.js + var _errorsSelection = d3_select(null); var _warningsSelection = d3_select(null); var _rulesList = d3_select(null); @@ -564,10 +569,10 @@ export function uiIssues(context) { label .append('span') - .text(function(d) { + .html(function(d) { var params = {}; if (d === 'unsquare_way') { - params.val = 6.5; + params.val = ''; } return t('issues.' + d + '.title', params); }); @@ -581,19 +586,70 @@ export function uiIssues(context) { .selectAll('input') .property('checked', active) .property('indeterminate', false); + + + // user-configurable square threshold + var degStr = context.storage('validate-square-degrees'); + if (degStr === null) { + degStr = '' + DEFAULTSQUARE; + } + + var span = items.selectAll('.square-degrees'); + var input = span.selectAll('.square-degrees-input') + .data([0]); + + // enter / update + input.enter() + .append('input') + .attr('type', 'number') + .attr('min', '' + MINSQUARE) + .attr('max', '' + MAXSQUARE) + .attr('step', '0.5') + .attr('class', 'square-degrees-input') + .call(utilNoAuto) + .on('input', function() { + this.style.width = (this.value.length + 1) + 'ch'; // resize + }) + .on('blur', changeSquare) + .merge(input) + .property('value', degStr) + .style('width', (degStr.length + 1) + 'ch'); // resize } + function changeSquare() { + var input = d3_select(this); + var degStr = utilGetSetValue(input).trim(); + var degNum = parseFloat(degStr, 10); + + if (!isFinite(degNum) || degNum > MAXSQUARE || degNum < MINSQUARE) { + degNum = DEFAULTSQUARE; + } + + degNum = Math.round(degNum * 10 ) / 10; // round to 1 decimal + degStr = '' + degNum; + + input + .property('value', degStr) + .style('width', (degStr.length + 1) + 'ch'); // resize + + context.storage('validate-square-degrees', degStr); + context.validator().changeSquareThreshold(degNum); + } + + + function hidePane() { + context.ui().togglePanes(); + } + + + var paneTooltip = tooltip() .placement((textDirection === 'rtl') ? 'right' : 'left') .html(true) .title(uiTooltipHtml(t('issues.title'), key)); - function hidePane() { - context.ui().togglePanes(); - } - uiIssues.togglePane = function() { if (d3_event) d3_event.preventDefault(); diff --git a/modules/validations/unsquare_way.js b/modules/validations/unsquare_way.js index c888df9fe..5ce332340 100644 --- a/modules/validations/unsquare_way.js +++ b/modules/validations/unsquare_way.js @@ -8,16 +8,17 @@ import { validationIssue, validationIssueFix } from '../core/validation'; export function validationUnsquareWay() { var type = 'unsquare_way'; + var DEFAULTSQUARE = 6.5; // see also issues.js // use looser epsilon for detection to reduce warnings of buildings that are essentially square already var epsilon = 0.05; - var degreeThreshold = 13; - var autofixDegreeThreshold = 6.5; var nodeThreshold = 10; + // var degreeThreshold = 13; + // var autofixDegreeThreshold = 6.5; + function isBuilding(entity, graph) { if (entity.type !== 'way' || entity.geometry(graph) !== 'area') return false; - return entity.tags.building && entity.tags.building !== 'no'; } @@ -55,6 +56,13 @@ export function validationUnsquareWay() { if (hasConnectedSquarableWays) return []; +// testing: user-configurable square threshold +var degreeThreshold = context.storage('validate-square-degrees'); +if (degreeThreshold === null) { + degreeThreshold = '' + DEFAULTSQUARE; +} +var autofixDegreeThreshold = degreeThreshold; + var points = nodes.map(function(node) { return context.projection(node.loc); }); if (!geoOrthoCanOrthogonalize(points, isClosed, epsilon, degreeThreshold, true)) return []; diff --git a/test/spec/core/validator.js b/test/spec/core/validator.js index 2e40b04d1..36cfb814a 100644 --- a/test/spec/core/validator.js +++ b/test/spec/core/validator.js @@ -1,4 +1,4 @@ -describe('iD.validations.validator', function () { +describe('iD.coreValidator', function () { var context; beforeEach(function() { From d7d46d93654115642c2075a7679308a2868f9886 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Mon, 20 May 2019 10:30:18 -0400 Subject: [PATCH 2/3] Add green fill for landuse=recreation_ground --- css/25_areas.css | 2 ++ 1 file changed, 2 insertions(+) diff --git a/css/25_areas.css b/css/25_areas.css index 6cd355679..72f6fbfe4 100644 --- a/css/25_areas.css +++ b/css/25_areas.css @@ -30,6 +30,7 @@ path.stroke.tag-leisure-track, path.stroke.tag-leisure-golf_course, path.stroke.tag-leisure-garden, path.stroke.tag-leisure-park, +path.stroke.tag-landuse-recreation_ground, path.stroke.tag-landuse-forest, path.stroke.tag-landuse-wood, path.stroke.tag-landuse-grass, @@ -43,6 +44,7 @@ path.fill.tag-leisure-track, path.fill.tag-leisure-golf_course, path.fill.tag-leisure-garden, path.fill.tag-leisure-park, +path.fill.tag-landuse-recreation_ground, path.fill.tag-landuse-forest, path.fill.tag-natural-wood, path.fill.tag-landuse-grass, From 460f136819ad6f2dfc1dc242d6a51ee8935b69ab Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Mon, 20 May 2019 15:48:01 -0400 Subject: [PATCH 3/3] Lower default threshold for unsquare building detection to 5 degrees Use the detection threshold for the action when fixing unsquare buildings Don't disallow unsquare autofixing based on maximum angle Disallow unsquare autofixing for features with wikidata tags Don't toggle rule when selecting degree threshold field in Safari Apply the change when pressing enter in the degree threshold field Select the input when clicking the degree threshold field Use the min or max threshold instead of the default when an input is out of bounds --- modules/actions/orthogonalize.js | 2 +- modules/ui/issues.js | 19 ++++++++++++++++-- modules/validations/unsquare_way.js | 31 +++++++++++------------------ 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/modules/actions/orthogonalize.js b/modules/actions/orthogonalize.js index 3cd0042f3..3eb0f13ca 100644 --- a/modules/actions/orthogonalize.js +++ b/modules/actions/orthogonalize.js @@ -6,7 +6,7 @@ import { } from '../geo'; -export function actionOrthogonalize(wayID, projection, vertexID, ep, degThresh) { +export function actionOrthogonalize(wayID, projection, vertexID, degThresh, ep) { var epsilon = ep || 1e-4; var threshold = degThresh || 13; // degrees within right or straight to alter diff --git a/modules/ui/issues.js b/modules/ui/issues.js index 4cccfdc54..92100bac5 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -18,7 +18,7 @@ export function uiIssues(context) { var MINSQUARE = 0; var MAXSQUARE = 20; - var DEFAULTSQUARE = 6.5; // see also unsquare_way.js + var DEFAULTSQUARE = 5; // see also unsquare_way.js var _errorsSelection = d3_select(null); var _warningsSelection = d3_select(null); @@ -610,6 +610,17 @@ export function uiIssues(context) { .on('input', function() { this.style.width = (this.value.length + 1) + 'ch'; // resize }) + .on('click', function () { + d3_event.preventDefault(); + d3_event.stopPropagation(); + this.select(); + }) + .on('keyup', function () { + if (d3_event.keyCode === 13) { // enter + this.blur(); + this.select(); + } + }) .on('blur', changeSquare) .merge(input) .property('value', degStr) @@ -622,8 +633,12 @@ export function uiIssues(context) { var degStr = utilGetSetValue(input).trim(); var degNum = parseFloat(degStr, 10); - if (!isFinite(degNum) || degNum > MAXSQUARE || degNum < MINSQUARE) { + if (!isFinite(degNum)) { degNum = DEFAULTSQUARE; + } else if (degNum > MAXSQUARE) { + degNum = MAXSQUARE; + } else if (degNum < MINSQUARE) { + degNum = MINSQUARE; } degNum = Math.round(degNum * 10 ) / 10; // round to 1 decimal diff --git a/modules/validations/unsquare_way.js b/modules/validations/unsquare_way.js index 5ce332340..8ecdebb05 100644 --- a/modules/validations/unsquare_way.js +++ b/modules/validations/unsquare_way.js @@ -1,21 +1,18 @@ import { t } from '../util/locale'; import { actionChangeTags } from '../actions/change_tags'; import { actionOrthogonalize } from '../actions/orthogonalize'; -import { geoOrthoCanOrthogonalize, geoOrthoMaxOffsetAngle } from '../geo/ortho'; +import { geoOrthoCanOrthogonalize } from '../geo/ortho'; import { utilDisplayLabel } from '../util'; import { validationIssue, validationIssueFix } from '../core/validation'; export function validationUnsquareWay() { var type = 'unsquare_way'; - var DEFAULTSQUARE = 6.5; // see also issues.js + var DEFAULT_DEG_THRESHOLD = 5; // see also issues.js // use looser epsilon for detection to reduce warnings of buildings that are essentially square already var epsilon = 0.05; var nodeThreshold = 10; - // var degreeThreshold = 13; - // var autofixDegreeThreshold = 6.5; - function isBuilding(entity, graph) { if (entity.type !== 'way' || entity.geometry(graph) !== 'area') return false; @@ -56,22 +53,18 @@ export function validationUnsquareWay() { if (hasConnectedSquarableWays) return []; -// testing: user-configurable square threshold -var degreeThreshold = context.storage('validate-square-degrees'); -if (degreeThreshold === null) { - degreeThreshold = '' + DEFAULTSQUARE; -} -var autofixDegreeThreshold = degreeThreshold; + // user-configurable square threshold + var storedDegreeThreshold = context.storage('validate-square-degrees'); + var degreeThreshold = isNaN(storedDegreeThreshold) ? DEFAULT_DEG_THRESHOLD : parseFloat(storedDegreeThreshold); var points = nodes.map(function(node) { return context.projection(node.loc); }); if (!geoOrthoCanOrthogonalize(points, isClosed, epsilon, degreeThreshold, true)) return []; var autoArgs; - // only allow autofixing features that are very close to square already - var maxOffsetAngle = geoOrthoMaxOffsetAngle(points, isClosed, degreeThreshold); - if (maxOffsetAngle && maxOffsetAngle < autofixDegreeThreshold) { - // note: use default params for actionOrthogonalize, not relaxed epsilon - var autoAction = actionOrthogonalize(entity.id, context.projection); + // don't allow autosquaring features linked to wikidata + if (!entity.tags.wikidata) { + // use same degree threshold as for detection + var autoAction = actionOrthogonalize(entity.id, context.projection, undefined, degreeThreshold); autoAction.transitionable = false; // when autofixing, do it instantly autoArgs = [autoAction, t('operations.orthogonalize.annotation.area')]; } @@ -85,7 +78,7 @@ var autofixDegreeThreshold = degreeThreshold; }, reference: showReference, entityIds: [entity.id], - hash: JSON.stringify(autoArgs !== undefined), + hash: JSON.stringify(autoArgs !== undefined) + degreeThreshold, fixes: [ new validationIssueFix({ icon: 'iD-operation-orthogonalize', @@ -93,9 +86,9 @@ var autofixDegreeThreshold = degreeThreshold; autoArgs: autoArgs, onClick: function(completionHandler) { var entityId = this.issue.entityIds[0]; - // note: use default params for actionOrthogonalize, not relaxed epsilon + // use same degree threshold as for detection context.perform( - actionOrthogonalize(entityId, context.projection), + actionOrthogonalize(entityId, context.projection, undefined, degreeThreshold), t('operations.orthogonalize.annotation.area') ); // run after the squaring transition (currently 150ms)