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

Handle URI with spaces in I3DM #277

Merged
merged 3 commits into from
Jun 12, 2023
Merged

Handle URI with spaces in I3DM #277

merged 3 commits into from
Jun 12, 2023

Conversation

javagl
Copy link
Contributor

@javagl javagl commented Jun 11, 2023

Addresses #276 :

Until now, the validator handled the case that the URI inside an I3DM was padded with 0x00 (zero) characters. But the specification says

Otherwise, if the glTF field is a UTF-8 string, it shall be padded with trailing Space characters (0x20) to satisfy alignment requirements of the tile, which shall be removed at runtime before requesting the glTF asset.

So the validator has to handle the case that the URI is padded with 0x20 (space) characters.

One question is still open: Should the validator consider it as an ERROR when the URI (i.e. the payload) is padded with 0x00 characters instead of spaces?

Right now, it silently allows that. But sticking to the specification text, I'd say "Yes, this should be an error!".

If this is confirmed, I'd update this PR to treat this as an error (and this aspect has to be reviewed in the 3d-tiles-tools in the context of CesiumGS/3d-tiles-tools#45 )

@javagl
Copy link
Contributor Author

javagl commented Jun 11, 2023

(Note: Depending on the last decision here, the change log still has to be updated)

@javagl javagl marked this pull request as draft June 12, 2023 13:22
@javagl
Copy link
Contributor Author

javagl commented Jun 12, 2023

Treating trailing 0x00 bytes as an ERROR would potentially cause existing (formerly "valid") files to become invalid. This may be done in the future, but only after carefully evaluating the implications.

For now, the change from this PR is:

  • Any 0x00 (zero) bytes in the URI buffer cause a WARNING
  • Trailing 0x20 (space) bytes are properly trimmed

Fixes #276

@javagl javagl marked this pull request as ready for review June 12, 2023 17:09
@lilleyse lilleyse merged commit 0e4c443 into main Jun 12, 2023
@lilleyse lilleyse deleted the handle-spaces-in-i3dm-uri branch June 12, 2023 17:31
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

Successfully merging this pull request may close these issues.

2 participants