DassaultSystemes-Technology / dspbr-pt

A WebGL2 Enterprise PBR sample renderer
Apache License 2.0
140 stars 19 forks source link

Support for non-indexed and interleaved geometry #32

Closed donmccurdy closed 3 years ago

donmccurdy commented 3 years ago

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

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

donmccurdy commented 3 years ago

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