nytimes / three-loader-3dtiles

This is a Three.js loader module for handling OGC 3D Tiles, created by Cesium. It currently supports the two main formats, Batched 3D Model (b3dm) - based on glTF Point cloud.
Other
439 stars 62 forks source link

fix getPositionFromLatLongHeight to not convert degress to radians #143

Closed tobias74 closed 3 months ago

tobias74 commented 4 months ago

The function tileset.ellipsoid.cartographicToCartesian(...) takes latitude and longitude in degrees, so there should not be a conversion to radians. This function is also called (correctly) in orientToGeocoord(...) with latitude and longitude in degrees.

Avnerus commented 4 months ago

Hi @tobias74! Thank you for all of the contributions, looks great. I left one question on the review, but looks like you are still pushing commits. Could you let me know when the PR is ready for review?

tobias74 commented 4 months ago

Hi @Avnerus!

This PR got a little bigger than originally planned :-) but I think it is ready now. In case I come up with more stuff I would just implement it on a new branch.

In case there are things that I should adjust please let me know :-)

tobias74 commented 4 months ago

This is the link to the attribution guidelines: https://developers.google.com/maps/documentation/tile/create-renderer#display-attributions

tobias74 commented 4 months ago

The build seems to be failing now but I cannot see the details, it only shows a 404: "We are sorry, the resource you requested cannot be found."

Avnerus commented 4 months ago

Thank you @tobias74. Regarding the attribution data, I think I'll add it in onTraversalComplete() similar to here. We also need to make sure that it is optional so that it will not run when it is not needed. I will update those changes.

Regarding the change to the reset and wgs84 transformation, could you describe your use-case? Are you manipulating the default transformation matrix of Google's 3D Tiles?

tobias74 commented 4 months ago

My usecase is a virtual indoor cycling simulator. I need to call "orientToGeocoord(...)" at certain intervals and transform the whole tileset to keep the rider close to the origin of the coordinate system to prevent those "single precision floating point" artefacts.

Concerning the attribution data, I can make those adjustments putting the functionality behind a flag in the options and run it in "tileset3d.options.onTraversalComplete". I will update this PR accordingly :-)

Avnerus commented 3 months ago

My usecase is a virtual indoor cycling simulator. I need to call "orientToGeocoord(...)" at certain intervals and transform the whole tileset to keep the rider close to the origin of the coordinate system to prevent those "single precision floating point" artefacts.

I see! Interesting use case.

Concerning the attribution data, I can make those adjustments putting the functionality behind a flag in the options and run it in "tileset3d.options.onTraversalComplete". I will update this PR accordingly :-)

If you haven't already, I wrote earlier that I could make the changes. I checked your PR into this branch so I'm following up from there. I would also like to make the semantics of the transformation VS reset more clear.

Avnerus commented 3 months ago

@tobias74 Please check the latest changes to the dev branch. They include your commits and my updates.

Avnerus commented 3 months ago

@tobias74 Check also this commit. Instead of making changes to all meshes with individual options, I added one contentPostProcess callback where you can make any change you want to the tile content after they are parsed.