From 68ee31ea29fa3f3e6ca7cf16031b2900787232f9 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 8 Nov 2012 15:45:39 -0800 Subject: [PATCH 1/2] Move controller to own file --- index.html | 2 ++ js/iD/actions/actions.js | 20 -------------------- js/iD/controller/controller.js | 20 +++++++++++++++++++- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/index.html b/index.html index 08e755cf2..2b5301b05 100755 --- a/index.html +++ b/index.html @@ -47,6 +47,8 @@ + + diff --git a/js/iD/actions/actions.js b/js/iD/actions/actions.js index 8cd2f09ef..1757efc1e 100644 --- a/js/iD/actions/actions.js +++ b/js/iD/actions/actions.js @@ -177,23 +177,3 @@ iD.actions.Move = { }, exit: function() { } }; - -// A controller holds a single action at a time and calls `.enter` and `.exit` -// to bind and unbind actions. -iD.controller = function(map) { - var controller = { action: null }; - - controller.go = function(x) { - x.controller = controller; - x.map = map; - if (controller.action) { - controller.action.exit(); - } - x.enter(); - controller.action = x; - }; - - controller.go(iD.actions.Move); - - return controller; -}; diff --git a/js/iD/controller/controller.js b/js/iD/controller/controller.js index 4375209c7..855f6e416 100644 --- a/js/iD/controller/controller.js +++ b/js/iD/controller/controller.js @@ -1 +1,19 @@ -iD.controller = {}; +// A controller holds a single action at a time and calls `.enter` and `.exit` +// to bind and unbind actions. +iD.controller = function(map) { + var controller = { action: null }; + + controller.go = function(x) { + x.controller = controller; + x.map = map; + if (controller.action) { + controller.action.exit(); + } + x.enter(); + controller.action = x; + }; + + controller.go(iD.actions.Move); + + return controller; +}; From c9c40311e73f89a0ca29f5c869e67c7e63f938e9 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 8 Nov 2012 21:42:10 -0800 Subject: [PATCH 2/2] Refactoring Graph manipulation Extract iD.History from Graph. History is a versioned series of Graphs that knows how to do, undo, and redo. Graph holds entities and an annotation. A parallel array of annotations in history is no longer necessary. Operations no longer need a map object to be threaded through. Fixed #65. --- index.html | 1 + js/iD/actions/actions.js | 10 ++-- js/iD/actions/operations.js | 81 ++++++++++++-------------------- js/iD/graph/Graph.js | 94 ++++++++++--------------------------- js/iD/graph/History.js | 38 +++++++++++++++ js/iD/renderer/Map.js | 64 +++++++++++++------------ 6 files changed, 133 insertions(+), 155 deletions(-) create mode 100644 js/iD/graph/History.js diff --git a/index.html b/index.html index 2b5301b05..a2692c933 100755 --- a/index.html +++ b/index.html @@ -58,6 +58,7 @@ + diff --git a/js/iD/actions/actions.js b/js/iD/actions/actions.js index 1757efc1e..adba9e3ca 100644 --- a/js/iD/actions/actions.js +++ b/js/iD/actions/actions.js @@ -39,7 +39,7 @@ iD.actions.AddPlace = { surface.on('click.addplace', function() { var ll = this.map.projection.invert( d3.mouse(surface.node())); - iD.operations.addNode(this.map, iD.actions._node(ll)); + this.map.do(iD.operations.addNode(iD.actions._node(ll))); this.exit(); }.bind(this)); @@ -95,7 +95,7 @@ iD.actions.AddRoad = { var node = iD.actions._node(ll); way.nodes.push(node.id); - iD.operations.changeWayNodes(this.map, way, node); + this.map.do(iD.operations.changeWayNodes(way, node)); this.controller.go(iD.actions.DrawRoad(way)); }.bind(this)); @@ -119,7 +119,7 @@ iD.actions.DrawRoad = function(way) { this.falsenode = iD.actions._node([0, 0]); - iD.operations.addTemporary(this.map, this.falsenode); + this.map.do(iD.operations.addTemporary(this.falsenode)); // way.nodes = way.nodes.slice(); way.nodes.push(this.falsenode.id); @@ -140,7 +140,7 @@ iD.actions.DrawRoad = function(way) { way.nodes.push(node.id); - iD.operations.changeWayNodes(this.map, way, node); + this.map.do(iD.operations.changeWayNodes(way, node)); way.nodes = way.nodes.slice(); way.nodes.push(this.falsenode.id); @@ -152,7 +152,7 @@ iD.actions.DrawRoad = function(way) { }.bind(this)); }, exit: function() { - iD.operations.addTemporary(this.map, this.falsenode); + this.map.do(iD.operations.addTemporary(this.falsenode)); this.map.surface.on('mousemove.drawroad', null); this.map.surface.on('click.drawroad', null); d3.select(document).on('.drawroad', null); diff --git a/js/iD/actions/operations.js b/js/iD/actions/operations.js index 39216d52e..f3c47e9d5 100644 --- a/js/iD/actions/operations.js +++ b/js/iD/actions/operations.js @@ -1,67 +1,46 @@ iD.operations = {}; -// operations take a map, and arguments that they modify in the map's graph. -// they use `graph.modify` to do this while keeping a previous version -// of the graph the same. - -iD.operations.addNode = function(map, node) { - map.graph.modify(function(graph) { - var o = {}; - o[node.id] = node; - return graph.set(o); - }, 'added a place'); - map.update(); +iD.operations.addNode = function(node) { + return function(graph) { + return graph.replace(node, 'added a place'); + } }; -iD.operations.startWay = function(map, way) { - map.graph.modify(function(graph) { - var o = {}; - o[way.id] = way; - return graph.set(o); - }, 'started a road'); - map.update(); +iD.operations.startWay = function(way) { + return function(graph) { + return graph.replace(way, 'started a road'); + }; }; -iD.operations.remove = function(map, node) { - map.graph.modify(function(graph) { - return graph.remove(node.id); - }, 'removed a feature'); - map.update(); +iD.operations.remove = function(node) { + return function(graph) { + return graph.remove(node, 'removed a feature'); + }; }; -iD.operations.changeWayNodes = function(map, way, node) { - map.graph.modify(function(graph) { - var o = {}; +iD.operations.changeWayNodes = function(way, node) { + return function(graph) { way.nodes = way.nodes.slice(); - o[way.id] = pdata.object(way).get(); - o[node.id] = node; - return graph.set(o); - }, 'added to a road'); - map.update(); + way = pdata.object(way).get(); + return graph.replace(way).replace(node, 'added to a road'); + }; }; -iD.operations.addTemporary = function(map, node) { - map.graph.modify(function(graph) { - var o = {}; - o[node.id] = node; - return graph.set(o); - }, ''); - map.update(); +iD.operations.changeTags = function(node, tags) { + return function(graph) { + var node = pdata.object(node).set({ tags: tags }).get(); + return graph.replace(node, 'changed tags'); + }; }; -iD.operations.changeTags = function(map, node, tags) { - map.graph.modify(function(graph) { - var o = {}; - var copy = pdata.object(node).set({ tags: tags }).get(); - o[copy.id] = copy; - return graph.set(o); - }, 'changed tags'); - map.update(); +iD.operations.addTemporary = function(node) { + return function(graph) { + return graph.replace(node); + }; }; -iD.operations.removeTemporary = function(map, node) { - map.graph.modify(function(graph) { - return graph.remove(node.id); - }, ''); - map.update(); +iD.operations.removeTemporary = function(node) { + return function(graph) { + return graph.remove(node); + }; }; diff --git a/js/iD/graph/Graph.js b/js/iD/graph/Graph.js index 4797689ea..77f122260 100644 --- a/js/iD/graph/Graph.js +++ b/js/iD/graph/Graph.js @@ -1,102 +1,60 @@ -iD.Graph = function() { }; +iD.Graph = function(entities, annotation) { + this.entities = entities || {}; + this.annotation = annotation; +}; iD.Graph.prototype = { - - // a pointer to the top of the stack. - head: {}, - // a pointer to the latest annotation - annotation: null, - - // stack of previous versions of this datastructure - prev: [], - // stack of previous annotations - annotations: [], + entity: function(id) { + return this.entities[id]; + }, // get all points that are not part of a way. this is an expensive // call that needs to be optimized. - pois: function(head) { + pois: function() { var included = [], pois = [], idx = {}; - for (var i in head) { - if (head[i].nodes) { - included = included.concat(head[i].nodes); + for (var i in this.entities) { + if (this.entities[i].nodes) { + included = included.concat(this.entities[i].nodes); } } for (var j = 0; j < included.length; j++) { idx[included[j]] = true; } - for (var k in head) { - if (head[k].type === 'node' && !idx[head[k].id]) { - pois.push(head[k]); + for (var k in this.entities) { + if (this.entities[k].type === 'node' && !idx[this.entities[k].id]) { + pois.push(this.entities[k]); } } return pois; }, - // rewind and fast-forward the graph. these preserve the other modes of the - // graph. these attempt to skip over any edits that didn't have an annotation, - // like 'invisible edits' and sub-edits. - undo: function() { - if (this.prev.length && this.prev[0] !== this.head) { - for (var idx = this.prev.indexOf(this.head) - 1; idx > 0; idx--) { - if (this.annotations[idx]) break; - } - this.head = this.prev[idx]; - this.annotation = this.annotations[idx]; - } - }, - redo: function() { - if (this.prev.length && this.prev[this.prev.length - 1] !== this.head) { - for (var idx = this.prev.indexOf(this.head) + 1; idx < this.prev.length - 1; idx++) { - if (this.annotations[idx]) break; - } - this.head = this.prev[idx]; - this.annotation = this.annotations[idx]; - } - }, - insert: function(a) { for (var i = 0; i < a.length; i++) { - if (this.head[a[i].id]) return; - this.head[a[i].id] = a[i]; + if (this.entities[a[i].id]) return; + this.entities[a[i].id] = a[i]; } }, - // the gist of all operations on the graph: the callback function - // receives the current graph and returns a modified graph. the graph - // given to the callback is guaranteed to be immutable at one level - the - // key -> object mappings. the callback is responsible for keeping objects - // in the graph immutable. - modify: function(callback, annotation) { - // create a pdata wrapper of current head - var o = pdata.object(this.head); + replace: function(entity, annotation) { + var o = {}; + o[entity.id] = entity; + return new iD.Graph(pdata.object(this.entities).set(o).get(), annotation); + }, - // Archive current version - this.prev.push(o.get()); - - // Let the operation make modification of a safe - // copy - var modified = callback(o); - - // Archive this version - this.prev.push(modified.get()); - // Annotate this version - this.annotations.push(annotation); - - // Make head the top of the previous stack - this.head = this.prev[this.prev.length - 1]; - this.annotation = this.annotations[this.annotations.length - 1]; + remove: function(entity, annotation) { + return new iD.Graph(pdata.object(this.entities).remove(entity.id).get(), annotation); }, // get all objects that intersect an extent. intersects: function(extent) { var items = []; - for (var i in this.head) { - if (this.head[i]) items.push(this.head[i]); + for (var i in this.entities) { + if (this.entities[i]) items.push(this.entities[i]); } return items; }, // Resolve the id references in a way, replacing them with actual objects. fetch: function(id) { - var o = this.head[id]; + var o = this.entities[id]; var f = _.clone(o); if (!f.nodes || !f.nodes.length) return f; f.nodes = f.nodes.map(function(c) { diff --git a/js/iD/graph/History.js b/js/iD/graph/History.js new file mode 100644 index 000000000..3c2c2f61d --- /dev/null +++ b/js/iD/graph/History.js @@ -0,0 +1,38 @@ +iD.History = function() { + this.stack = [new iD.Graph()]; + this.index = 0; +}; + +iD.History.prototype = { + graph: function() { + return this.stack[this.index]; + }, + + do: function(operation) { + this.stack = this.stack.slice(0, this.index + 1); + this.stack.push(operation(this.graph())); + this.index++; + }, + + undo: function() { + while (this.index > 0) { + this.index--; + if (this.stack[this.index].annotation) break; + } + }, + + redo: function() { + while (this.index < this.stack.length - 1) { + this.index++; + if (this.stack[this.index].annotation) break; + } + }, + + entity: function(id) { + return this.graph().entity(id); + }, + + fetch: function(id) { + return this.graph().fetch(id); + } +}; diff --git a/js/iD/renderer/Map.js b/js/iD/renderer/Map.js index 500c70583..f95967dbc 100755 --- a/js/iD/renderer/Map.js +++ b/js/iD/renderer/Map.js @@ -22,9 +22,9 @@ iD.Map = function(elem) { width, height, dispatch = d3.dispatch('move', 'update'), // data - graph = new iD.Graph(), - connection = new iD.Connection(graph), - inspector = iD.Inspector(graph), + history = new iD.History(), + connection = new iD.Connection(history.graph()), + inspector = iD.Inspector(history), parent = d3.select(elem), selection = [], projection = d3.geo.mercator() @@ -38,13 +38,12 @@ iD.Map = function(elem) { // this is used with handles dragbehavior = d3.behavior.drag() .origin(function(d) { - var data = (typeof d === 'string') ? graph.head[d] : d; - graph.modify(function(o) { - var c = {}; - c[data.id] = pdata.object(data).set({ modified: true }).get(); - return o.set(c); - }, ''); - p = projection(ll2a(data)); + var entity = (typeof d === 'string') ? history.entity(d) : d; + history.do(function(graph) { + var node = pdata.object(entity).set({ modified: true }).get(); + return graph.replace(node); + }); + var p = projection(ll2a(entity)); return { x: p[0], y: p[1] }; }) .on('drag', function(d) { @@ -52,30 +51,27 @@ iD.Map = function(elem) { return 'translate(' + d3.event.x + ',' + d3.event.y + ')'; }); var ll = projection.invert([d3.event.x, d3.event.y]); - graph.head[d].lon = ll[0]; - graph.head[d].lat = ll[1]; + history.entity(d).lon = ll[0]; + history.entity(d).lat = ll[1]; drawVector(); }) .on('dragend', function(d) { - var data = (typeof d === 'string') ? graph.head[d] : d; - graph.modify(function(o) { - var c = {}; - c[data.id] = pdata.object(c[data.id]).get(); - o.set(c); - return o; - }, 'moved an element'); + var entity = (typeof d === 'string') ? history.entity(d) : d; + history.do(function(graph) { + return graph.replace(entity, 'moved an element'); + }); map.update(); }), // geo linegen = d3.svg.line() .defined(function(d) { - return !!graph.head[d]; + return !!history.entity(d); }) .x(function(d) { - return projection(ll2a(graph.head[d]))[0]; + return projection(ll2a(history.entity(d)))[0]; }) .y(function(d) { - return projection(ll2a(graph.head[d]))[1]; + return projection(ll2a(history.entity(d)))[1]; }), // Abstract linegen so that it pulls from `.children`. This // makes it possible to call simply `.attr('d', nodeline)`. @@ -119,7 +115,7 @@ iD.Map = function(elem) { var tileclient = iD.Tiles(tilegroup, projection); function drawVector() { - var all = graph.intersects(getExtent()); + var all = history.graph().intersects(getExtent()); var ways = all.filter(function(a) { return a.type === 'way' && !iD.Way.isClosed(a); @@ -127,7 +123,7 @@ iD.Map = function(elem) { areas = all.filter(function(a) { return a.type === 'way' && iD.Way.isClosed(a); }), - points = graph.pois(graph.head); + points = history.graph().pois(); var fills = fill_g.selectAll('path.area').data(areas, key), casings = casing_g.selectAll('path.casing').data(ways, key), @@ -192,7 +188,7 @@ iD.Map = function(elem) { .attr('r', 5) .call(dragbehavior); handles.attr('transform', function(d) { - return 'translate(' + projection(ll2a(graph.head[d])) + ')'; + return 'translate(' + projection(ll2a(history.entity(d))) + ')'; }); } @@ -225,11 +221,11 @@ iD.Map = function(elem) { } inspector.on('change', function(d, tags) { - iD.operations.changeTags(map, d, tags); + map.do(iD.operations.changeTags(d, tags)); }); inspector.on('remove', function(d) { - iD.operations.remove(map, d); + map.do(iD.operations.remove(d)); }); function zoomPan() { @@ -254,18 +250,23 @@ iD.Map = function(elem) { // ----------- var undolabel = d3.select('button#undo small'); dispatch.on('update', function() { - undolabel.text(graph.annotation); + undolabel.text(history.graph().annotation); redraw(); }); + function _do(operation) { + history.do(operation); + map.update(); + } + // Undo/redo function undo() { - graph.undo(); + history.undo(); map.update(); } function redo() { - graph.redo(); + history.redo(); map.update(); } @@ -354,9 +355,10 @@ iD.Map = function(elem) { map.projection = projection; map.setSize = setSize; - map.graph = graph; + map.history = history; map.surface = surface; + map.do = _do; map.undo = undo; map.redo = redo;