search

NREL / floorspace.js

Other
66 stars 35 forks source link

Space deform occasionally #301

Closed fsudaman closed 4 years ago

fsudaman commented 6 years ago

Our engineer had encountered the following issue a few times. When drawing geometries a space will suddenly and unexpectedly deform occasionally. It seems as though the geometry is generated incorrectly, but the issue is not visible until the relevant line is modified in some way. Please see the attached PDF that demonstrates the issue observed and OSM file to reproduce the problem.

New Geometry Test - Bug Report.pdf

New geometry test.zip

bgschiller commented 6 years ago

Note to self: It's possible that "canonicalizing" the geometry after any drawing action could go a long way towards fixing this issue (and others).

Try adding the tests for this, using OpenStudio's idea of 'circular equals': same vertex positions, but potentially wrapping at a different point.

Even if we can't get the canonicalization done in time, the tests could be useful for a future implementer.

shorowit commented 6 years ago

Was playing around with the tool and created a reproducible deformity. Open the attached file and make a drawing change as indicated in the video.

deformity.json.txt FloorspaceJS.webm.txt

Results in this: image

bgschiller commented 4 years ago

I downloaded Scott's deformity.json file and saw that, according to the current version of floorspace, Space 1-2 is already invalid at the time of import. (We don't warn about this, but you can check by calling the validateFaceGeometry function manually). The error is:

error: "An edge is being touched by a vertex on the same face at (50, 45)"

To me, this indicates that we're catching this issue upstream. It would be impossible to create deformity.json with today's version of floorspace. I believe there are other bugs where floorspace incorrectly raises similar error messages. Those are false positives of the validateFaceGeometry function. However, we are now correctly identifying this invalid case, so I think it's safe to close this issue.