connormanning / ept-tools

Entwine Point Tile point cloud utilities
https://entwine.io
MIT License
54 stars 27 forks source link

Invalid feature table alignment #39

Open ulrichson opened 1 year ago

ulrichson commented 1 year ago

Hello,

I was running the 3d-tiles-validator on a tileset created with ept-tools and it threw errors, mentioning that the feature table is not aligned correctly, e.g.:

{
  "type": "CONTENT_VALIDATION_ERROR",
  "path": "1-0-0-0.pnts",
  "message": "1-0-0-0.pnts caused validation errors",
  "severity": "ERROR",
  "causes": [
    {
      "type": "BINARY_INVALID_ALIGNMENT",
      "path": "1-0-0-0.pnts",
      "message": "The feature table binary must be aligned to 8 bytes",
      "severity": "ERROR"
    }
  ]
}

According to the feature table spec the start position must also be aligned:

The binary body shall start and end on an 8-byte boundary within the containing tile binary. The binary body shall be padded with additional bytes, of any value, to satisfy this requirement.

Currently, the ept-tools don't do that and thus I created a fork with following changes:

https://github.com/connormanning/ept-tools/compare/master...ulrichson:ept-tools:byte-alignment-fix?expand=1 (in these changes there is also another fix, i.e. the children array must not be empty according to the validator)

With these modifications, the 3d-tiles-validator doesn't complain.


Do you agree, that this would be the correct implementation?

I can open a PR, although tests will fail currently since they assume a different format.

connormanning commented 1 year ago

Something like this is likely correct, I remember fixing this once on a development branch I was working on internally.

This issue aside, you might consider using viewer.copc.io instead, which will translate your dataset from EPT/COPC to 3D Tiles in the browser so you are not required to translate them to 3D Tiles on disk or use an EPT Tools server - just point at your data over HTTP. See for example COPC and EPT (the Cesium links here) sample data.

I should update the readme for this repo to point at that... and I'll see if I can dig up my existing fix for this issue.

ulrichson commented 1 year ago

@connormanning thanks! I will look into it!