Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Importer should be more resilient to malformed OCAD files #5

Open
mpickering opened this issue Apr 4, 2020 · 3 comments
Open

Importer should be more resilient to malformed OCAD files #5

mpickering opened this issue Apr 4, 2020 · 3 comments

Comments

@mpickering
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

Yes, sounds like a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants