diff --git a/index.html b/index.html index d8be52dcd..f9b627c16 100644 --- a/index.html +++ b/index.html @@ -119,6 +119,7 @@ + diff --git a/js/id/graph/difference.js b/js/id/graph/difference.js new file mode 100644 index 000000000..8bea5664c --- /dev/null +++ b/js/id/graph/difference.js @@ -0,0 +1,113 @@ +/* + iD.Difference represents the difference between two graphs. + It knows how to calculate the set of entities that were + created, modified, or deleted, and also contains the logic + for recursively extending a difference to the complete set + of entities that will require a redraw, taking into account + child and parent relationships. + */ +iD.Difference = function (base, head) { + var changes = {}, length = 0; + + _.each(head.entities, function(h, id) { + var b = base.entities[id]; + if (h !== b) { + changes[id] = {base: b, head: h}; + length++; + } + }); + + _.each(base.entities, function(b, id) { + var h = head.entities[id]; + if (!changes[id] && h !== b) { + changes[id] = {base: b, head: h}; + length++; + } + }); + + var difference = {}; + + difference.length = function () { + return length; + }; + + difference.changes = function() { + return changes; + }; + + difference.modified = function() { + var result = []; + _.each(changes, function(change) { + if (change.base && change.head) result.push(change.head); + }); + return result; + }; + + difference.created = function() { + var result = []; + _.each(changes, function(change) { + if (!change.base && change.head) result.push(change.head); + }); + return result; + }; + + difference.deleted = function() { + var result = []; + _.each(changes, function(change) { + if (change.base && !change.head) result.push(change.base); + }); + return result; + }; + + difference.complete = function(extent) { + var result = {}, id, change; + + function addParents(parents) { + for (var i = 0; i < parents.length; i++) { + var parent = parents[i]; + + if (parent.id in result) + continue; + + result[parent.id] = parent; + addParents(head.parentRelations(parent)); + } + } + + for (id in changes) { + change = changes[id]; + + var h = change.head, + b = change.base, + entity = h || b; + + if (extent && !entity.intersects(extent, h ? head : base)) + continue; + + result[id] = h; + + if (entity.type === 'way') { + var nh = h ? h.nodes : [], + nb = b ? b.nodes : [], + diff; + + diff = _.difference(nh, nb); + for (var i = 0; i < diff.length; i++) { + result[diff[i]] = head.entity(diff[i]); + } + + diff = _.difference(nb, nh); + for (var i = 0; i < diff.length; i++) { + result[diff[i]] = head.entity(diff[i]); + } + } + + addParents(head.parentWays(entity)); + addParents(head.parentRelations(entity)); + } + + return result; + }; + + return difference; +}; diff --git a/js/id/graph/graph.js b/js/id/graph/graph.js index a19bfe094..2692aeba6 100644 --- a/js/id/graph/graph.js +++ b/js/id/graph/graph.js @@ -232,65 +232,5 @@ iD.Graph.prototype = { } } return items; - }, - - difference: function (graph) { - - function diff(a, b) { - var result = [], - keys = Object.keys(a.entities), - entity, oldentity, id, i; - - for (i = 0; i < keys.length; i++) { - id = keys[i]; - entity = a.entities[id]; - oldentity = b.entities[id]; - if (entity !== oldentity) { - - // maybe adding affected children better belongs in renderer/map.js? - if (entity && entity.type === 'way' && - oldentity && oldentity.type === 'way') { - result = result - .concat(_.difference(entity.nodes, oldentity.nodes)) - .concat(_.difference(oldentity.nodes, entity.nodes)); - - } else if (entity && entity.type === 'way') { - result = result.concat(entity.nodes); - - } else if (oldentity && oldentity.type === 'way') { - result = result.concat(oldentity.nodes); - } - - result.push(id); - } - } - return result; - } - - return _.unique(diff(this, graph).concat(diff(graph, this)).sort()); - }, - - modified: function() { - var result = [], base = this.base().entities; - _.each(this.entities, function(entity, id) { - if (entity && base[id]) result.push(id); - }); - return result; - }, - - created: function() { - var result = [], base = this.base().entities; - _.each(this.entities, function(entity, id) { - if (entity && !base[id]) result.push(id); - }); - return result; - }, - - deleted: function() { - var result = [], base = this.base().entities; - _.each(this.entities, function(entity, id) { - if (!entity && base[id]) result.push(id); - }); - return result; } }; diff --git a/js/id/graph/history.js b/js/id/graph/history.js index 42e1b1604..cbadbf87f 100644 --- a/js/id/graph/history.js +++ b/js/id/graph/history.js @@ -21,7 +21,9 @@ iD.History = function() { } function change(previous) { - dispatch.change(history.graph().difference(previous)); + var difference = iD.Difference(previous, history.graph()); + dispatch.change(difference); + return difference; } var history = { @@ -42,7 +44,7 @@ iD.History = function() { stack.push(perform(arguments)); index++; - change(previous); + return change(previous); }, replace: function () { @@ -51,7 +53,7 @@ iD.History = function() { // assert(index == stack.length - 1) stack[index] = perform(arguments); - change(previous); + return change(previous); }, pop: function () { @@ -60,7 +62,7 @@ iD.History = function() { if (index > 0) { index--; stack.pop(); - change(previous); + return change(previous); } }, @@ -80,7 +82,7 @@ iD.History = function() { } dispatch.undone(); - change(previous); + return change(previous); }, redo: function () { @@ -92,7 +94,7 @@ iD.History = function() { } dispatch.redone(); - change(previous); + return change(previous); }, undoAnnotation: function () { @@ -111,31 +113,27 @@ iD.History = function() { } }, - changes: function () { - var initial = stack[0].graph, - current = stack[index].graph; + difference: function () { + var base = stack[0].graph, + head = stack[index].graph; + return iD.Difference(base, head); + }, + changes: function () { + var difference = history.difference(); return { - modified: current.modified().map(function (id) { - return current.entity(id); - }), - created: current.created().map(function (id) { - return current.entity(id); - }), - deleted: current.deleted().map(function (id) { - return initial.entity(id); - }) - }; + modified: difference.modified(), + created: difference.created(), + deleted: difference.deleted() + } }, hasChanges: function() { - return !!this.numChanges(); + return this.difference().length() > 0; }, numChanges: function() { - return d3.sum(d3.values(this.changes()).map(function(c) { - return c.length; - })); + return this.difference().length(); }, imagery_used: function(source) { diff --git a/js/id/renderer/map.js b/js/id/renderer/map.js index 1f22a7c81..839aa18a3 100644 --- a/js/id/renderer/map.js +++ b/js/id/renderer/map.js @@ -64,44 +64,19 @@ iD.Map = function(context) { extent = map.extent(), graph = context.graph(); - function addParents(parents) { - for (var i = 0; i < parents.length; i++) { - var parent = parents[i]; - if (only[parent.id] === undefined) { - only[parent.id] = parent; - addParents(graph.parentRelations(parent)); - } - } - } - if (!difference) { all = graph.intersects(extent); filter = d3.functor(true); } else { - var only = {}; - - for (var j = 0; j < difference.length; j++) { - var id = difference[j], - entity = graph.entity(id); - - // Even if the entity is false (deleted), it needs to be - // removed from the surface - only[id] = entity; - - if (entity && entity.intersects(extent, graph)) { - addParents(graph.parentWays(only[id])); - addParents(graph.parentRelations(only[id])); - } - } - - all = _.compact(_.values(only)); + var complete = difference.complete(extent); + all = _.compact(_.values(complete)); filter = function(d) { if (d.type === 'midpoint') { for (var i = 0; i < d.ways.length; i++) { - if (d.ways[i].id in only) return true; + if (d.ways[i].id in complete) return true; } } else { - return d.id in only; + return d.id in complete; } }; } diff --git a/test/index.html b/test/index.html index da3cd475c..b6d925923 100644 --- a/test/index.html +++ b/test/index.html @@ -115,6 +115,7 @@ + @@ -164,6 +165,7 @@ + diff --git a/test/index_packaged.html b/test/index_packaged.html index 948ed0ae1..ef91e2823 100644 --- a/test/index_packaged.html +++ b/test/index_packaged.html @@ -55,6 +55,7 @@ + diff --git a/test/spec/graph/difference.js b/test/spec/graph/difference.js new file mode 100644 index 000000000..f12a77254 --- /dev/null +++ b/test/spec/graph/difference.js @@ -0,0 +1,202 @@ +describe("iD.Difference", function () { + describe("#changes", function () { + it("includes created entities", function () { + var node = iD.Node({id: 'n'}), + base = iD.Graph(), + head = base.replace(node), + diff = iD.Difference(base, head); + expect(diff.changes()).to.eql({n: {base: undefined, head: node}}); + }); + + it("includes undone created entities", function () { + var node = iD.Node({id: 'n'}), + base = iD.Graph(), + head = base.replace(node), + diff = iD.Difference(head, base); + expect(diff.changes()).to.eql({n: {base: node, head: undefined}}); + }); + + it("includes modified entities", function () { + var n1 = iD.Node({id: 'n'}), + n2 = n1.update(), + base = iD.Graph([n1]), + head = base.replace(n2), + diff = iD.Difference(base, head); + expect(diff.changes()).to.eql({n: {base: n1, head: n2}}); + }); + + it("includes undone modified entities", function () { + var n1 = iD.Node({id: 'n'}), + n2 = n1.update(), + base = iD.Graph([n1]), + head = base.replace(n2), + diff = iD.Difference(head, base); + expect(diff.changes()).to.eql({n: {base: n2, head: n1}}); + }); + + it("includes deleted entities", function () { + var node = iD.Node({id: 'n'}), + base = iD.Graph([node]), + head = base.remove(node), + diff = iD.Difference(base, head); + expect(diff.changes()).to.eql({n: {base: node, head: undefined}}); + }); + + it("includes undone deleted entities", function () { + var node = iD.Node({id: 'n'}), + base = iD.Graph([node]), + head = base.remove(node), + diff = iD.Difference(head, base); + expect(diff.changes()).to.eql({n: {base: undefined, head: node}}); + }); + + it("doesn't include created entities that were subsequently deleted", function () { + var node = iD.Node(), + base = iD.Graph(), + head = base.replace(node).remove(node), + diff = iD.Difference(base, head); + expect(diff.changes()).to.eql({}); + }); + }); + + describe("#created", function () { + it("returns an array of created entities", function () { + var node = iD.Node({id: 'n'}), + base = iD.Graph(), + head = base.replace(node), + diff = iD.Difference(base, head); + expect(diff.created()).to.eql([node]); + }); + }); + + describe("#modified", function () { + it("returns an array of modified entities", function () { + var n1 = iD.Node({id: 'n'}), + n2 = n1.move([1, 2]), + base = iD.Graph([n1]), + head = base.replace(n2), + diff = iD.Difference(base, head); + expect(diff.modified()).to.eql([n2]); + }); + }); + + describe("#deleted", function () { + it("returns an array of deleted entities", function () { + var node = iD.Node({id: 'n'}), + base = iD.Graph([node]), + head = base.remove(node), + diff = iD.Difference(base, head); + expect(diff.deleted()).to.eql([node]); + }); + }); + + describe("#complete", function () { + it("includes created entities", function () { + var node = iD.Node({id: 'n'}), + base = iD.Graph(), + head = base.replace(node), + diff = iD.Difference(base, head); + expect(diff.complete()['n']).to.equal(node); + }); + + it("includes modified entities", function () { + var n1 = iD.Node({id: 'n'}), + n2 = n1.move([1, 2]), + base = iD.Graph([n1]), + head = base.replace(n2), + diff = iD.Difference(base, head); + expect(diff.complete()['n']).to.equal(n2); + }); + + it("includes deleted entities", function () { + var node = iD.Node({id: 'n'}), + base = iD.Graph([node]), + head = base.remove(node), + diff = iD.Difference(base, head); + expect(diff.complete()).to.eql({n: undefined}); + }); + + it("includes nodes added to a way", function () { + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n2'}), + w1 = iD.Way({id: 'w', nodes: ['n1']}), + w2 = w1.addNode('n2'), + base = iD.Graph([n1, n2, w1]), + head = base.replace(w2), + diff = iD.Difference(base, head); + + expect(diff.complete()['n2']).to.equal(n2); + }); + + it("includes nodes removed from a way", function () { + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n2'}), + w1 = iD.Way({id: 'w', nodes: ['n1', 'n2']}), + w2 = w1.removeNode('n2'), + base = iD.Graph([n1, n2, w1]), + head = base.replace(w2), + diff = iD.Difference(base, head); + + expect(diff.complete()['n2']).to.equal(n2); + }); + + it("includes parent ways of modified nodes", function () { + var n1 = iD.Node({id: 'n'}), + n2 = n1.move([1, 2]), + way = iD.Way({id: 'w', nodes: ['n']}), + base = iD.Graph([n1, way]), + head = base.replace(n2), + diff = iD.Difference(base, head); + + expect(diff.complete()['w']).to.equal(way); + }); + + it("includes parent relations of modified entities", function () { + var n1 = iD.Node({id: 'n'}), + n2 = n1.move([1, 2]), + rel = iD.Relation({id: 'r', members: [{id: 'n'}]}), + base = iD.Graph([n1, rel]), + head = base.replace(n2), + diff = iD.Difference(base, head); + + expect(diff.complete()['r']).to.equal(rel); + }); + + it("includes parent relations of modified entities, recursively", function () { + var n1 = iD.Node({id: 'n'}), + n2 = n1.move([1, 2]), + rel1 = iD.Relation({id: 'r1', members: [{id: 'n'}]}), + rel2 = iD.Relation({id: 'r2', members: [{id: 'r1'}]}), + base = iD.Graph([n1, rel1, rel2]), + head = base.replace(n2), + diff = iD.Difference(base, head); + + expect(diff.complete()['r2']).to.equal(rel2); + }); + + it("includes parent relations of parent ways of modified nodes", function () { + var n1 = iD.Node({id: 'n'}), + n2 = n1.move([1, 2]), + way = iD.Way({id: 'w', nodes: ['n']}), + rel = iD.Relation({id: 'r', members: [{id: 'w'}]}), + base = iD.Graph([n1, way, rel]), + head = base.replace(n2), + diff = iD.Difference(base, head); + + expect(diff.complete()['r']).to.equal(rel); + }); + + it("copes with recursive relations", function () { + var node = iD.Node({id: 'n'}), + rel1 = iD.Relation({id: 'r1', members: [{id: 'n'}, {id: 'r2'}]}), + rel2 = iD.Relation({id: 'r2', members: [{id: 'r1'}]}), + base = iD.Graph([node, rel1, rel2]), + head = base.replace(node.move([1, 2])), + diff = iD.Difference(base, head); + + expect(diff.complete()).to.be.ok; + }); + + it("limits changes to those within a given extent"); + }); +}); diff --git a/test/spec/graph/graph.js b/test/spec/graph/graph.js index 05bcbbe89..a357ff360 100644 --- a/test/spec/graph/graph.js +++ b/test/spec/graph/graph.js @@ -333,86 +333,4 @@ describe('iD.Graph', function() { expect(graph.childNodes(way)).to.eql([node]); }); }); - - describe("#difference", function () { - it("returns an Array of ids of changed entities", function () { - var initial = iD.Node({id: "n1"}), - updated = initial.update({}), - created = iD.Node(), - deleted = iD.Node({id: 'n2'}), - graph1 = iD.Graph([initial, deleted]), - graph2 = graph1.replace(updated).replace(created).remove(deleted); - expect(graph2.difference(graph1)).to.eql([created.id, updated.id, deleted.id]); - }); - - - it("includes created entities, and reverse", function () { - var node = iD.Node(), - graph1 = iD.Graph(), - graph2 = graph1.replace(node); - expect(graph2.difference(graph1)).to.eql([node.id]); - expect(graph1.difference(graph2)).to.eql([node.id]); - }); - - it("includes entities changed from base, and reverse", function () { - var node = iD.Node(), - graph1 = iD.Graph(node), - graph2 = graph1.replace(node.update()); - expect(graph2.difference(graph1)).to.eql([node.id]); - expect(graph1.difference(graph2)).to.eql([node.id]); - }); - - it("includes already changed entities that were updated, and reverse", function () { - var node = iD.Node(), - graph1 = iD.Graph().replace(node), - graph2 = graph1.replace(node.update()); - expect(graph2.difference(graph1)).to.eql([node.id]); - expect(graph1.difference(graph2)).to.eql([node.id]); - }); - - it("includes affected child nodes", function () { - var n = iD.Node({id: 'n'}), - n2 = iD.Node({id: 'n2'}), - w1 = iD.Way({id: 'w1', nodes: ['n']}), - w1_ = iD.Way({id: 'w1', nodes: ['n', 'n2']}), - graph1 = iD.Graph([n, n2, w1]), - graph2 = graph1.replace(w1_); - expect(graph2.difference(graph1)).to.eql(['n2', 'w1']); - expect(graph1.difference(graph2)).to.eql(['n2', 'w1']); - }); - - }); - - describe("#modified", function () { - it("returns an Array of ids of modified entities", function () { - var node = iD.Node({id: 'n1'}), - node_ = iD.Node({id: 'n1'}), - graph = iD.Graph([node]).replace(node_); - expect(graph.modified()).to.eql([node.id]); - }); - }); - - describe("#created", function () { - it("returns an Array of ids of created entities", function () { - var node1 = iD.Node({id: 'n-1'}), - node2 = iD.Node({id: 'n2'}), - graph = iD.Graph([node2]).replace(node1); - expect(graph.created()).to.eql([node1.id]); - }); - }); - - describe("#deleted", function () { - it("returns an Array of ids of deleted entities", function () { - var node1 = iD.Node({id: "n1"}), - node2 = iD.Node(), - graph = iD.Graph([node1, node2]).remove(node1); - expect(graph.deleted()).to.eql([node1.id]); - }); - - it("doesn't include created entities that were subsequently deleted", function () { - var node = iD.Node(), - graph = iD.Graph().replace(node).remove(node); - expect(graph.deleted()).to.eql([]); - }); - }); }); diff --git a/test/spec/graph/history.js b/test/spec/graph/history.js index dc685b446..3f28f437e 100644 --- a/test/spec/graph/history.js +++ b/test/spec/graph/history.js @@ -14,6 +14,10 @@ describe("iD.History", function () { }); describe("#perform", function () { + it("returns a difference", function () { + expect(history.perform(action).changes()).to.eql({}); + }); + it("updates the graph", function () { var node = iD.Node(); history.perform(function (graph) { return graph.replace(node); }); @@ -27,8 +31,8 @@ describe("iD.History", function () { it("emits a change event", function () { history.on('change', spy); - history.perform(action); - expect(spy).to.have.been.calledWith([]); + var difference = history.perform(action); + expect(spy).to.have.been.calledWith(difference); }); it("performs multiple actions", function () { @@ -42,6 +46,10 @@ describe("iD.History", function () { }); describe("#replace", function () { + it("returns a difference", function () { + expect(history.replace(action).changes()).to.eql({}); + }); + it("updates the graph", function () { var node = iD.Node(); history.replace(function (graph) { return graph.replace(node); }); @@ -56,8 +64,8 @@ describe("iD.History", function () { it("emits a change event", function () { history.on('change', spy); - history.replace(action); - expect(spy).to.have.been.calledWith([]); + var difference = history.replace(action); + expect(spy).to.have.been.calledWith(difference); }); it("performs multiple actions", function () { @@ -71,6 +79,11 @@ describe("iD.History", function () { }); describe("#pop", function () { + it("returns a difference", function () { + history.perform(action, "annotation"); + expect(history.pop().changes()).to.eql({}); + }); + it("updates the graph", function () { history.perform(action, "annotation"); history.pop(); @@ -86,12 +99,16 @@ describe("iD.History", function () { it("emits a change event", function () { history.perform(action); history.on('change', spy); - history.pop(); - expect(spy).to.have.been.calledWith([]); + var difference = history.pop(); + expect(spy).to.have.been.calledWith(difference); }); }); describe("#undo", function () { + it("returns a difference", function () { + expect(history.undo().changes()).to.eql({}); + }); + it("pops the undo stack", function () { history.perform(action, "annotation"); history.undo(); @@ -121,12 +138,16 @@ describe("iD.History", function () { it("emits a change event", function () { history.perform(action); history.on('change', spy); - history.undo(); - expect(spy).to.have.been.calledWith([]); + var difference = history.undo(); + expect(spy).to.have.been.calledWith(difference); }); }); describe("#redo", function () { + it("returns a difference", function () { + expect(history.redo().changes()).to.eql({}); + }); + it("emits an redone event", function () { history.perform(action); history.undo(); @@ -139,8 +160,8 @@ describe("iD.History", function () { history.perform(action); history.undo(); history.on('change', spy); - history.redo(); - expect(spy).to.have.been.calledWith([]); + var difference = history.redo(); + expect(spy).to.have.been.calledWith(difference); }); });