Clamp to 3D Tiles most detailed

[lilleyse]

This builds on the first 3D Tiles clamping PR and adds new functions for getting the most detailed height of 3D Tiles, on screen or off screen. It does this by loading the needed leaf tiles asynchronously and then calling the synchronous clampToHeight, sampleHeight functions. It returns a promise to the result.

The new functions are:

scene.pickFromRayMostDetailed (private)
scene.drillPickFromRayMostDetailed (private)
scene.sampleHeightMostDetailed
scene.clampToHeightMostDetailed

I still have some more cleanup work to do, but this is ready to experiment with. I'll update once I think it's in a good enough state to review.

@pjcozzi I'm really starting to see what you mean about moving these pick functions out of Scene. It only got worse.

To do:

• Cleanup
• CHANGES.md
• Cesium3DTileset unit tests
• Decide what to do for descendant traversal - Clamp to 3D Tiles most detailed #7115 (comment)
• Move pick code out of Scene. Might be a follow up PR.

Fixes #7120

[cesium-concierge]

Thanks for the pull request @lilleyse!

• ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

---

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

[pjcozzi]

@OmarShehata could you give this a shot to test and review once @lilleyse says its ready?

In the meantime, I'll have a look at the Sandcastle example.

[pjcozzi]

Are we sure we should call this "off screen?" What speaks to the user best? Async? Most Detailed?

[pjcozzi]

Really just that comment for now, this is cool, and could be used for profiles, e.g., change 30 to 300.

I'll also look at the implementation when ready.

[OmarShehata]

While I do support having smaller, more focused Sandcastle examples in general, in this case I wonder if it would be better to combine this with the existing Clamp to 3D Tiles example, in the same way the terrain sample and sampleMostDetailed are in the same example:

http://localhost:8080/Apps/Sandcastle/index.html?src=Terrain.html

"Most detailed" is probably the most relevant to the user, but I can imagine users running into this issue when they realize sampleHeight only works for already visible tiles and want to figure out how to do it for tiles offscreen. I think in any case the goal for the user is just clamping to 3D Tiles/getting tiles height, so a combined example geared towards that use case makes the most sense to me.

[lilleyse]

I think keeping them separate is better because they represent different enough use cases.

But maybe the new demo should be called Sample Height from 3D Tiles. The main difference being this demo samples some heights and shows the result while the first demo actually physically clamps something to the tileset.

[OmarShehata]

I think that makes more sense! I think that naming will make it easier to find.

[OmarShehata]

I just tested this out, and it seems to work pretty well! I'd say the only thing I was confused about with using it, when I copy pasted the example code to use in my own code, it took me a while to realize that it was returning a single Cartesian when I pass a single Cartesian. It seems less error-prone to me to just always take an array and return an array.

I also noticed that when I clamp a position to a tile, it sometimes forces that tile to load before the parent(s). Is that intentional? So for example, on the BIM tileset (localhost:8080/...) I can make the big tower load first by clamping a position to it.

[sherousee]

I tested scene.pickFromRayMostDetailed(), and it is fantastic. Can't wait to start using. When do you think we will see this merged and released?

[lilleyse]

I just tested this out, and it seems to work pretty well! I'd say the only thing I was confused about with using it, when I copy pasted the example code to use in my own code, it took me a while to realize that it was returning a single Cartesian when I pass a single Cartesian. It seems less error-prone to me to just always take an array and return an array.

Yeah better to be consistent here. I changed it to use arrays only.

I'm also trying to decide whether sampleHeightMostDetailed and clampToHeightMostDetailed should modify the positions in place like sampleTerrain(MostDetailed) would. I am leaning towards "yes".

So far the analogs are:

Sync functions:
Globe.getHeight (Takes single cartographic, returns height, does not modify cartographic in place)
Scene.sampleHeight (Same as above)
Scene.clampToHeight (Takes single cartesian, optional result parameter stores result (user can set it to the input cartesian if they want to modify it in place))

Async functions:
sampleTerrain (Takes array, modifies cartographic positions in place, sets .height to undefined if there was nothing to sample)
sampleTerrainMostDetailed (Same as above)
Scene.sampleHeightMostDetailed (Same as above)
Scene.clampToHeightMostDetailed (Same as above but with cartesians. Sets the array element to undefined if there was nothing to sample)

[lilleyse]

I also noticed that when I clamp a position to a tile, it sometimes forces that tile to load before the parent(s). Is that intentional? So for example, on the BIM tileset (localhost:8080/...) I can make the big tower load first by clamping a position to it.

Ah.... that's a weird edge case.

It's happening because if a tile is selected but neither it nor any of its ancestors are loaded it tries to selected any loaded descendants to help fill in the gaps. And it does this even if the tile wouldn't normally refine to those descendants. This is mainly designed for when immediatelyLoadDesiredLevelOfDetail is true and you quickly zoom out.

I have to decide whether this is worth addressing or just leaving. Possibly the descendant refinement should be reserved for when immediatelyLoadDesiredLevelOfDetail is true.

[lilleyse]

I tested scene.pickFromRayMostDetailed(), and it is fantastic. Can't wait to start using. When do you think we will see this merged and released?

Great to hear! I'm trying to finish it up now so it will have enough time to sit in master before a 1.51 release.

[lilleyse]

Added tileset tests and incorporated your suggestions @pjcozzi. This is ready for another review.

[pjcozzi]

Is the tasklist up-to-date?

CHANGES.md
Cesium3DTileset unit tests
Decide what to do for descendant traversal - #7115 (comment)
Move pick code out of Scene. Might be a follow up PR.

[pjcozzi]

@lilleyse just curious - did you try this with a point cloud?

[pjcozzi]

All looks good to me, even tested in Safari.

@lilleyse any last considerations or OK to merge?

[lilleyse]

@lilleyse just curious - did you try this with a point cloud?

I tried now and unfortunately got what I was afraid of happening - it picked through the point cloud despite requesting the high detail tiles. localhost:8080 Example.

We had ideas for fixing this, like rendering to a larger than 1x1 canvas with attenuation turned on. Is this something we should write an issue for and do later?

[lilleyse]

Decide what to do for descendant traversal - #7115 (comment)

I think I'm going to leave this one because it's more of an oddity than a legitimate bug, and only shows up in the BIM tileset because the root tile is so large and slow to download.

[lilleyse]

Opened #7247 and #7246 for cleanup related comments.

[lilleyse]

@pjcozzi The task list is updated and the code is ready. Point cloud support would be the only thing holding this up.

[pjcozzi]

@lilleyse just curious - did you try this with a point cloud?

I tried now and unfortunately got what I was afraid of happening - it picked through the point cloud despite requesting the high detail tiles. localhost:8080 Example.

We had ideas for fixing this, like rendering to a larger than 1x1 canvas with attenuation turned on. Is this something we should write an issue for and do later?

Please submit an issue.

[pjcozzi]

Is appveyor expected to fail right now?

[pjcozzi]

Is appveyor expected to fail right now?

If so, this is ready to merge.

[lilleyse]

Merged in master and CI passed.

[pjcozzi]

🚀