search

perliedman / ocad2geojson

JavaScript OCAD file reader and exporter for GeoJSON, SVG and Mapbox Style Spec
https://www.liedman.net/ocad2geojson/
GNU Affero General Public License v3.0
39 stars 4 forks source link

Importer should be more resilient to malformed OCAD files #5

Open mpickering opened 4 years ago

mpickering commented 4 years ago

It seems that all the OCAD files I have fail to load into ocad2geojson because they are technically invalid in some manner. When I load them into open orienteering mapper, there are some warnings about these issues but they are corrected somehow and things continue to work.

image

One issue in particular I ran into was that a specific element had a colour which wasn't present in the colour array, which was causing a crash when rendering the SVG.

perliedman commented 4 years ago

Hm, yes interesting. I have discovered some of these issues with OCAD files I have access to, and plugged those holes, but yes, a lot more error checking could be added.

The OCAD file reader has the concept of warnings, which is currently used when reading symbols (see https://github.com/perliedman/ocad2geojson/blob/master/src/ocad-reader/symbol-index.js#L59), but the exporters have very little in terms of general error handling.

We could probably add something similar to the exporters, so unexpected errors at least are limited to a symbol or single map object, instead of crashing the whole process.

mpickering commented 4 years ago

I think there should be an invariant that elements can only reference colours which appear in the colour array so the correct place to check for the error is in the reader as you suggest.

This particular element looked completely empty anyway and had no coordinates so I'm not sure how it ended up in the file.

perliedman commented 4 years ago

Yes, sounds like a good idea.