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

GlobeControls limit Frustum with horizon for performance #483

Merged
merged 11 commits into from
Feb 19, 2024

Conversation

AlaricBaraou
Copy link
Contributor

Currently for simplicity GlobeControls camera far is far enough to include the entire earth resulting in unnecessary tiles being loaded.

With these changes we limit the frustum to the distance of the horizon based on our current latitude and elevation.

This seems to work well but some tiles are being loaded while they don't intersect the frustum.

I edited the googleMapsExample.js to visualize it.

I'm not sure where is the issue but I'm pretty sure it's either the frustum or the OOB.
This particular example might help identifying the issue, when the camera is zoomed in on the north pole we get false positive on the intersectsFrustum check.
If we look at it from the side, it's considered like an intersection as it seems like the frustum far limit does intersect with the obbs.
wrongFrustumCheck
But if we look at it from above, we can see that the frustum doesn't.
wrongFrustumCheckFromAbove

Those false positive unload as soon as on the side view, the frustum move outside of those obb.

@gkjohnson
Copy link
Contributor

Awesome! I'll take a deeper look at this next week. It would be nice to have some before / after stats on the number of tiles loaded and number of tiles rendered after the changes you've made so far.

I'm not sure where is the issue but I'm pretty sure it's either the frustum or the OOB.
This particular example might help identifying the issue, when the camera is zoomed in on the north pole we get false positive on the intersectsFrustum check.

I took a look and the issue is in the OBB frustum intersect code. In fact three.js' AABB frustum intersect code that the OBB logic is based on isn't correct, either. It will mark bounds as in the frustum even when they're clearly outside of it since it's only checking if the box is completely on one side of any plane which misses the common case of a box that intersects multiple planes if it's large. Here's a demo:

image

https://jsfiddle.net/89rtuaqp/2/

This post discusses the problem and a correct application of separating axis theorem. I'll make a three.js issue. We can see what they say - we may have to make our own extended frustum class that stores enough data for performing a more correct OBB intersection test.

@gkjohnson
Copy link
Contributor

mrdoob/three.js#27756 may take a bit to resolve so I think it's best for us to modify our current implementation to be more robust, but I think that can happen in another PR. This can be done by storing the frustum corners as points on the Frustum so they can be tested against the OBB planes as referenced in the inigo post.

@AlaricBaraou
Copy link
Contributor Author

Good timing, I was working on a custom Frustum but it's hard to test it with the tiles logic, I might try in a simpler environment.

I added a class AdvFrustum that calculate the points of the Frustum to use them for different operations as suggested in the article you linked. But I think there are still case that are not handled or it might be the logic of the nested tiles etc that I don't really grasp at the moment.

Also the latest commit take care of "Google Maps Example: Add ability to specify height in hash" #482

@AlaricBaraou
Copy link
Contributor Author

a simpler example to play with it

https://jsfiddle.net/0sb3gden/30/

The extended Frustum works well with axis aligned box but not with OBB at the moment.

@AlaricBaraou
Copy link
Contributor Author

AlaricBaraou commented Feb 18, 2024

I was about to write that it's good now but apparently now we have some false negative.

Looking good here outside of the fact that the tiles bounding box are bigger than necessary but that's not from this PR
frustumOk

But here it discard some tiles that are in obvious intersections.
frustumNOk

I`ll try to reproduce that one in jsfiddle
edit: yes it creates a false negative on this one
https://jsfiddle.net/627tdm3e/174/
I'll investigate tomorrow.

edit edit: two of my box planes are wrong, I'll fix it

Copy link
Contributor

@gkjohnson gkjohnson left a comment

Choose a reason for hiding this comment

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

It looks like we're getting some depth fighting issues - but it looks like this was already happening 🤔

image

We can look at this in another PR but we might want to find a smarter way to set the "near" value.

@gkjohnson gkjohnson marked this pull request as ready for review February 19, 2024 08:48
Copy link
Contributor

@gkjohnson gkjohnson left a comment

Choose a reason for hiding this comment

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

Added some comments for some small things to change - and a few questions on the example and GlobeControls additions.

@AlaricBaraou
Copy link
Contributor Author

I addressed all points, just a note about this issue #482
It can't be init under a certain height, at least in the area I was testing it.
I added a comment to check later

@gkjohnson
Copy link
Contributor

It can't be init under a certain height, at least in the area I was testing it.

Likely what's happening is that the coarse levels of detail on the model load and have a much higher height than the lower LoDs which pushes the camera up due to the "GlobeControls" update function. I think it's okay to say that this fixes #482.

@gkjohnson
Copy link
Contributor

gkjohnson commented Feb 19, 2024

A few stats - some very nice improvements!

Top Down View

Before After % improvement
visible tiles 104 55 ~47%
loaded tiles 307 189 ~38%
image

Horizon View

Before After % improvement
visible tiles 332 186 ~43%
loaded tiles 574 376 ~34%
image

@gkjohnson
Copy link
Contributor

One thing I did notice is that the globe controls far plane will clip the horizon a bit. It's subtle but you can see it change a little. Likely due to the earth model not exactly matching the ellipsoid. We don't have to fix it immediately but may something to keep in mind for the future there are ideas on how to address it.

Before

image

After
image

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.

2 participants