From 6b0819312523790514d7878468c1228d3a3ff64d Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 6 Dec 2012 14:11:45 -0500 Subject: [PATCH 1/6] Introduce Entity transients; fix #187 --- js/id/graph/entity.js | 43 +++++++++++++++++++++++++++++++++++++-- js/id/graph/graph.js | 37 ++------------------------------- test/spec/graph/entity.js | 38 +++++++++++++++++++++++++++++++++- 3 files changed, 80 insertions(+), 38 deletions(-) diff --git a/js/id/graph/entity.js b/js/id/graph/entity.js index b0019f18d..b3f698704 100644 --- a/js/id/graph/entity.js +++ b/js/id/graph/entity.js @@ -1,13 +1,23 @@ iD.Entity = function(a, b, c) { if (!(this instanceof iD.Entity)) return new iD.Entity(a, b, c); - _.extend(this, {tags: {}}, a, b, c); + this.tags = {}; + this.transients = {}; + + var sources = [a, b, c], source; + for (var i = 0; i < sources.length; ++i) { + source = sources[i]; + for (var prop in source) { + if (prop !== 'transients' && Object.prototype.hasOwnProperty.call(source, prop)) { + this[prop] = source[prop]; + } + } + } if (!this.id) { this.id = iD.util.id(this.type); this._updated = true; } - delete this._extent; if (iD.debug) { Object.freeze(this); @@ -31,6 +41,35 @@ iD.Entity.prototype = { return this._updated && +this.id.slice(1) > 0; }, + intersects: function(extent, resolver) { + if (this.type === 'node') { + return this.loc[0] > extent[0][0] && + this.loc[0] < extent[1][0] && + this.loc[1] < extent[0][1] && + this.loc[1] > extent[1][1]; + } else if (this.type === 'way') { + var _extent = this.transients.extent; + + if (!_extent) { + _extent = this.transients.extent = [[-Infinity, Infinity], [Infinity, -Infinity]]; + for (var i = 0, l = this.nodes.length; i < l; i++) { + var node = resolver.entity(this.nodes[i]); + if (node.loc[0] > _extent[0][0]) _extent[0][0] = node.loc[0]; + if (node.loc[0] < _extent[1][0]) _extent[1][0] = node.loc[0]; + if (node.loc[1] < _extent[0][1]) _extent[0][1] = node.loc[1]; + if (node.loc[1] > _extent[1][1]) _extent[1][1] = node.loc[1]; + } + } + + return _extent[0][0] > extent[0][0] && + _extent[1][0] < extent[1][0] && + _extent[0][1] < extent[0][1] && + _extent[1][1] > extent[1][1]; + } else { + return false; + } + }, + hasInterestingTags: function() { return _.keys(this.tags).some(function (key) { return key != "attribution" && diff --git a/js/id/graph/graph.js b/js/id/graph/graph.js index a97356ff1..a830214d4 100644 --- a/js/id/graph/graph.js +++ b/js/id/graph/graph.js @@ -55,46 +55,13 @@ iD.Graph.prototype = { return iD.Graph(entities, annotation); }, - nodeIntersect: function(entity, extent) { - return entity.loc[0] > extent[0][0] && - entity.loc[0] < extent[1][0] && - entity.loc[1] < extent[0][1] && - entity.loc[1] > extent[1][1]; - }, - - wayIntersect: function(entity, extent) { - return entity._extent[0][0] > extent[0][0] && - entity._extent[1][0] < extent[1][0] && - entity._extent[0][1] < extent[0][1] && - entity._extent[1][1] > extent[1][1]; - }, - - indexWay: function(way) { - if (way.type === 'way' && !way._extent) { - // top left, bottom right - var extent = [[-Infinity, Infinity], [Infinity, -Infinity]]; - var w = way; - for (var j = 0, l = w.nodes.length; j < l; j++) { - if (w.nodes[j].loc[0] > extent[0][0]) extent[0][0] = w.nodes[j].loc[0]; - if (w.nodes[j].loc[0] < extent[1][0]) extent[1][0] = w.nodes[j].loc[0]; - if (w.nodes[j].loc[1] < extent[0][1]) extent[0][1] = w.nodes[j].loc[1]; - if (w.nodes[j].loc[1] > extent[1][1]) extent[1][1] = w.nodes[j].loc[1]; - } - way._extent = extent; - } - return true; - }, - // get all objects that intersect an extent. intersects: function(extent) { var items = []; for (var i in this.entities) { var entity = this.entities[i]; - if (entity.type === 'node' && this.nodeIntersect(entity, extent)) { - items.push(entity); - } else if (entity.type === 'way') { - var w = this.fetch(entity.id); - if (this.indexWay(w) && this.wayIntersect(w, extent)) items.push(w); + if (entity.intersects(extent, this)) { + items.push(this.fetch(entity.id)); } } return items; diff --git a/test/spec/graph/entity.js b/test/spec/graph/entity.js index 71954b73a..0273ee4c6 100644 --- a/test/spec/graph/entity.js +++ b/test/spec/graph/entity.js @@ -23,7 +23,17 @@ describe('Entity', function () { var attrs = {tags: {foo: 'bar'}}, e = iD.Entity().update(attrs); expect(attrs).to.eql({tags: {foo: 'bar'}}); - }) + }); + + it("doesn't copy transients", function () { + var entity = iD.Entity(); + entity.transients['foo'] = 'bar'; + expect(entity.update({}).transients).not.to.have.property('foo'); + }); + + it("doesn't copy prototype properties", function () { + expect(iD.Entity().update({})).not.to.have.ownProperty('update'); + }); }); describe("#created", function () { @@ -102,6 +112,16 @@ describe('Node', function () { it("sets tags as specified", function () { expect(iD.Node({tags: {foo: 'bar'}}).tags).to.eql({foo: 'bar'}); }); + + describe("#intersects", function () { + it("returns true for a node within the given extent", function () { + expect(iD.Node({loc: [0, 0]}).intersects([[-180, 90], [180, -90]])).to.equal(true); + }); + + it("returns false for a node outside the given extend", function () { + expect(iD.Node({loc: [0, 0]}).intersects([[100, 90], [180, -90]])).to.equal(false); + }); + }); }); describe('Way', function () { @@ -133,6 +153,22 @@ describe('Way', function () { it("sets tags as specified", function () { expect(iD.Way({tags: {foo: 'bar'}}).tags).to.eql({foo: 'bar'}); }); + + describe("#intersects", function () { + it("returns true for a way with a node within the given extent", function () { + var node = iD.Node({loc: [0, 0]}), + way = iD.Way({nodes: [node.id]}), + graph = iD.Graph([node, way]); + expect(way.intersects([[-180, 90], [180, -90]], graph)).to.equal(true); + }); + + it("returns false for way with no nodes within the given extent", function () { + var node = iD.Node({loc: [0, 0]}), + way = iD.Way({nodes: [node.id]}), + graph = iD.Graph([node, way]); + expect(way.intersects([[100, 90], [180, -90]], graph)).to.equal(false); + }); + }); }); describe('Relation', function () { From 0e2889d5d6c4e1b2511b2b011e1854d61979d7ca Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 6 Dec 2012 14:20:02 -0500 Subject: [PATCH 2/6] Add freezing specs --- test/spec/graph/entity.js | 26 ++++++++++++++++++++++++++ test/spec/graph/graph.js | 10 ++++++++++ 2 files changed, 36 insertions(+) diff --git a/test/spec/graph/entity.js b/test/spec/graph/entity.js index 0273ee4c6..7b15a4788 100644 --- a/test/spec/graph/entity.js +++ b/test/spec/graph/entity.js @@ -1,4 +1,18 @@ describe('Entity', function () { + if (iD.debug) { + it("is frozen", function () { + expect(Object.isFrozen(iD.Entity())).to.be.true; + }); + + it("freezes tags", function () { + expect(Object.isFrozen(iD.Entity().tags)).to.be.true; + }); + + it("does not freeze transients", function () { + expect(Object.isFrozen(iD.Entity().transients)).to.be.false; + }); + } + describe("#update", function () { it("returns a new Entity", function () { var a = iD.Entity(), @@ -125,6 +139,12 @@ describe('Node', function () { }); describe('Way', function () { + if (iD.debug) { + it("freezes nodes", function () { + expect(Object.isFrozen(iD.Way().nodes)).to.be.true; + }); + } + it("returns a way", function () { expect(iD.Way().type).to.equal("way"); }); @@ -172,6 +192,12 @@ describe('Way', function () { }); describe('Relation', function () { + if (iD.debug) { + it("freezes nodes", function () { + expect(Object.isFrozen(iD.Relation().members)).to.be.true; + }); + } + it("returns a relation", function () { expect(iD.Relation().type).to.equal("relation"); }); diff --git a/test/spec/graph/graph.js b/test/spec/graph/graph.js index 31ef038b9..16cbf6995 100644 --- a/test/spec/graph/graph.js +++ b/test/spec/graph/graph.js @@ -16,6 +16,16 @@ describe('iD.Graph', function() { expect(graph.annotation).to.equal('first graph'); }); + if (iD.debug) { + it("is frozen", function () { + expect(Object.isFrozen(iD.Graph())).to.be.true; + }); + + it("freezes entities", function () { + expect(Object.isFrozen(iD.Graph().entities)).to.be.true; + }); + } + describe('operations', function() { it('#remove', function() { var entities = { 'n-1': { From 4ee6e3ac1a905c4cffb9474697732ec700a521bf Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 6 Dec 2012 14:22:33 -0500 Subject: [PATCH 3/6] Remove duplicate intersection function --- js/id/renderer/map.js | 2 +- js/id/util.js | 7 ------- test/spec/util.js | 15 +-------------- 3 files changed, 2 insertions(+), 22 deletions(-) diff --git a/js/id/renderer/map.js b/js/id/renderer/map.js index 3d3978b5d..bc6dfd940 100644 --- a/js/id/renderer/map.js +++ b/js/id/renderer/map.js @@ -134,7 +134,7 @@ iD.Map = function() { else ways.push(a); } else if (a._poi) { points.push(a); - } else if (!a._poi && a.type === 'node' && iD.util.geo.nodeIntersect(a, extent)) { + } else if (!a._poi && a.type === 'node' && a.intersects(extent)) { waynodes.push(a); } } diff --git a/js/id/util.js b/js/id/util.js index 7128673f0..ddc6fc070 100644 --- a/js/id/util.js +++ b/js/id/util.js @@ -90,13 +90,6 @@ iD.util.geo.dist = function(a, b) { Math.pow(a[1] - b[1], 2)); }; -iD.util.geo.nodeIntersect = function(entity, extent) { - return entity.loc[0] > extent[0][0] && - entity.loc[0] < extent[1][0] && - entity.loc[1] < extent[0][1] && - entity.loc[1] > extent[1][1]; -}; - iD.util.geo.chooseIndex = function(way, point, map) { var dist = iD.util.geo.dist; var projNodes = way.nodes.map(function(n) { diff --git a/test/spec/util.js b/test/spec/util.js index 6cd2e62aa..1dccda623 100644 --- a/test/spec/util.js +++ b/test/spec/util.js @@ -35,20 +35,7 @@ describe('Util', function() { expect(iD.util.geo.interp(a, b, 0)).to.eql([0, 0]); }); }); - describe('#nodeIntersect', function() { - it('correctly says that a node is in an extent', function() { - expect(iD.util.geo.nodeIntersect({ - loc: [0, 0] - }, [[-180, 90], - [180, -90]])).to.be.true; - }); - it('correctly says that a node is outside of an extent', function() { - expect(iD.util.geo.nodeIntersect({ - loc: [0, 0] - }, [[100, 90], - [180, -90]])).to.be.false; - }); - }); + describe('#dist', function() { it('distance between two same points is zero', function() { var a = [0, 0], From 44959a834d45af20760539b308b3675c1be647b3 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 6 Dec 2012 15:03:11 -0500 Subject: [PATCH 4/6] Clean elastic flag on all mode exit paths (fixes #194) --- js/id/modes/draw_area.js | 20 ++++++++++++-------- js/id/modes/draw_road.js | 15 ++++++++------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/js/id/modes/draw_area.js b/js/id/modes/draw_area.js index 69cd60f24..9fbd03b14 100644 --- a/js/id/modes/draw_area.js +++ b/js/id/modes/draw_area.js @@ -13,6 +13,13 @@ iD.modes.DrawArea = function(way_id) { firstnode_id = _.first(way.nodes), node = iD.Node({loc: mode.map.mouseCoordinates()}); + function finish(next) { + way = mode.history.graph().entity(way.id); + way.tags = _.omit(way.tags, 'elastic'); + mode.history.perform(iD.actions.ChangeEntityTags(way, way.tags)); + return mode.controller.enter(next); + } + mode.history.perform(iD.actions.AddWayNode(way, node)); mode.map.surface.on('mousemove.drawarea', function() { @@ -29,12 +36,8 @@ iD.modes.DrawArea = function(way_id) { mode.history.replace(iD.actions.DeleteNode(node)); mode.history.replace(iD.actions.AddWayNode(way, mode.history.graph().entity(way.nodes[0]))); - way = mode.history.graph().entity(way.id); - way.tags = _.omit(way.tags, 'elastic'); - mode.history.perform(iD.actions.ChangeEntityTags(way, way.tags)); - // End by clicking on own tail - return mode.controller.enter(iD.modes.Select(way)); + return finish(iD.modes.Select(way)); } else { // connect a way to an existing way mode.history.replace(iD.actions.AddWayNode(way, datum)); @@ -48,9 +51,10 @@ iD.modes.DrawArea = function(way_id) { }); mode.map.keybinding().on('⎋.drawarea', function() { - mode.controller.exit(); - }) - .on('⌫.drawarea', function() { + finish(iD.modes.Browse()); + }); + + mode.map.keybinding().on('⌫.drawarea', function() { d3.event.preventDefault(); var lastNode = _.last(way.nodes); mode.history.replace(iD.actions.removeWayNode(way, diff --git a/js/id/modes/draw_road.js b/js/id/modes/draw_road.js index 581d7ae76..2a437e884 100644 --- a/js/id/modes/draw_road.js +++ b/js/id/modes/draw_road.js @@ -16,6 +16,12 @@ iD.modes.DrawRoad = function(way_id, direction) { firstNode = way.nodes[0], lastNode = _.last(way.nodes); + function finish(next) { + way.tags = _.omit(way.tags, 'elastic'); + mode.history.perform(iD.actions.ChangeEntityTags(way, way.tags)); + return mode.controller.enter(next); + } + mode.history.perform(iD.actions.AddWayNode(way, node, index)); mode.map.surface.on('mousemove.drawroad', function() { @@ -40,12 +46,7 @@ iD.modes.DrawRoad = function(way_id, direction) { mode.history.graph().entity(lastNode), index)); } - way.tags = _.omit(way.tags, 'elastic'); - mode.history.perform(iD.actions.ChangeEntityTags( - way, way.tags)); - - // End by clicking on own tail - return mode.controller.enter(iD.modes.Select(way)); + return finish(iD.modes.Select(way)); } else { // connect a way to an existing way mode.history.replace(iD.actions.AddWayNode(way, datum, index)); @@ -70,7 +71,7 @@ iD.modes.DrawRoad = function(way_id, direction) { }); mode.map.keybinding().on('⎋.drawroad', function() { - mode.controller.exit(); + finish(iD.modes.Browse()); }); mode.map.keybinding().on('⌫.drawroad', function() { From d02e35e16b56d128c281114b4094ce7ba253a535 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 6 Dec 2012 15:08:07 -0500 Subject: [PATCH 5/6] Fix error when hovering way accuracy handles --- js/id/renderer/map.js | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/js/id/renderer/map.js b/js/id/renderer/map.js index bc6dfd940..8d1239375 100644 --- a/js/id/renderer/map.js +++ b/js/id/renderer/map.js @@ -280,17 +280,21 @@ iD.Map = function() { } function hoverIn() { - var entity = d3.select(d3.event.target).datum(); - hover = entity.id; - drawVector(iD.util.trueObj([hover])); - d3.select('.messages').text(entity.tags.name || '#' + entity.id); + var datum = d3.select(d3.event.target).datum(); + if (datum instanceof iD.Entity) { + hover = datum.id; + drawVector(iD.util.trueObj([hover])); + d3.select('.messages').text(datum.tags.name || '#' + datum.id); + } } function hoverOut() { - var oldHover = hover; - hover = null; - drawVector(iD.util.trueObj([oldHover])); - d3.select('.messages').text(''); + if (hover) { + var oldHover = hover; + hover = null; + drawVector(iD.util.trueObj([oldHover])); + d3.select('.messages').text(''); + } } function zoomPan() { From bdbcc5c331547b64072cfa933915dd169a9a7c16 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 6 Dec 2012 15:11:41 -0500 Subject: [PATCH 6/6] Fix hover --- js/id/renderer/map.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/js/id/renderer/map.js b/js/id/renderer/map.js index 8d1239375..f5f08ef17 100644 --- a/js/id/renderer/map.js +++ b/js/id/renderer/map.js @@ -207,11 +207,13 @@ iD.Map = function() { .data(data, key); lines.exit().remove(); lines.enter().append('path') + .classed('hover', classHover) .classed('active', classActive); lines .order() .attr('d', getline) .attr('class', class_gen) + .classed('hover', classHover) .classed('active', classActive); return lines; }