Skip to content

Commit

Permalink
fix(functions): Create required TRS attributes to apply quantize() to…
Browse files Browse the repository at this point in the history 
… EXT_mesh_gpu_instancing (#1586)
  • Loading branch information
@donmccurdy
donmccurdy authored Feb 8, 2025
1 parent 1f37fac commit cd51e6c
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 8 deletions.
40 changes: 32 additions & 8 deletions packages/functions/src/quantize.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,13 +200,13 @@ export function quantize(_options: QuantizeOptions = QUANTIZE_DEFAULTS): Transfo
}

function quantizePrimitive(
doc: Document,
document: Document,
prim: Primitive | PrimitiveTarget,
nodeTransform: VectorTransform<vec3>,
options: Required<QuantizeOptions>,
): void {
const isTarget = prim instanceof PrimitiveTarget;
const logger = doc.getLogger();
const logger = document.getLogger();

for (const semantic of prim.listSemantics()) {
if (!isTarget && !options.pattern.test(semantic)) continue;
Expand Down Expand Up @@ -280,7 +280,7 @@ function getNodeTransform(volume: bbox): VectorTransform<vec3> {
}

/** Applies corrective scale and offset to nodes referencing a quantized Mesh. */
function transformMeshParents(doc: Document, mesh: Mesh, nodeTransform: VectorTransform<vec3>): void {
function transformMeshParents(document: Document, mesh: Mesh, nodeTransform: VectorTransform<vec3>): void {
const transformMatrix = fromTransform(nodeTransform);
for (const parent of mesh.listParents()) {
if (!(parent instanceof Node)) continue;
Expand All @@ -297,13 +297,13 @@ function transformMeshParents(doc: Document, mesh: Mesh, nodeTransform: VectorTr

const batch = parent.getExtension<InstancedMesh>('EXT_mesh_gpu_instancing');
if (batch) {
parent.setExtension('EXT_mesh_gpu_instancing', transformBatch(batch, nodeTransform));
parent.setExtension('EXT_mesh_gpu_instancing', transformBatch(document, batch, nodeTransform));
continue;
}

let targetNode: Node;
if (isParentNode || isAnimated) {
targetNode = doc.createNode('').setMesh(mesh);
targetNode = document.createNode('').setMesh(mesh);
parent.addChild(targetNode).setMesh(null);
animChannels
.filter((channel) => channel.getTargetPath() === WEIGHTS)
Expand Down Expand Up @@ -333,21 +333,34 @@ function transformSkin(skin: Skin, nodeTransform: VectorTransform<vec3>): Skin {
}

/** Applies corrective scale and offset to GPU instancing batches. */
function transformBatch(batch: InstancedMesh, nodeTransform: VectorTransform<vec3>): InstancedMesh {
function transformBatch(document: Document, batch: InstancedMesh, nodeTransform: VectorTransform<vec3>): InstancedMesh {
if (!batch.getAttribute('TRANSLATION') && !batch.getAttribute('ROTATION') && !batch.getAttribute('SCALE')) {
return batch;
}

batch = batch.clone(); // quantize() does cleanup.
const instanceTranslation = batch.getAttribute('TRANSLATION')?.clone();

let instanceTranslation = batch.getAttribute('TRANSLATION')?.clone();
const instanceRotation = batch.getAttribute('ROTATION')?.clone();
const instanceScale = batch.getAttribute('SCALE')?.clone();
let instanceScale = batch.getAttribute('SCALE')?.clone();

const tpl = (instanceTranslation || instanceRotation || instanceScale)!;

const T_IDENTITY = [0, 0, 0] as vec3;
const R_IDENTITY = [0, 0, 0, 1] as vec4;
const S_IDENTITY = [1, 1, 1] as vec3;

// Transformed batch may now require instance translation or scale.
// See: https://github.com/donmccurdy/glTF-Transform/issues/1584

if (!instanceTranslation && nodeTransform.offset) {
instanceTranslation = document.createAccessor().setType('VEC3').setArray(makeArray(tpl.getCount(), T_IDENTITY));
}

if (!instanceScale && nodeTransform.scale) {
instanceScale = document.createAccessor().setType('VEC3').setArray(makeArray(tpl.getCount(), S_IDENTITY));
}

const t = [0, 0, 0] as vec3;
const r = [0, 0, 0, 1] as vec4;
const s = [1, 1, 1] as vec3;
Expand Down Expand Up @@ -608,3 +621,14 @@ function fromTransform(transform: VectorTransform<vec3>): mat4 {
function clamp(value: number, range: vec2): number {
return Math.min(Math.max(value, range[0]), range[1]);
}

function makeArray(elementCount: number, initialElement: vec2 | vec3 | vec4) {
const elementSize = initialElement.length;
const array = new Float32Array(elementCount * elementSize);

for (let i = 0; i < elementCount; i++) {
array.set(initialElement, i * elementSize);

This comment has been minimized.

Sign in to view

Copy link
@kzhsw

kzhsw Feb 10, 2025

Contributor

Is copyWithin an option here?
Should be able to make it faster for large arrays.

function makeArray(elementCount: number, initialElement: vec2 | vec3 | vec4) {
    const elementSize = initialElement.length;
    const array = new Float32Array(elementCount * elementSize);
    
    // Set the initial element at the start of the array
    array.set(initialElement, 0);
    
    let currentLength = 1;
    while (currentLength < elementCount) {
        // Calculate how many elements we can copy in this iteration (up to 2^i)
        const copyLength = Math.min(currentLength, elementCount - currentLength);
        // Copy existing elements to fill the next chunk
        array.copyWithin(
            currentLength * elementSize,  // Target position
            0,                            // Source start
            copyLength * elementSize      // Source end (exclusive)
        );
        currentLength += copyLength;
    }
    
    return array;
}

This comment has been minimized.

Sign in to view

Copy link
@donmccurdy

donmccurdy Feb 10, 2025

Author Owner

It seems for loops are hard to beat!

https://jsbench.me/iem6z6x3og/

Mayyybe results could be different in Node.js but I'm guessing not.

Is this something you'd want to do a PR for?

This comment has been minimized.

Sign in to view

Copy link
@kzhsw

kzhsw Feb 11, 2025

Contributor

https://jsbench.me/mvm6zzxf3q/1

It is faster in browsers, since using set is O(n) js calls and using copyWithin can make it O(logn) calls

This comment has been minimized.

Sign in to view

Copy link
@donmccurdy

donmccurdy Feb 12, 2025

Author Owner

That does win out over a for loop, nice!

https://jsbench.me/2dm72atf0p/

Is this a meaningful optimization for glTF files you're working with? The O(logn) approach is not as simple as I might prefer, but if it's a measurable improvement for existing files I'm open to that.
}

return array;
}
5 changes: 5 additions & 0 deletions packages/functions/test/quantize.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,11 @@ test('instancing', async (t) => {
[12.5, 12.5, 0, 13.5, 13.5, 1],
'batch translation includes quantization transform',
);
t.deepEqual(
Array.from(batch.getAttribute('SCALE').getArray()),
[2.5, 2.5, 2.5, 2.5, 2.5, 2.5],
'batch scale added to attributes',
);
});

test('volumetric materials', async (t) => {
Expand Down

0 comments on commit cd51e6c

Please sign in to comment.