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

BatchedMesh: Exception with InterleavedBufferAttribute #27973

Closed
marwie opened this issue Mar 22, 2024 · 6 comments · Fixed by #27978
Closed

BatchedMesh: Exception with InterleavedBufferAttribute #27973

marwie opened this issue Mar 22, 2024 · 6 comments · Fixed by #27978
Milestone

Comments

@marwie
Copy link
Contributor

marwie commented Mar 22, 2024

Description

Batched mesh addGeometry throws exception for geometry with InterleavedBufferAttributes

Reproduction steps

  1. checkout https://github.com/needle-tools/three.js/tree/batched-mesh-interleaved-buffer
  2. run webgl_mesh_batch example
  3. observe error

The mesh was processed with gltf-transform and simplified

weld: primitive[0] of mesh "Sphere" → tolerance: 0.0001
weld: Tolerance thresholds: POSITION=0.0001, NORMAL=0.05, TANGENT=0.05, TEXCOORD_0=0.0001, TEXCOORD_1=0.0001
weld: 515 → 515 (+0.00%) vertices.
simplify: primitive[0] of mesh "Sphere" → ratio: 0.0625, error: 0.3, lockBorder: true
simplify: 515 → 92 (–82.14%) vertices, error: 0.1558.
sparse: Updated 0 accessors.
sparse: Complete.

Code

see 54b78a0

Sphere.zip

Live example

I have to submit this another time if necessary

Screenshots

image
image

image

Version

162

Device

Desktop

Browser

Chrome

OS

Windows

@marwie
Copy link
Contributor Author

marwie commented Mar 22, 2024

Here's a bit more info. Seems like when creating a new instance of the InterleavedBufferAttribute class the passed in interleavedBuffer field is actually the array data

@marwie
Copy link
Contributor Author

marwie commented Mar 22, 2024

A quick hack in _initializeGeometry works needle-tools@fa7e823

Maybe someone can explain why the arrays are created with new array.constructor and suggest a better(?) solution

But the beautiful lowres sphere renders:
image

@gkjohnson
Copy link
Collaborator

These lines are the problematic ones:

const dstAttribute = new srcAttribute.constructor( dstArray, itemSize, normalized );
dstAttribute.setUsage( srcAttribute.usage );

It looks like these lines were added in #25059 but I don't think there shouldn't be any reason to ensure the data is using an InterleavedBufferAttribute if the original geometry is. Likewise I don't believe there's any reason to retain the "usage" setting from the original attribute.

new srcAttribute.constructor should be able to be able to be changed to new BufferAttribute and the setUsage line can be removed, if you'd like to try that out and make a PR.

Maybe someone can explain why the arrays are created with new array.constructor and suggest a better(?) solution

This is done to retain the original precision used for the attribute data and should not change.

marwie added a commit to needle-tools/three.js that referenced this issue Mar 23, 2024
@marwie
Copy link
Contributor Author

marwie commented Mar 23, 2024

Thanks - that seems to work. I'll do some more testing and then open a PR. Thank you

marwie added a commit to needle-tools/three.js that referenced this issue Mar 23, 2024
@marwie
Copy link
Contributor Author

marwie commented Mar 23, 2024

Is it expected that the batched mesh looks like this when enabling wireframe on the material?

image

marwie added a commit to needle-tools/three.js that referenced this issue Mar 23, 2024
marwie added a commit to needle-tools/three.js that referenced this issue Mar 23, 2024
@gkjohnson
Copy link
Collaborator

gkjohnson commented Mar 23, 2024

Is it expected that the batched mesh looks like this when enabling wireframe on the material?

It's hard to say without knowing what the original geometry looks like. I'm not so familiar with how wireframes work internally but it's possible BatchedMesh needs special handling.

@donmccurdy donmccurdy changed the title BatchedMesh exception with InterleavedBufferAttribute BatchedMesh: Exception with InterleavedBufferAttribute Mar 23, 2024
Mugen87 pushed a commit that referenced this issue Mar 24, 2024
marwie added a commit to needle-tools/three.js that referenced this issue Mar 24, 2024
marwie added a commit to needle-tools/three.js that referenced this issue Mar 24, 2024
@Mugen87 Mugen87 closed this as completed Mar 25, 2024
@Mugen87 Mugen87 added this to the r163 milestone Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants