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

Support for non-indexed and interleaved geometry #32

Closed
donmccurdy opened this issue Nov 15, 2021 · 4 comments
Closed

Support for non-indexed and interleaved geometry #32

donmccurdy opened this issue Nov 15, 2021 · 4 comments

Comments

@donmccurdy
Copy link

donmccurdy commented Nov 15, 2021

The renderer currently fails silently if given the following types of input geometry:

  • non-indexed (or mixed index and non-indexed?)
  • interleaved attributes
  • mixed component types (e.g. some positions uint8, others uint16)

The error seems to be related to the mergeBufferGeometries step, which displays an error in the console. I'm not sure if it's possible to proceed without merging? three.js does have a mergeVertices utility that can help with creating an index. The other two (de-quantizing, de-interleaving) would require some custom code here.

I'm wondering if it would be an option to pre-process the input glTF file with gltf-transform, to normalize the structure for the path tracer, like:

import { WebIO, VertexLayout } from '@gltf-transform/core';
import { ALL_EXTENSIONS } from '@gltf-transform/extensions';
import { weld, dequantize } from '@gltf-transform/functions';

const io = new WebIO()
  .registerExtensions(ALL_EXTENSIONS)
  .setVertexLayout(VertexLayout.SEPARATE);

// Read.
const document = io.readBinary(inputGLB /* ArrayBuffer */);

// Process.
await document.transform(
  weld(),
  dequantize()
);

// Write.
const outputGLB = io.writeBinary(document);

^the dequantize function doesn't yet exist, but I could add it if that's of interest.

@bsdorra
Copy link
Member

bsdorra commented Nov 17, 2021

Merging is currently necessary. The BVH generation and the renderer expect a continuous, homogenous and non-indexed representation of the scenes geometry. Not very flexible or memory efficient, but it reduces the complexity when fetching the triangle data from a sampler in the pathtracing shader.
Using mergeBufferGeometries (including its limitations) is the result of my laziness to write a proper custom translation, happy to get rid of it.

gltf-transform as preprocess to transform the data into the expected form sounds great. But, as I need to disentangle the interleaved meshes anyways, I could also add dequantization... Conversely, does (de-)interleaving (vertex-)data sound like a useful functionality to have in gltf-transform?

@donmccurdy
Copy link
Author

Conversely, does (de-)interleaving (vertex-)data sound like a useful functionality to have in gltf-transform?

That's already available, added because some loaders don't support interleaved data. three.js supports interleaving of course, but certain packages (including three-mesh-bvh, until recently) don't, and so the option is configurable when writing a document with gltf-transform:

import { WebIO, VertexLayout } from '@gltf-transform/core';
import { ALL_EXTENSIONS } from '@gltf-transform/extensions';

const io = new WebIO()
  .registerExtensions(ALL_EXTENSIONS)
  .setVertexLayout(VertexLayout.SEPARATE); // INTERLEAVED (default) or SEPARATE

const outputGLB = io.writeBinary(document);

Perhaps there's a better term for it than 'separate'. 😅

bsdorra added a commit that referenced this issue Nov 18, 2021
Fixes two of the three problems reported in #32
Loader now supports
- interleaved attributes
- non-indexed (or mixed index and non-indexed?)
- TODO: dequantization
@bsdorra
Copy link
Member

bsdorra commented Nov 19, 2021

Added gltf-transform to pre-process the input. Works as expected.
Thanks for the report, and gltf-transform of course! ;) I'll open a new issue to track "support" for KHR_mesh_quantization

@bsdorra bsdorra closed this as completed Nov 19, 2021
@donmccurdy
Copy link
Author

Awesome, that was quick! I think the dequantize() function will be a pretty quick addition. I've started a PR in donmccurdy/glTF-Transform#431.

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

No branches or pull requests

2 participants