Skip to content
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

Bounds do not seem to get generated on a created mesh... #743

Closed
andypoly opened this issue Jan 6, 2025 · 6 comments
Closed

Bounds do not seem to get generated on a created mesh... #743

andypoly opened this issue Jan 6, 2025 · 6 comments
Assignees
Labels
bug Something isn't working import Import of glTF files
Milestone

Comments

@andypoly
Copy link

andypoly commented Jan 6, 2025

Describe the bug
MeshGenerator calls GenerateMesh(),CreateMeshResultAsync which checks for "boundsAreValid".
If they are then it never calls msh.RecalculateBounds() which means post-load the mesh does not have any bounds applied (they are centre 0, size 0).

Expected behavior

Any generated meshes should have RecalculateBounds() called on them.
See https://docs.unity3d.com/6000.0/Documentation/ScriptReference/Mesh.SetSubMesh.html
It says "Note that this only applies to the bounds of the SubMesh. You must call Mesh.RecalculateBounds to recalculate the bounds of the Mesh itself."

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • glTFast version
    LATEST
  • Unity Editor version [e.g. 2021.2.1f1]
    2022.3.41
  • Render Pipeline and version [e.g. Universal Render Pipeline 12.0]
    Builtin - but NA
  • Platform: [e.g. Editor, Windows Player, iOS]
    Editor
@andypoly andypoly added the bug Something isn't working label Jan 6, 2025
@andypoly andypoly closed this as completed Jan 7, 2025
@atteneder
Copy link
Owner

Hi @andypoly ,

I was about to look into it. Is there a reason you closed it?

From the top of my head: If boundsAreValid is true, it means the bounds have been extracted successfully from the glTF file (specifically the position accessor's min/max properties) and don't have to be recalculated (which is an expensive operation).

If you found a case where this does not work properly, please report with a sample file.

Thanks

@andypoly
Copy link
Author

andypoly commented Jan 7, 2025

Ah so I closed because I thought I should be putting in https://github.com/Unity-Technologies/com.unity.cloud.gltfast instead - I am a bit unclear! I have put it there also now.

As I tried to explain the submesh bounds are indeed valid, but as the Unity docs say you still have to recalculate the bounds for the whole mesh (even if one submesh) so you can not skip that bit. I could provide a project example but not really got the time!

@atteneder
Copy link
Owner

Thanks, got it! I'll check that.

For now it doesn't matter where you post the bug issue.

@atteneder atteneder reopened this Jan 7, 2025
@andypoly
Copy link
Author

andypoly commented Jan 7, 2025

ok thanks! Note one workaround I tried is you can manually set the msh bounds instead of RecalculateBounds if that is too much.
So if all sub mesh bounds are valid just create a bounds encapsulating each sub mesh bounds as you grab them and assign it!

@atteneder atteneder self-assigned this Jan 7, 2025
@atteneder atteneder added the import Import of glTF files label Jan 7, 2025
@atteneder atteneder moved this from To do to Planned in glTFast development Jan 7, 2025
@atteneder atteneder added this to the 6.10.1 milestone Jan 7, 2025
@atteneder
Copy link
Owner

I confirmed it's broken for single-submesh meshes (for others it mysteriously works).

The fix is currently going through testing and will land in the next release.

@atteneder atteneder moved this from Planned to In progress in glTFast development Jan 8, 2025
Needle-Mirror-Bot pushed a commit to needle-mirror/com.unity.cloud.gltfast that referenced this issue Jan 9, 2025
## [6.10.1] - 2025-01-09

### Added
- Test for `ConvertBoneWeightsUInt8ToFloatInterleavedJob`
- Test for `ConvertBoneWeightsUInt16ToFloatInterleavedJob`
- *BoundsTests* which certifies correct mesh bounds.

### Changed
- Downgraded package dependencies to version bundled with Editor.
  - `com.unity.collections` to version `1.2.4` (from `1.5.1`)
  - `com.unity.mathematics` to version `1.2.6` (from `1.3.1`)
- When a position accessor lacks min/max properties, the corresponding error message is communicated via the `ICodeLogger` instead of a plain console log.

### Fixed
- Build error when used along with packages that depend on `com.unity.collections` versions older than 1.5 (e.g. Polyspatial 1.x; fixes [#730](atteneder/glTFast#730)).
- Invalid mesh bounds on meshes with one submesh (fixes [#743](atteneder/glTFast#743)).

### Removed

### Deprecated

### Security
@atteneder
Copy link
Owner

fixed in 6.10.1

@github-project-automation github-project-automation bot moved this from In progress to Done in glTFast development Jan 9, 2025
atteneder added a commit to Unity-Technologies/com.unity.cloud.gltfast that referenced this issue Jan 30, 2025
* feat(test): Added mesh bounds checks.

* fix: Invalid mesh bounds on meshes with one submesh.

fixes atteneder#743

* refactor: Removed unused parameter.

* fix: Typo

* fix: Updated number of test assets.

* fix(test): Moved *Bounds* model into dedicated folder in order to fix editor importer tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working import Import of glTF files
Projects
Archived in project
Development

No branches or pull requests

2 participants