NASA-AMMOS / 3DTilesRendererJS

Renderer for 3D Tiles in Javascript using three.js
https://nasa-ammos.github.io/3DTilesRendererJS/example/bundle/mars.html
Apache License 2.0
1.53k stars 276 forks source link

Points Cloud 3D Tiles are not displayed correctly. #182

Closed RomTreg closed 3 years ago

RomTreg commented 3 years ago
Screenshot 2021-05-26 at 23 21 41

tileset.json.zip

I've attached tiles.json and screenshot just in case . All tiles are displayed at the center and change their orientation apparently. Any ideas on how to fix this?

gkjohnson commented 3 years ago

Hi @RomTreg! It's very possible it's a bug in the way point clouds are handled considering we've mostly use this with mesh data. We don't have the bandwidth to fix every issue associated with the project right now but I might be able to point to where the issue might be if you'd like to help make a PR and get it fixed (if it is an issue with the library and not generated tileset).

Are you able to provide a full tileset with pnts files instead of just the tileset.json? It's not possible to see the issue otherwise. Some more information that might help as well:

Thanks for the report!

RomTreg commented 3 years ago

Thank you for a quick response.

Answering to your question:

  1. Yes, I will provide image of the Tileset and Tileset itself
  2. Tileboxes are visualised aside from the model. Actually we experienced the same issue with textured models, but changing coordinates in root to 0,0,0 solve the issue
  3. We generate tile sets with Metashape (agisoft)
  4. We tested it with cesium and it works, but we work with threejs and do not plan to switch to cesium at the moment
RomTreg commented 3 years ago

@gkjohnson , everything has been sent to your email. thanks!

gkjohnson commented 3 years ago

Thanks for sending it over -- when I load one of the pnts files in the pntsExample.html page I see the following warning in the console:

PNTSLoader.js:37 PNTSLoader: Unsupported FeatureTable feature "RTC_CENTER" detected.

Which indicates that the pnts files you have are using a feature that we haven't added support for yet and would explain why the loaded points are not offset. Would you like to add the feature in a PR? The docs on the feature are available here -- effectively it specifies an offset for all the points defined in the file.

You can see how RTC_CENTER has already been implemented for B3DMLoader here, so it should be a similar and simple addition. Note that the RTC_CENTER offset is applied to the scene object rather than the points themselves to avoid floating point errors on the GPU.

Let me know if you need any more clarification! I'd appreciate the help

RomTreg commented 3 years ago

Thanks

We will take a look.

I will let you know or get back with further questions

On 28 May 2021, at 09:27, Garrett Johnson @.***> wrote:

Thanks for sending it over -- when I load one of the pnts files in the pntsExample.html page https://nasa-ammos.github.io/3DTilesRendererJS/example/bundle/pntsExample.html I see the following warning in the console:

PNTSLoader.js:37 PNTSLoader: Unsupported FeatureTable feature "RTC_CENTER" detected. Which indicates that the pnts files you have are using a feature that we haven't added support for yet and would explain why the loaded points are not offset. Would you like to add the feature in a PR? The docs on the feature are available here https://github.com/CesiumGS/3d-tiles/tree/master/specification/TileFormats/PointCloud#rtc_center -- effectively it specifies an offset for all the points defined in the file.

You can see how RTC_CENTER has already been implemented for B3DMLoader here https://github.com/NASA-AMMOS/3DTilesRendererJS/blob/master/src/three/B3DMLoader.js#L55-L62, so it should be a similar and simple addition. Note that the RTC_CENTER offset is applied to the scene object rather than the points themselves to avoid floating point errors on the GPU.

Let me know if you need any more clarification! I'd appreciate the help

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NASA-AMMOS/3DTilesRendererJS/issues/182#issuecomment-850174968, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALBCIJI7P5EW2PBOA7AN5D3TP4ZT3ANCNFSM45SZVGOQ.

alex-lancer commented 3 years ago

Hi! I'm in @RomTreg team and worked over RTC correction for PNTSLoader. I tried to implement it like in B3DMLoader and added these coordinates to newly created Point object. (src/three/PNTSLoader.js:60)

    const rtcCenter = featureTable.getData( 'RTC_CENTER' );

    if ( rtcCenter ) {

        result.scene.position.x += rtcCenter[ 0 ];
        result.scene.position.y += rtcCenter[ 1 ];
        result.scene.position.z += rtcCenter[ 2 ];

    }

But this did not lead to a significant improvement in the result.

Screenshot (1)

RomTreg commented 3 years ago

However as i see, it fixes bounding boxes placement:))

Thanks, Roman Tregubov

On 28 May 2021, at 21:57, alex-lancer @.***> wrote:

 Hi! I'm in @RomTreg team and worked over RTC correction for PNTSLoader. I tried to implement it like in B3DMLoader and added these coordinates to newly created Point object. (src/three/PNTSLoader.js:60)

` const rtcCenter = featureTable.getData( 'RTC_CENTER' );

if ( rtcCenter ) {

  result.scene.position.x += rtcCenter[ 0 ];
  result.scene.position.y += rtcCenter[ 1 ];
  result.scene.position.z += rtcCenter[ 2 ];

} ` But this did not lead to a significant improvement in the result.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

gkjohnson commented 3 years ago

I've looked over the 3d tiles spec again and done a quick test and it seems that this section of code (intended to address this gltf up adjustment) should not apply to the PNTS files, so you'll want to wrap that in an if ( extension === 'pnts' ) condition.

I'll make another issue for it in a bit but it's possible that this means that the transform is being incorrectly applied for CMPT and maybe I3DM files, as well, but without test cases it's hard to verify.

Give the above suggestion a try in addition to the RTC_CENTER snippet -- the model should line correctly.

alex-lancer commented 3 years ago

I have applied you advice(with some changes) and it works now! I have wrapped with if ( extension !== 'pnts' ) this lines Can I make a PR? We are using 3DTilesRenderer as npm dependency so it will be great to have this fix with package update.

gkjohnson commented 3 years ago

Can I make a PR? We are using 3DTilesRenderer as npm dependency so it will be great to have this fix with package update.

Absolutely! We can get it merged and the published to npm.

RomTreg commented 3 years ago

Thanks, Garrett, for your help and for the great product!

Thanks, Roman Tregubov

On 29 May 2021, at 22:47, Garrett Johnson @.***> wrote:

 Can I make a PR? We are using 3DTilesRenderer as npm dependency so it will be great to have this fix with package update.

Absolutely! We can get it merged and the published to npm.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

gkjohnson commented 3 years ago

Thanks guys! I'll try to get this published later this week. Let us know if you guys run into any other issues!

gkjohnson commented 3 years ago

I published the fix for this that you guys made in v0.2.11 today -- thanks again for you help!

RomTreg commented 3 years ago

Great!!

Thanks, Roman Tregubov

On 4 Jun 2021, at 00:16, Garrett Johnson @.***> wrote:

 I published the fix for this that you guys made in v0.2.11 today -- thanks again for you help!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.