From ddc5e324f6cfa7fdcc25bfc679b0f25c70ad97b2 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Sat, 2 Feb 2013 18:28:44 -0500 Subject: [PATCH] Extract iD.Difference 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. Additionally, all history mutators now return a difference. --- index.html | 1 + js/id/graph/difference.js | 113 +++++++++++++++++++ js/id/graph/graph.js | 60 ---------- js/id/graph/history.js | 44 ++++---- js/id/renderer/map.js | 33 +----- test/index.html | 2 + test/index_packaged.html | 1 + test/spec/graph/difference.js | 202 ++++++++++++++++++++++++++++++++++ test/spec/graph/graph.js | 82 -------------- test/spec/graph/history.js | 41 +++++-- 10 files changed, 375 insertions(+), 204 deletions(-) create mode 100644 js/id/graph/difference.js create mode 100644 test/spec/graph/difference.js 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); }); });