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

Add Unreal API for terrain height querying #1421

Open
wants to merge 71 commits into
base: main
Choose a base branch
from
Open

Conversation

csciguy8
Copy link
Contributor

@csciguy8 csciguy8 commented May 13, 2024

Builds on cesium-native PR 783, so merge that first.

Introduces new test to verify results and measure performance of new cesium-native terrain height query functionality.
(using the existing performance test framework)

Still a bit of work to do here...

image

@kring kring marked this pull request as ready for review September 19, 2024 05:04
@kring
Copy link
Member

kring commented Sep 19, 2024

This is ready to review!

It's taking ~30 seconds to generate distance fields for built-in meshes
(not our tiles) during automated test runs on CI, and if this happens at
the wrong time it can cause async tests to time out and fail. E.g.:

```
[2024.09.19-13.55.42:741][157]LogMeshUtilities: Finished distance field build in 14.6s - 126x126x126 sparse distance field, 1.9Mb total, 0.1Mb always loaded, 52% occupied, 1322 triangles, EditorSkySphere
[2024.09.19-13.55.57:236][896]LogMeshUtilities: Finished distance field build in 14.3s - 126x126x126 sparse distance field, 1.9Mb total, 0.1Mb always loaded, 52% occupied, 1322 triangles, SM_SkySphere
[2024.09.19-13.55.58:335][ 28]LogMeshUtilities: Finished distance field build in 1.0s - 42x42x42 sparse distance field, 0.1Mb total, 0.0Mb always loaded, 96% occupied, 16 triangles, Cube
[2024.09.19-13.55.59:518][170]LogMeshUtilities: Finished distance field build in 1.2s - 42x42x42 sparse distance field, 0.1Mb total, 0.0Mb always loaded, 96% occupied, 320 triangles, Sphere
[2024.09.19-13.56.00:710][313]LogMeshUtilities: Finished distance field build in 1.0s - 42x42x42 sparse distance field, 0.1Mb total, 0.0Mb always loaded, 96% occupied, 170 triangles, Cylinder
[2024.09.19-13.56.05:147][845]LogAutomationController: Error: Test Completed. Result={Fail} Name={works with a single position} Path={Cesium.Unit.SampleHeightMostDetailed.Cesium World Terrain.works with a single position}
```
@kring
Copy link
Member

kring commented Sep 20, 2024

Or... not quite ready for review. The new tests are taking an extraordinarily long time on CI - over 30 seconds when they just take a few milliseconds locally! My best guess is that the Cesium3DTileset is either not ticking, or it's ticking much more slowly, and so the height queries are taking ages.

@kring
Copy link
Member

kring commented Sep 23, 2024

The test failures in UE 5.3 and 5.4 are caused by this previously-discovered problem:
#1219
If Actors don't tick, then terrain doesn't load. Not sure what best to do about this yet.

@kring
Copy link
Member

kring commented Sep 25, 2024

The problem mentioned above has been fixed, so this is ready to review.

@j9liu
Copy link
Contributor

j9liu commented Sep 26, 2024

This seems to work well! I'm still adding some comments but the results look great so far. In our 04_MAIN_CesiumSublevels level I tested putting a globe-anchored in every location, which involved querying against both Cesium World Terrain and Aerometrex Denver Photogrammetry. Both queries completed and resulted in accurate placement on each tileset -- nice!

Also tested what happens if you give the function a position that isn't on the tileset, and it correct returns false for the sample's success.

Here's the Blueprint I used if anyone wanted it for reference:
https://blueprintue.com/blueprint/t0mh0s4t/

I also noticed that the performance tests take between 6-10 seconds, is that expected? I thought it might have to do with the cache, but even with a warm cache it took around 5-6 seconds on my machine. Maybe I had too many browser tabs open 😛

Copy link
Contributor

@j9liu j9liu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! I didn't pore over the changes to the test code, but I verified that the tests pass on my machine and that the behavior works.

My comments are mainly doc / naming tweaks. To speed things up for the release, I'm going to implement most of them myself, but I will leave the open-ended ones up in case you have opinions @kring .

Source/CesiumRuntime/Public/CesiumSampleHeightResult.h Outdated Show resolved Hide resolved
* will have more information about the problem.
*/
UPROPERTY(BlueprintReadWrite, Category = "Cesium")
bool HeightSampled;
Copy link
Contributor

@j9liu j9liu Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar nitpick to the cesium-native comment... could this be named differently, depending on what we decide for cesium-native? Open to alternatives of course.

Suggested change
bool HeightSampled;
bool SampleSuccessful;

Source/CesiumRuntime/Public/Cesium3DTileset.h Outdated Show resolved Hide resolved
Source/CesiumRuntime/Private/Cesium3DTileset.cpp Outdated Show resolved Hide resolved
Source/CesiumRuntime/Private/Cesium3DTileset.cpp Outdated Show resolved Hide resolved
@kring
Copy link
Member

kring commented Sep 27, 2024

I also noticed that the performance tests take between 6-10 seconds, is that expected?

That doesn't sound too far out of line. The tests are doing 400 height queries, spread out over a large enough area that it requires loading quite a few tiles, especially in the GP3DT case.

@kring
Copy link
Member

kring commented Sep 27, 2024

I think I've addressed everthing, this is ready for another look.

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.

3 participants