donmccurdy / glTF-Transform

glTF 2.0 SDK for JavaScript and TypeScript, on Web and Node.js.
https://gltf-transform.dev
MIT License
1.42k stars 150 forks source link

Invalid bounding box calculation when one of the meshes lacks a POSITION attribute. #1360

Closed timknip closed 7 months ago

timknip commented 7 months ago

Describe the bug When one or more of the meshes don't have the POSITION attribute, bounds is not calculated correctly and returns [ [ Infinity, Infinity, Infinity], [-Infinity, -Infinity, -Infinity] ] => getMeshBounds returns the initial infinite bounds if the mesh has no positions. Then the expandBounds calls here make the result an infinite bounding box.

To Reproduce Steps to reproduce the behavior:

  1. Load attached GLB
  2. Get it's bounds and check

Expected behavior A correct bounding box

min: [
    -0.45675763487815857,
    -2.7882036668004553e-8,
    -0.30479879926513953
  ],
  max: [ 
     0.4564840495586395, 
     1.8287999105357924, 
     0.28434090200747697 
  ]

Versions: 3.10.0

Some check on whether getMeshBounds returns a valid bounds seems in order. Or perhaps simply pass in the result bounds.

5093-9845273-Merit 2-Drawer Wardrobe.zip

donmccurdy commented 7 months ago

I was under the impression that the glTF spec prohibits mesh primitives from excluding the 'POSITION' attribute, but that seems to be only a warning in the validator:

CleanShot 2024-04-17 at 12 38 00@2x

I suspect there are more operations in glTF Transform that currently assume a 'POSITION' attribute, so I would recommend including one if possible. But as you said, this should probably be fixed — PRs would be welcome, or I'll take a look when I'm able!

timknip commented 7 months ago

Ok ;-) PR#1361 solves it by simply checking for valid floats

donmccurdy commented 7 months ago

Thanks! Fixed by https://github.com/donmccurdy/glTF-Transform/pull/1361.