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

b3dm samples - TilesetWithRequestVolume - city invalid? #256

Closed
bertt opened this issue Dec 20, 2022 · 4 comments
Closed

b3dm samples - TilesetWithRequestVolume - city invalid? #256

bertt opened this issue Dec 20, 2022 · 4 comments

Comments

@bertt
Copy link

bertt commented Dec 20, 2022

Hi,

When validating the b3dm's in https://github.com/CesiumGS/3d-tiles-samples/tree/main/1.0/TilesetWithRequestVolume/city, I noticed that ul.b3dm and ll.b3dm are not valid:

   "type": "CONTENT_VALIDATION_ERROR",
      "path": "ll.b3dm",
      "message": "ll.b3dm caused validation errors",
      "severity": "ERROR",
      "causes": [
        {
          "type": "BINARY_INVALID_ALIGNMENT",
          "path": "ll.b3dm",
          "message": "The byte length must be aligned to 8 bytes",
          "severity": "ERROR"
        }
      ]
    }

Looks like the glb byteLength is not aligned to an 8-byte boundary in these cases (ll.b3dm - 9700 bytes, ul.b3dm - 9684 bytes).

Question: Is the validator correct in these cases? If so how to fix the b3dm when the glb byteLength is not aligned to an 8-byte boundary?

I tried to fix this validator error by adding padding but then I got an other warning:

       "causes": [
            {
              "type": "CONTENT_VALIDATION_WARNING",
              "message": "Extra data after the end of GLB stream.",
              "severity": "WARNING"
            }
          ]

It would be helpful to have valid b3dm sample files.

@javagl
Copy link
Contributor

javagl commented Jan 3, 2023

Just a short ping for now:

The validator is correct here. There are some data sets that cause validation issues which have only been detected with the new validator. These data sets include unit test- and sample data in CesiumJS, as well as some of the older sample models in the 3d-tiles-samples repository. (We recently started addressing these issues, e.g. by fixing the spec data via CesiumGS/cesium#10852, but we plan to also apply similar fixes to all other tilesets that are publicly available).

For the ll.b3dm in particular: This B3DM file was originally intended to be "just some dummy object". Unfortunately, it appears in many samples and tests (and ... sometimes the file sizes of these "instances" are different - I think none of them is really "valid" in terms of the alignment requirements).

I just did some experiments, with some preliminary, in-flight results shown in PaddingIssues-2023-01-03.zip. It contains 4 versions of a dummy tileset that refers to the ll.b3dm:

  • A, the original one, where the 3D Tiles Validator complains: "The byte length must be aligned to 8 bytes"
  • B, with additional 0-bytes at the end of the B3DM, where the glTF validator complains: "Extra data after the end of GLB stream."
  • C, with additional 0-bytes at the end of the BIN chunk of the GLB, where the glTF validator complains: "GLB-stored BIN chunk contains 4 extra padding byte(s)."
  • D, with additional -bytes (spaces) at the end of the JSON chunk of the GLB, which passes validation.

It is... unfortunate that it is not trivially possible to wrap a GLB into a B3DM without modifying the GLB itself, on a level that one normally does not even have access to (when somebody is just using some existing GLB file or GLB writer). We're currently investigating possible solutions for this issue.

@bertt
Copy link
Author

bertt commented Jan 4, 2023

Ok thanks!

The whole padding rule system feels a bit academic to me, resulting in lots of 'invalid' tilesets that show up in clients anyway. Maybe it's an option to get rid of it?

@javagl
Copy link
Contributor

javagl commented Jan 4, 2023

A short update: From the options mentioned above, I think that only option B is a reasonable one. Options C and D would require modifying the GLB data in a way that should not be required by someone who just wants to "wrap" a GLB into a B3DM. Option B should be easy to implement for clients (and there's already a draft PR for covering this in the validator). But it may require a clarification in the specification. The suggestion for that is tracked at CesiumGS/3d-tiles#729

@javagl
Copy link
Contributor

javagl commented Jan 10, 2023

This should be fixed via #258 and will be part of the next release. It now implements the validation according to the clarifications in the spec that have been added via CesiumGS/3d-tiles#729 : It is possible to insert padding bytes at the end of the B3DM data (to ensure that its length is divisible by 8). These padding bytes are added after the GLB payload, but will be ignored in the validation of the GLB payload.

@javagl javagl closed this as completed Jan 10, 2023
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