NREL / floorspace.js

Other
68 stars 36 forks source link

Unable to create new overlapping space in simple floorplan #407

Open MeltedFreddo opened 3 years ago

MeltedFreddo commented 3 years ago

Repro steps from blank floorplan:

image

Upon clicking to complete drawing the new space, the polygon box will disappear and a console error with the message "v is undefined" will be output. If you save the floorplan and try and reopen it, the console will error again with the message "A vertex is referenced by a face, but does not exist!"

Reason for failure:

Within the function createFaceFromPoints the new space geometry is stored in a variable faceGeometry. The edges and vertices from this variable form the basis of later variables newEdges and newVertices. However prior to calling storeFace, function calls to replaceFacePoints for the spaces that need to be redrawn to accomodate the new space cause many if not all of these vertices to be created in the currentStoryGeometry under a different id.

storeFace itself calls replaceFacePoints which is able to detect that the vertices already exist and should be reused, however the original vertex ids remain untouched in the newVertices array. This means that when splitEdges is called, some edges will be created containing references to vertices that have not been saved to currentStoryGeometry.

Potential fix:

Ensure that the newEdges and newVertices arrays only contain references to valid edges and vertices. I put this block of code just before the call to storeFace which resolved the issue but probably doubles up on some of the duplicate-checking code further down the line:

` const generatedVertices = currentStoryGeometry.vertices.slice(oldVertexLength);

const replacementVertIds = _.chain(newVertices)
.map((vert) => {
  const gVert = _.find(generatedVertices, v => distanceBetweenPoints(v, vert) < (currentProjectSpacing / 20));
  if (!gVert) return null; // this vertex doesn't match any existing ones
  _.remove(generatedVertices, v => v.id === gVert.id); //remove it from the list as we already have those coords in newVertices
  if (vert.id === gVert.id) return null;  // this vertex already exists
  return [vert.id, gVert.id]; // this vertex has been generated by replaceFacePoints so use the generated id
})
.compact()
.fromPairs()
.value();  

newVertices = newVertices.map(v => ({
...v,
id: replacementVertIds[v.id] || v.id,
}));
newEdges = newEdges.map(e => ({
  ...e,
  v1: replacementVertIds[e.v1] || e.v1,
  v2: replacementVertIds[e.v2] || e.v2
  }));

newEdges = newEdges.concat(currentStoryGeometry.edges.slice(oldEdgeLength));
//add any generated vertices that did not need superimposition on newVertices
newVertices.concat(generatedVertices);`