search

asmaloney / libE57Format

Library for reading & writing the E57 file format
Boost Software License 1.0
137 stars 66 forks source link

Fix 32-bit build issues #234

Closed asmaloney closed 1 year ago

asmaloney commented 1 year ago

Fixes #232

SunBlack commented 1 year ago
  • I realize that the 32-bit check I have in ReaderImpl.cpp will silently drop points if there are more than 4,294,967,295 in a single scan. This seems unlikely (impossible?). There is no mechanism to return partial success, so instead of throwing an exception I decided to just drop them since it's such an extreme edge case.

Is it possible to throw a warning somehow? Or you could set a flag (as I'm not using your API, just another of my team, I don't know how the API is currently designed).

asmaloney commented 1 year ago

Is it possible to throw a warning somehow?

Unfortunately this is an API I inherited and it doesn't have warnings or flags of any kind - I can throw an exception, I can just return false, or I can process as much as possible. I could add a flag system, but I don't think I want to get into that right now. In the meantime I output a warning to the console if we are truncating.

I think the error handling could use a look - like why doesReaderImpl::ReadData3D function just return false on invalid parameters? But this isn't something I'm going to tackle now - maybe for 4.0 if there's any interest in/contributions to v3.0.

I'm also not really that keen on spending a whole bunch of time on 32-bit to be honest. Do you think anyone will be trying to deal with > 4,294,967,295 points in one scan (especially anyone building this as 32-bit)? (I don't think I've compiled anything for 32-bit in over a decade.)

SunBlack commented 1 year ago

Is it possible to throw a warning somehow?

Unfortunately this is an API I inherited and it doesn't have warnings or flags of any kind - I can throw an exception, I can just return false, or I can process as much as possible. I could add a flag system, but I don't think I want to get into that right now. In the meantime I output a warning to the console if we are truncating.

Indeed, a hint on the console is enough. It is only about the fact that you can somehow notice that truncation has taken effect there.

I'm also not really that keen on spending a whole bunch of time on 32-bit to be honest. Do you think anyone will be trying to deal with > 4,294,967,295 points in one scan (especially anyone building this as 32-bit)? (I don't think I've compiled anything for 32-bit in over a decade.)

We have some point clouds with more points, but they were also just composed of several tiles. From there: Yup, unlikely that you get such large E57 files - and if then, unlikely that you work only with 32-bit system.

asmaloney commented 1 year ago

E57 allows N (where N is "zero or more") scans in each file, so as long as they are each < 4,294,967,295 points (on 32-bit build) we should be fine. They can always avoid using this "Simple API" and use the full API directly if that's a problem. Anyone trying to process that much data on a 32-bit build... needs to invest in new hardware/software. 😆

Thanks @SunBlack.