Multipolygon part must be inside (#139)

Author: Beakerboy

src/building.js

@@ -219,11 +219,14 @@ class Building {
       }
       // Filter all relations
       parts = this.fullXmlData.getElementsByTagName('relation');
-      for (let i = 0; i < parts.length; i++) {
-        if (parts[i].querySelector('[k="building:part"]')) {
-          const id = parts[i].getAttribute('id');
+      for (const xmlPart of parts) {
+        if (xmlPart.querySelector('[k="building:part"]')) {
+          const id = xmlPart.getAttribute('id');
           try {
-            this.parts.push(new MultiBuildingPart(id, this.fullXmlData, this.nodelist, this.outerElement.options));
+            const part = new MultiBuildingPart(id, this.fullXmlData, this.nodelist, this.outerElement.options);
+            if (this.partIsInside(part)) {
+              this.parts.push(part);
+            }
           } catch (e) {
             window.printError(e);
           }
@@ -416,7 +419,7 @@ class Building {
 
   /**
    * Check if any point in a part is within this building's outline.
-   * It only checknof points are inside, not if crossing events occur, or
+   * It only checks if points are inside, not if crossing events occur, or
    * if the part completly surrounds the building.
    * @param {BuildingPart} part - the part to be tested
    * @returns {bool} is it?
@@ -428,6 +431,8 @@ class Building {
         return true;
       }
     }
+    // @todo
+    // return BuildingShapeUtils.surrounds(this.outerElement.shape, part.center);
     return false;
   }
 }

src/buildingpart.js

@@ -22,13 +22,13 @@ class BuildingPart {
   // DOM of the building part way
   way;
 
-  // THREE.Shape of the outline.
+  /** @type {THREE.Shape} the outline. */
   shape;
 
-  // THREE.Mesh of the roof
+  /** @type {THREE.Mesh} the roof */
   roof;
 
-  // array of Cartesian coordinates of every node.
+  /** @type {Object.<string, [number, number]>} Cartesian coordinates of every node keyed by refId. */
   nodelist = [];
 
   // Metadata of the building part.
@@ -66,7 +66,7 @@ class BuildingPart {
   /**
    * @param {number} id - the OSM id of the way or multipolygon.
    * @param {XMLDocument} fullXmlData - XML for the region.
-   * @param {[[number, number],...]} nodelist - Cartesian coordinates of each node keyed by node refID
+   * @param {Object.<string, [number, number]>} nodelist - Cartesian coordinates of each node keyed by node refID
    * @param {object} options - default values for the building part.
    */
   constructor(id, fullXmlData, nodelist, defaultOptions = {}) {

test/building.test.js

@@ -315,8 +315,7 @@ test('Test downloading type=building with multipolygon outline and multiple inne
   expect(global.fetch.mock.calls[2][0]).toBe(urlBase + 'map?bbox=30.4980057,59.9380365,30.4993839,59.9385087');
 });
 
-test('Part must be within outline', () => {
-  const data = `<?xml version="1.0" encoding="UTF-8"?>
+const nonIntersectingWays = `<?xml version="1.0" encoding="UTF-8"?>
 <osm>
  <node id="1" lat="0.001" lon="0.001"/>
  <node id="2" lat="0.001" lon="0"/>
@@ -342,8 +341,73 @@ test('Part must be within outline', () => {
  </way>
 </osm>
 `;
-  expect(new Building('11', data).parts.length).toBe(0);
+
+const nonIntersectingWayAndMulti = `<?xml version="1.0" encoding="UTF-8"?>
+<osm>
+ <node id="1" lat="0.001" lon="0.001"/>
+ <node id="2" lat="0.001" lon="0"/>
+ <node id="3" lat="0" lon="0"/>
+ <node id="4" lat="0" lon=".0005"/>
+ <node id="5" lat="0" lon=".001"/>
+ <node id="6" lat=".0001" lon=".001"/>
+ <node id="7" lat=".0001" lon="0.005"/>
+ <way id="11">
+  <nd ref="1"/>
+  <nd ref="2"/>
+  <nd ref="3"/>
+  <nd ref="1"/>
+  <tag k="building" v="apartments"/>
+ </way>
+ <way id="22">
+  <nd ref="4"/>
+  <nd ref="5"/>
+  <nd ref="6"/>
+  <nd ref="7"/>
+  <nd ref="4"/>
+  </way>
+  <relation id="40">
+    <member type="way" ref="22" role="outer"/>
+    <tag k="building:part" v="yes"/>
+  </relation>
+</osm>
+`;
+
+describe.each([
+  [nonIntersectingWays, 0, 'ways non-intersecting'],
+  [nonIntersectingWayAndMulti, 0, 'multipolygon non-intersecting'],
+])('Part must be within outline', (data, expected, description) => {
+  test(`${description}`, () => {
+    expect(new Building('11', data).parts.length).toBe(expected);
+  });
+});
+
+/** Test partIsInside
+class BldgMock extends Building {
+
+  constructor() {
+    this.shape = new Shape();
+  }
+}
+
+class PartMock {
+  constructor(shape) {
+    this.shape = shape;
+  }
+}
+
+describe.each([
+  [[], false, 'Two Separate Ways'],
+  [[], true, 'Two Ways share edge'],
+  [[], false, 'multipolygon outside'],
+])('partIsInside', (shapePoints, expected, description) => {
+  test(`${description}`, () => {
+    const shape = new Shape(shapePoints.map((point) => new Vector2(...point)));
+    const part = new PartMock(shape);
+    const bldg = new BldgMock();
+    expect(bldg.partIsInside(part)).toBe(expected);
+  });
 });
+*/
 
 window.printError = printError;
 

test/multipolygon.test.js

@@ -46,6 +46,8 @@ test('Test Simple Multipolygon', () => {
   expect(errors.length).toBe(0);
 });
 
+// @todo Test a multipolygon with multiple closed outer ways.
+
 window.printError = printError;
 
 var errors = [];