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

External tilesets are disallowed inside multiple contents #11960

Closed
javagl opened this issue Apr 26, 2024 · 4 comments
Closed

External tilesets are disallowed inside multiple contents #11960

javagl opened this issue Apr 26, 2024 · 4 comments

Comments

@javagl
Copy link
Contributor

javagl commented Apr 26, 2024

The title basically says it all: When using multiple contents that refer to external tilesets, then a message

Error: External tilesets are disallowed inside multiple contents

is printed and the external tilesets are not rendered.

This might be a leftover from the 3DDILES_multiple_contents extension: This explicitly disallowed external tilesets in the contents (see https://github.com/CesiumGS/3d-tiles/pull/583/files ). But in 3D Tiles 1.1, there is no such restriction.

Attached a really minimal test case of one tileset that only has two contents, which in turn are (external) tilesets, to reproduce the issue:

external tilesets in multiple contents.zip

@javagl
Copy link
Contributor Author

javagl commented Dec 4, 2024

I did have a short look at this. And I pragmatically created a test case:

  • a tileset with a root that as 4 contents
  • the first content is a red unit cube GLB
  • the other contents are three external tilesets (that refer to the green, blue, and white content, respectively)

multipleContentsExternal 2024-12-04b.zip

Then, I simply commented out the check for external tilesets in multiple contents, and it works:

Cesium Multiple External 0001

Yay 🎉

But ... there is a flag tile.hasTilesetContent. This is a boolean that is set for tiles that have external tilesets as content. Now ... one could be tempted to think that the flag still can to true here, because after all, this tile does have tileset content. But it also has non-tileset content 🤔

Searching for this flag reveals several places where this is checked. Among them are things like this...

/**
 * @private
 * @param {Cesium3DTileset} tileset
 * @param {Cesium3DTile} tile
 * @returns {number}
 */
function getPriorityReverseScreenSpaceError(tileset, tile) {
  const parent = tile.parent;
  const useParentScreenSpaceError =
    defined(parent) &&
    (!tileset.isSkippingLevelOfDetail ||
      tile._screenSpaceError === 0.0 ||
      parent.hasTilesetContent ||
      parent.hasImplicitContent);
  const screenSpaceError = useParentScreenSpaceError
    ? parent._screenSpaceError
    : tile._screenSpaceError;
  return tileset.root._screenSpaceError - screenSpaceError;
}

Can I be sure that setting this flag to true will not cause a bug here? No. Similarly for all the other places where this flag is queried (although most of them at least look like they might be easier to understand ... and interestingly, this flag is always checked in tile.hasTilesetContent || tile.hasImplicitContent - never alone)

I can read more, and do more tests, but there's always the risk that I miss a case - like the one where the _priorityReverseScreenSpaceError is computed and used.

So if someone can say from the tip of the head of whether it's safe to simply allow external tilesets in multiple contents (and set hasTilesetContent=true), that could safe a lot of time...

@ggetz ggetz removed the JTC label Dec 6, 2024
@ggetz
Copy link
Contributor

ggetz commented Dec 6, 2024

@javagl Good point, this could have implications on tileset refinement, similar to the issue that came up here: #9356

I haven't thoroughly checked that the case you bring up here is safe to change, but I would start by looking through Cesium3DTilesetTraversal.js to confirm.

@javagl
Copy link
Contributor Author

javagl commented Dec 6, 2024

@ggetz That thoroughness is the main difficulty when trying to address this. It might be that a bug shows up when one tile has two children of which one is empty and the other has two contents where one is a GLB and the other is an external tileset that is an implicit tilest that has a geometric error that is smaller than the geometric error of the parent tile and 'skipLevelOfDetail' is set to true ...?! 🥴 ?! The point is: The "toy test" that I created doesn't say much...

The only approach that I can imagine (and that's still on my TODO list) is to search for all uses of "hasTilesetContent", see how this flag affects that part of the code, and then try to understand the code to make somewhat educated guesses about possible implications (maybe by intentionally constructing test cases that "provoke" a difference between hasTilesetContent being true/false).

@javagl
Copy link
Contributor Author

javagl commented Feb 1, 2025

This should be fixed now via #12440

@javagl javagl closed this as completed Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants