From f1deed2712afc2fae3746b3dff3f7c1271d21956 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Fri, 18 Oct 2013 10:22:29 -0400 Subject: [PATCH] Fix area sorting corner case (fixes #1903) --- js/id/svg/areas.js | 22 +++++++++++--------- test/spec/svg/areas.js | 46 +++++++++++++++++++++++++++++------------- 2 files changed, 44 insertions(+), 24 deletions(-) diff --git a/js/id/svg/areas.js b/js/id/svg/areas.js index 51f368def..ba6c42313 100644 --- a/js/id/svg/areas.js +++ b/js/id/svg/areas.js @@ -62,23 +62,28 @@ iD.svg.Areas = function(projection) { fill: areas }; + var paths = surface.selectAll('.layer-shadow, .layer-stroke, .layer-fill') + .selectAll('path.area') + .filter(filter) + .data(function(layer) { return data[layer]; }, iD.Entity.key); + + // Remove exiting areas first, so they aren't included in the `fills` + // array used for sorting below (https://github.com/systemed/iD/issues/1903). + paths.exit() + .remove(); + + var fills = surface.selectAll('.layer-fill path.area')[0]; + var bisect = d3.bisector(function(node) { return -node.__data__.area(graph); }).left; - var fills = surface.selectAll('.layer-fill path.area')[0]; - function sortedByArea(entity) { if (this.__data__ === 'fill') { return fills[bisect(fills, -entity.area(graph))]; } } - var paths = surface.selectAll('.layer-shadow, .layer-stroke, .layer-fill') - .selectAll('path.area') - .filter(filter) - .data(function(layer) { return data[layer]; }, iD.Entity.key); - paths.enter() .insert('path', sortedByArea) .each(function(entity) { @@ -94,8 +99,5 @@ iD.svg.Areas = function(projection) { paths .attr('d', path); - - paths.exit() - .remove(); }; }; diff --git a/test/spec/svg/areas.js b/test/spec/svg/areas.js index cf652ef11..e0a42725b 100644 --- a/test/spec/svg/areas.js +++ b/test/spec/svg/areas.js @@ -1,7 +1,8 @@ describe("iD.svg.Areas", function () { var surface, projection = Object, - filter = d3.functor(false); + all = d3.functor(true), + none = d3.functor(false); beforeEach(function () { surface = d3.select(document.createElementNS('http://www.w3.org/2000/svg', 'svg')) @@ -17,7 +18,7 @@ describe("iD.svg.Areas", function () { 'w': iD.Way({id: 'w', tags: {building: 'yes'}, nodes: ['a', 'b', 'c', 'a']}) }); - surface.call(iD.svg.Areas(projection), graph, [graph.entity('w')], filter); + surface.call(iD.svg.Areas(projection), graph, [graph.entity('w')], none); expect(surface.select('path.way')).to.be.classed('way'); expect(surface.select('path.area')).to.be.classed('area'); @@ -32,7 +33,7 @@ describe("iD.svg.Areas", function () { 'w': iD.Way({id: 'w', tags: {building: 'yes'}, nodes: ['a', 'b', 'c', 'a']}) }); - surface.call(iD.svg.Areas(projection), graph, [graph.entity('w')], filter); + surface.call(iD.svg.Areas(projection), graph, [graph.entity('w')], none); expect(surface.select('.area')).to.be.classed('tag-building'); expect(surface.select('.area')).to.be.classed('tag-building-yes'); @@ -46,11 +47,28 @@ describe("iD.svg.Areas", function () { .append('path') .attr('class', 'other'); - surface.call(iD.svg.Areas(projection), graph, [area], filter); + surface.call(iD.svg.Areas(projection), graph, [area], none); expect(surface.selectAll('.other')[0].length).to.equal(1); }); + it("handles deletion of a way and a member vertex (#1903)", function () { + var graph = iD.Graph({ + 'a': iD.Node({id: 'a', loc: [0, 0]}), + 'b': iD.Node({id: 'b', loc: [1, 0]}), + 'c': iD.Node({id: 'c', loc: [1, 1]}), + 'd': iD.Node({id: 'd', loc: [1, 1]}), + 'w': iD.Way({id: 'w', tags: {area: 'yes'}, nodes: ['a', 'b', 'c', 'a']}), + 'x': iD.Way({id: 'x', tags: {area: 'yes'}, nodes: ['a', 'b', 'd', 'a']}) + }); + + surface.call(iD.svg.Areas(projection), graph, [graph.entity('x')], all); + graph = graph.remove(graph.entity('x')).remove(graph.entity('d')); + + surface.call(iD.svg.Areas(projection), graph, [graph.entity('w')], all); + expect(surface.select('.area').size()).to.equal(1); + }); + describe("z-indexing", function() { var graph = iD.Graph({ 'a': iD.Node({id: 'a', loc: [-0.0002, 0.0001]}), @@ -66,30 +84,30 @@ describe("iD.svg.Areas", function () { }); it("stacks smaller areas above larger ones in a single render", function () { - surface.call(iD.svg.Areas(projection), graph, [graph.entity('s'), graph.entity('l')], filter); + surface.call(iD.svg.Areas(projection), graph, [graph.entity('s'), graph.entity('l')], none); expect(surface.select('.area:nth-child(1)')).to.be.classed('tag-landuse-park'); expect(surface.select('.area:nth-child(2)')).to.be.classed('tag-building-yes'); }); it("stacks smaller areas above larger ones in a single render (reverse)", function () { - surface.call(iD.svg.Areas(projection), graph, [graph.entity('l'), graph.entity('s')], filter); + surface.call(iD.svg.Areas(projection), graph, [graph.entity('l'), graph.entity('s')], none); expect(surface.select('.area:nth-child(1)')).to.be.classed('tag-landuse-park'); expect(surface.select('.area:nth-child(2)')).to.be.classed('tag-building-yes'); }); it("stacks smaller areas above larger ones in separate renders", function () { - surface.call(iD.svg.Areas(projection), graph, [graph.entity('s')], filter); - surface.call(iD.svg.Areas(projection), graph, [graph.entity('l')], filter); + surface.call(iD.svg.Areas(projection), graph, [graph.entity('s')], none); + surface.call(iD.svg.Areas(projection), graph, [graph.entity('l')], none); expect(surface.select('.area:nth-child(1)')).to.be.classed('tag-landuse-park'); expect(surface.select('.area:nth-child(2)')).to.be.classed('tag-building-yes'); }); it("stacks smaller areas above larger ones in separate renders (reverse)", function () { - surface.call(iD.svg.Areas(projection), graph, [graph.entity('l')], filter); - surface.call(iD.svg.Areas(projection), graph, [graph.entity('s')], filter); + surface.call(iD.svg.Areas(projection), graph, [graph.entity('l')], none); + surface.call(iD.svg.Areas(projection), graph, [graph.entity('s')], none); expect(surface.select('.area:nth-child(1)')).to.be.classed('tag-landuse-park'); expect(surface.select('.area:nth-child(2)')).to.be.classed('tag-building-yes'); @@ -105,7 +123,7 @@ describe("iD.svg.Areas", function () { graph = iD.Graph([a, b, c, w, r]), areas = [w, r]; - surface.call(iD.svg.Areas(projection), graph, areas, filter); + surface.call(iD.svg.Areas(projection), graph, areas, none); expect(surface.select('.fill')).to.be.classed('relation'); }); @@ -119,7 +137,7 @@ describe("iD.svg.Areas", function () { graph = iD.Graph([a, b, c, w, r]), areas = [w, r]; - surface.call(iD.svg.Areas(projection), graph, areas, filter); + surface.call(iD.svg.Areas(projection), graph, areas, none); expect(surface.selectAll('.stroke')[0].length).to.equal(0); }); @@ -132,7 +150,7 @@ describe("iD.svg.Areas", function () { r = iD.Relation({members: [{id: w.id, type: 'way'}], tags: {type: 'multipolygon'}}), graph = iD.Graph([a, b, c, w, r]); - surface.call(iD.svg.Areas(projection), graph, [w, r], filter); + surface.call(iD.svg.Areas(projection), graph, [w, r], none); expect(surface.selectAll('.way.fill')[0].length).to.equal(0); expect(surface.selectAll('.relation.fill')[0].length).to.equal(1); @@ -147,7 +165,7 @@ describe("iD.svg.Areas", function () { r = iD.Relation({members: [{id: w.id, type: 'way'}], tags: {type: 'multipolygon'}}), graph = iD.Graph([a, b, c, w, r]); - surface.call(iD.svg.Areas(projection), graph, [w, r], filter); + surface.call(iD.svg.Areas(projection), graph, [w, r], none); expect(surface.selectAll('.stroke')[0].length).to.equal(0); });