Skip to content

Commit c07bc96

Browse files
BeakerboyBeakerboy
authored
Do not allow self intersecting ways to be included in ring formation. (#104)
1 parent e8abc08 commit c07bc96

File tree

2 files changed

+115
-47
lines changed

2 files changed

+115
-47
lines changed

‎src/extras/BuildingShapeUtils.js‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,24 @@ class BuildingShapeUtils extends ShapeUtils {
8181
* @return {[DOM.Element]} array of closed ways.
8282
*/
8383
static combineWays(ways) {
84+
const validWays = [];
85+
86+
for (const way of ways) {
87+
if (BuildingShapeUtils.isSelfIntersecting(way)) {
88+
const id = way.getAttribute('id');
89+
const msg = 'Way ' + id + ' is self-intersecting';
90+
window.printError(msg);
91+
} else {
92+
const i = 3 + 'q';
93+
validWays.push(way);
94+
}
95+
}
96+
8497
const closedWays = [];
8598
const wayBegins = {};
8699
const wayEnds = {};
87100

88-
ways.forEach(w => {
101+
validWays.forEach(w => {
89102
const firstNodeID = w.querySelector('nd').getAttribute('ref');
90103
if (wayBegins[firstNodeID]) {
91104
wayBegins[firstNodeID].push(w);
@@ -142,7 +155,7 @@ class BuildingShapeUtils extends ShapeUtils {
142155
return [];
143156
}
144157

145-
ways.forEach(w => {
158+
validWays.forEach(w => {
146159
const wayID = w.getAttribute('id');
147160
if (usedWays.has(wayID)){
148161
return;

‎test/combine_ways.test.js‎

Lines changed: 100 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -13,70 +13,125 @@ import { Shape } from 'three';
1313
import { BuildingShapeUtils } from '../src/extras/BuildingShapeUtils.js';
1414
// import { JSDOM } from 'jsdom';
1515

16-
test('Test no combining necessary. one open way', () => {
17-
var way = '<way id="1"><nd ref="1"/><nd ref="2"/><nd ref="3"/><nd ref="4"/></way>';
18-
let parser = new window.DOMParser();
19-
let xml = parser.parseFromString(way, 'text/xml').getElementsByTagName('way')[0];
20-
let result = BuildingShapeUtils.combineWays([xml]);
21-
expect(result.length).toBe(0);
16+
beforeEach(() => {
17+
errorMsgs = [];
2218
});
2319

24-
describe('Combine Ways', () => {
25-
test.each([
20+
describe.each([
21+
[
22+
['<way id="1"><nd ref="1"/><nd ref="2"/><nd ref="3"/><nd ref="4"/></way>',
23+
], 0, 0, [], 'Single Open Way',
24+
],
25+
[
2626
[
27-
[
28-
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
29-
'<way id="2"><nd ref="2"/><nd ref="3"/></way>',
30-
'<way id="3"><nd ref="3"/><nd ref="1"/></way>',
31-
], 1, 4, 'Test combining 3 ways 1->2->3',
32-
],
27+
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
28+
'<way id="2"><nd ref="2"/><nd ref="3"/></way>',
29+
'<way id="3"><nd ref="3"/><nd ref="1"/></way>',
30+
], 1, 4, [], 'Test combining 3 ways 1->2->3',
31+
],
32+
[
3333
[
34-
[
35-
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
36-
'<way id="2"><nd ref="3"/><nd ref="1"/></way>',
37-
'<way id="3"><nd ref="2"/><nd ref="3"/></way>',
38-
], 1, 4, 'Test combining 3 ways 2->1->3',
39-
],
34+
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
35+
'<way id="2"><nd ref="3"/><nd ref="1"/></way>',
36+
'<way id="3"><nd ref="2"/><nd ref="3"/></way>',
37+
], 1, 4, [], 'Test combining 3 ways 2->1->3',
38+
],
39+
[
4040
[
41-
[
42-
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
43-
'<way id="2"><nd ref="3"/><nd ref="2"/></way>',
44-
'<way id="3"><nd ref="3"/><nd ref="1"/></way>',
45-
], 1, 4, 'Test combining tip to tip',
46-
],
41+
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
42+
'<way id="2"><nd ref="3"/><nd ref="2"/></way>',
43+
'<way id="3"><nd ref="3"/><nd ref="1"/></way>',
44+
], 1, 4, [], 'Test combining tip to tip',
45+
],
46+
[
4747
[
48-
[
49-
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
50-
'<way id="2"><nd ref="1"/><nd ref="3"/></way>',
51-
'<way id="3"><nd ref="2"/><nd ref="3"/></way>',
52-
], 1, 4, 'Test combining tail to tail',
53-
],
48+
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
49+
'<way id="2"><nd ref="1"/><nd ref="3"/></way>',
50+
'<way id="3"><nd ref="2"/><nd ref="3"/></way>',
51+
], 1, 4, [], 'Test combining tail to tail',
52+
],
53+
[
5454
[
55-
[
56-
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
57-
'<way id="2"><nd ref="3"/><nd ref="4"/></way>',
58-
'<way id="3"><nd ref="4"/><nd ref="1"/></way>',
59-
'<way id="4"><nd ref="2"/><nd ref="3"/></way>',
60-
], 1, 5, 'Test combining 4 ways',
61-
],
55+
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
56+
'<way id="2"><nd ref="3"/><nd ref="4"/></way>',
57+
'<way id="3"><nd ref="4"/><nd ref="1"/></way>',
58+
'<way id="4"><nd ref="2"/><nd ref="3"/></way>',
59+
], 1, 5, [], 'Test combining 4 ways',
60+
],
61+
[
6262
[
63-
[
64-
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
65-
'<way id="2"><nd ref="3"/><nd ref="2"/></way>',
66-
], 0, 0, 'Test combining 2 open ways',
67-
],
68-
])('${description}', (ways, length, nodes, description) => {
63+
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
64+
'<way id="2"><nd ref="3"/><nd ref="2"/></way>',
65+
], 0, 0, [], 'Test combining 2 open ways into one open way',
66+
],
67+
[
68+
[
69+
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
70+
'<way id="2"><nd ref="3"/><nd ref="2"/></way>',
71+
'<way id="3"><nd ref="4"/><nd ref="5"/></way>',
72+
], 0, 0, [], 'Test combining 3 open ways into 2 open ways',
73+
],
74+
[
75+
[
76+
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
77+
'<way id="2"><nd ref="2"/><nd ref="4"/></way>',
78+
'<way id="3"><nd ref="2"/><nd ref="3"/></way>',
79+
'<way id="4"><nd ref="1"/><nd ref="3"/></way>',
80+
], 1, 4, [], 'Combining 4 open ways into 1 closed & 1 remaining open way',
81+
],
82+
[
83+
[
84+
'<way id="1"><nd ref="1"/><nd ref="2"/><nd ref="3"/></way>',
85+
'<way id="2"><nd ref="3"/><nd ref="5"/><nd ref="1"/></way>',
86+
'<way id="3"><nd ref="3"/><nd ref="4"/><nd ref="1"/></way>',
87+
], 1, 5, [], 'Dealing with amiguity. Only make one closed way',
88+
],
89+
//[
90+
// [
91+
// '<way id="1"><nd ref="1"/><nd ref="2"/><nd ref="3"/></way>',
92+
// '<way id="2"><nd ref="3"/><nd ref="4"/><nd ref="2"/><nd ref="5"/><nd ref="1"/></way>',
93+
// ], 0, 0, [], 'Closed way is self intersecting',
94+
//],
95+
[
96+
[
97+
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
98+
'<way id="2"><nd ref="2"/><nd ref="3"/><nd ref="4"/><nd ref="5"/><nd ref="3"/><nd ref="1"/></way>',
99+
], 0, 0, ['Way 2 is self-intersecting'], 'Open way is self intersecting',
100+
],
101+
[
102+
[
103+
'<way id="1"><nd ref="1"/><nd ref="2"/></way>',
104+
'<way id="2"><nd ref="2"/><nd ref="3"/><nd ref="4"/><nd ref="5"/><nd ref="3"/><nd ref="1"/></way>',
105+
'<way id="3"><nd ref="2"/><nd ref="3"/><nd ref="1"/></way>',
106+
], 1, 4, ['Way 2 is self-intersecting'], 'Open way is self intersecting, but ring formed',
107+
],
108+
])('Combine Ways', (ways, length, nodes, errors, description) => {
109+
test(`${description}`, () => {
69110
let parser = new window.DOMParser();
70111
const xml = [];
71112
for (const way of ways){
72113
xml.push(parser.parseFromString(way, 'text/xml').getElementsByTagName('way')[0]);
73114
}
74115
let result = BuildingShapeUtils.combineWays(xml);
75116
expect(result.length).toBe(length);
117+
expect(errorMsgs.length).toBe(errors.length);
118+
if (errors.length) {
119+
for (const error of errors) {
120+
expect(errorMsgs.shift()).toBe(error);
121+
}
122+
}
76123
if (length) {
77124
expect(BuildingShapeUtils.isClosed(result[0]));
78125
expect(BuildingShapeUtils.isSelfIntersecting(result[0])).toBe(false);
79126
expect(result[0].getElementsByTagName('nd').length).toBe(nodes);
80127
}
81128
});
82129
});
130+
131+
window.printError = printError;
132+
133+
var errorMsgs = [];
134+
135+
function printError(txt) {
136+
errorMsgs.push(txt);
137+
}

0 commit comments

Comments
 (0)
⚡