-
Notifications
You must be signed in to change notification settings - Fork 225
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 GlobeRectangle::splitAtAntiMeridian
#848
Conversation
Not sure if I should merge this myself, but it all looks pretty straightforward. It's interesting to note that any "noise" in coordinates getting the wrong longitude value must be coming from the conversion from local coordinates to ECEF, as the the atan function used to calculate longitude must choose the correct quadrant based on the sign of the ECEF y coordinate. |
@timoore I'd say go for it, if you feel it's ready to go. If you think another reviewer could help, you can always tag others (like me) to give input too. |
Tagging myself as a reviewer to help move this one along... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good @kring !
I added a couple more tests as I was reviewing, so may want to glance at those.
Now, if a tile truly crosses the anti-meridian, this won't work quite as well. An actual meaningful portion of the tile's rectangle will be thrown away.
Do you think it's possible for this to be an observable problem right now? (with raster overlays or other features). If so, sounds worth writing an issue.
Tested with cesium-unreal as a sanity check. All good.
To observe this right now, you'd have to find a tile that crosses the anti-meridian. I don't know of any tilesets with tiles like that offhand. |
Your new tests look good, thanks! |
This new method splits a
GlobeRectangle
that crosses the anti-meridian into two globe rectangles that touch but do not cross it.This PR also uses this new method in
RasterOverlayUtilities::createRasterOverlayTextureCoordinates
in a slightly dodgy way. Consider a tile that touches the anti-meridian, such as a tile from Cesium World Terrain. Due to slight numerical noise in the model -> ECEF -> longitude/latitude transformation, vertices on that boundary may end up on the wrong side of it. For example, a tile that spans from 170 to 180 degrees longitude might have a vertex that's deemed to be at -179.99999 degrees longitude. And previously, when that happened, the results were fairly disastrous.projectRectangleSimple
would produce an invalid rectangle and all of the generated texture coordinates would be invalid.With the change in this PR, the portion of the rectangle on the "wrong" side of the anti-meridian is thrown away. Texture coordinates for any vertices that happen to land slightly on the wrong side get clamped to the valid range, which is exactly the right thing to do in this example where a Cesium World Terrain tile touches the anti-meridian.
Now, if a tile truly crosses the anti-meridian, this won't work quite as well. An actual meaningful portion of the tile's rectangle will be thrown away. But fixing that is pretty tricky, requiring that this function be able to produce multiple projected overlay rectangles for the input glTF. And in any case the end result is much better after this PR than it was before before it. After this PR, the larger portion of the tile, at least, will have correct raster overlay mapping. Previously none of it would have worked correctly.