3DStreet / 3dstreet-editor

3DStreet Editor Repo
https://3dstreet.app
Other
18 stars 3 forks source link

fix incorrect boundingBox #383

Closed Algorush closed 6 months ago

Algorush commented 6 months ago

Vincent Fretin from Aframe advise to remove the old function Box3.prototype.expandByObject to correctly define boundingBox: https://github.com/aframevr/aframe-inspector/pull/686 I did this in the 3DStreet-editor repository in this PR and now the boundingBox is defined correctly for pedestrians and for almost all other objects... except the tram. I don't know why yet. It’s interesting that in a clean scene with two objects - a tram and a pedestrian, the boundingBox is determined correctly for both

netlify[bot] commented 6 months ago

Deploy Preview for 3dstreet-editor-builds ready!

Name Link
Latest commit 150367d61d4d363ed474ec3a1e5f516d04f00544
Latest deploy log https://app.netlify.com/sites/3dstreet-editor-builds/deploys/65a4895dee76ec0008b83697
Deploy Preview https://deploy-preview-383--3dstreet-editor-builds.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

kfarr commented 6 months ago

Thanks @Algorush. Which tram? If it is this model then you can ignore as we will be replacing: <a-asset-item id="trolleymodel" src="${assetUrl}objects/godarvilletram.gltf"></a-asset-item>

Algorush commented 6 months ago

No, this one: https://github.com/3DStreet/3dstreet/blob/c4104fec3a83461f453f751e3324a2f9c19a2546/src/assets.js#L186

image

kfarr commented 6 months ago

@Algorush Ok thanks well in that case yes we should support correct bounding box for that model.

I just tested this and while I wasn't able to test animation, things work as expected (nothing is broken) for bounding box behavior so I'll merge

vincentfretin commented 5 months ago

@Algorush you may want my other change as well https://github.com/aframevr/aframe-inspector/pull/686/commits/ea6fb76500f90ecdfcc94d648ddfca17c664735e

Algorush commented 5 months ago

@Algorush you may want my other change as well aframevr/aframe-inspector@ea6fb76

@vincentfretin Thank you very much for mention about this