Fix replacement refinement with empty tiles

[lilleyse]

For #3241

I tried another approach at solving replacement refinement when empty tiles are involved. selectTiles is now back to its original state for the most part. The new idea is to find the descendant tiles as a pre-process in loadTileset. Then at the end of selectTiles the refining tiles check if all their descendant tiles were actually selected this frame. If not, the refining tile is selected and its descendants are deselected. So it's more of a reactive approach, which is actually safer because I noticed some edge cases with the previous approach.

The main drawback is in order to "deselect" a tile, I added a boolean in Cesium3DTile called selected. There's probably a better way to do this, but I was worried that modifying the selectedTiles array would cause a performance hit.

One final thing, I experimented with removing the tilesetContent flag because it doesn't serve much of a purpose anymore, except to speed up traversal slightly for those nodes. It works with or without it, but I kept it in for now.

[pjcozzi]

Thanks @lilleyse, this approach is good with me - and sounds cleaner.

@e-andersson do you want to do an initial review?

[lilleyse]

Also just a note for reviewing, it's a lot clearer to look at 3cf51b5 alone.

[e-andersson]

This comment can be removed now, doesn't apply anymore

[e-andersson]

This looks pretty good. No particular comments except for the those two comments above. The approach seems sound, but I haven't tried running it yet. (Will try it, but may not be able to get to it today.)

[pjcozzi]

Thanks for the reivew, @e-andersson!

[pjcozzi]

Perhaps mention in the comment that this is computed at runtime, not loaded from tileset.json.

Also, as we discussed before, but now that you have more context, would it be useful for tileset.json to have any metadata for this, e.g., number of descendants with content?

[lilleyse]

I don't think any metadata would help right now, unless it could link to those descendants directly, which doesn't seem possible.

[pjcozzi]

Do all the tests pass for you in this branch? They don't for me, but the failures are inconsistent. I think it is timing or my machine.

[pjcozzi]

That's all my comments.

[lilleyse]

Do all the tests pass for you in this branch? They don't for me, but the failures are inconsistent. I think it is timing or my machine.

There were some failing tests that I just fixed, and one or two tests that don't seem related. The debugFreezeFrame test still fails because of the new selected bool in Cesium3DTileset being set to false automatically. I suppose Cesium3DTile could know if debugFreezeFrame is true and not set selected to false. In general I think the selected boolean is not great, and would rather do something else.

[pjcozzi]

We should fix these tests before merging. Perhaps instead of using a selected boolean, we can track the last frame number.

[lilleyse]

I organized it differently such that debugFreezeFrame works correctly. The tests all pass now.

[pjcozzi]

JSHint fails:

Specs/Scene/Cesium3DTilesetSpec.js
  line 520  col 22  'root' is already defined.

[lilleyse]

Oops. Fixed that.

[pjcozzi]

Looks good. Will also merge this into the doc branch now.

[e-andersson]

@lilleyse Have you tested this in the viewer with some data, and if so, what data? It seems to me that we're back at the problem where blank space is shown when the tileset content tile is selected and has not yet loaded the json data. Have any other important changes to how the tileset metadata is handled been made? Perhaps I've missed something else, though looking at the spec I think our datasets are compliant.

On a side note, it seems the data (tileset2.json, tileset3.json) in Specs/Data/Cesium3DTiles/Tilesets/TilesetOfTilesets/ are missing geometricError in the top level, which is marked required in the schema.