CesiumGS / 3d-tiles-tools

Apache License 2.0
313 stars 47 forks source link

Avoid duplicate storage of batch- and feature IDs #124

Closed javagl closed 6 months ago

javagl commented 6 months ago

Fixes https://github.com/CesiumGS/3d-tiles-tools/issues/123

As noted in the issue: When the GLB in a B3DM re-used the same batch ID accessor for multiple primitives, then the conversion into feature IDs did cause duplicated storage of IDs. For example, when the input contained

Accessors: 2
Mesh 0 of 1
  Primitive 0 of 4
Attributes:  { _BATCHID: 0 }
  Primitive 1 of 4
Attributes:  { BATCHID: 0 }
  Primitive 2 of 4
Attributes:  { _BATCHID: 1 }
  Primitive 3 of 4
Attributes:  { BATCHID: 1 }

then the output contained

Accessors:  6
Mesh 0 of 1
  Primitive 0 of 4
Attributes:  { _FEATURE_ID_0: 4 }
  Primitive 1 of 4
Attributes:  { BATCHID: 0, _FEATURE_ID_0: 1 }
  Primitive 2 of 4
Attributes:  { _FEATURE_ID_0: 5 }
  Primitive 3 of 4
Attributes:  { BATCHID: 2, _FEATURE_ID_0: 3 }

This PR fixes this by keeping track of the accessors that have already been converted from batch ID into feature ID, and properly disposing of the former batch ID accessors, resulting in this:

Accessors:  2
Mesh 0 of 1
  Primitive 0 of 4
Attributes:  { _FEATURE_ID_0: 0 }
  Primitive 1 of 4
Attributes:  { _FEATURE_ID_0: 0 }
  Primitive 2 of 4
Attributes:  { _FEATURE_ID_0: 1 }
  Primitive 3 of 4
Attributes:  { _FEATURE_ID_0: 1 }

There are no tests yet, but the output above has been created with the following snippet, which will be converted into a test soon:

import { Accessor, Document } from "@gltf-transform/core";
import { TileFormats } from "./src/tilesets";
import { GltfTransform, TileFormatsMigrationB3dm } from "./src/tools";

async function createDocument() {
  const document = new Document();
  const buffer = document.createBuffer();

  const mesh = document.createMesh();

  // Create two accessors that will be used as BATCHID
  // acccessors in numltiple primitives
  const accessor0 = document.createAccessor();
  accessor0.setBuffer(buffer);
  accessor0.setType(Accessor.Type.SCALAR);
  const ids0 = [0, 1, 0, 1, 0, 1, 0, 1, 0];
  accessor0.setArray(new Int16Array(ids0));

  const accessor1 = document.createAccessor();
  accessor1.setBuffer(buffer);
  accessor1.setType(Accessor.Type.SCALAR);
  const ids1 = [0, 1, 0, 1, 0, 1, 0, 1, 0];
  accessor1.setArray(new Int16Array(ids1));

  // Create 4 primitives:

  // The first two primitives refer to the first accessor, as
  // the  "_BATCHID" and the (legacy) "BATCHID" attribute
  const primitive0a = document.createPrimitive();
  primitive0a.setAttribute("_BATCHID", accessor0);
  mesh.addPrimitive(primitive0a);

  const primitive0b = document.createPrimitive();
  primitive0b.setAttribute("BATCHID", accessor0);
  mesh.addPrimitive(primitive0b);

  // The first two primitives refer to the second accessor, as
  // the  "_BATCHID" and the (legacy) "BATCHID" attribute
  const primitive1a = document.createPrimitive();
  primitive1a.setAttribute("_BATCHID", accessor1);
  mesh.addPrimitive(primitive1a);

  const primitive1b = document.createPrimitive();
  primitive1b.setAttribute("BATCHID", accessor1);
  mesh.addPrimitive(primitive1b);

  return document;
}

async function createB3dmBuffer(document: Document) {
  const io = await GltfTransform.getIO();
  const inputGlbData = await io.writeBinary(document);
  const featureTableJson = {
    BATCH_LENGTH: 2,
  };
  const featureTableBinary = Buffer.alloc(0);
  const batchTableJson = {};
  const batchTableBinary = Buffer.alloc(0);

  const inputB3dmTileData = TileFormats.createB3dmTileDataFromGlb(
    Buffer.from(inputGlbData),
    featureTableJson,
    featureTableBinary,
    batchTableJson,
    batchTableBinary
  );
  const b3dmBuffer = TileFormats.createTileDataBuffer(inputB3dmTileData);
  return b3dmBuffer;
}

async function printInfo(document: Document) {
  const io = await GltfTransform.getIO();
  const jsonDocument = await io.writeJSON(document);
  const json = jsonDocument.json;

  console.log("Accessors: ", json.accessors?.length);

  const meshes = json.meshes ?? [];
  for (let m = 0; m < meshes.length; m++) {
    console.log("Mesh " + m + " of " + meshes.length);
    const mesh = meshes[m];
    const primitives = mesh.primitives ?? [];
    for (let p = 0; p < primitives.length; p++) {
      console.log("  Primitive " + p + " of " + primitives.length);
      const primitive = primitives[p];
      const attributes = primitive.attributes;
      console.log("Attributes: ", attributes);
    }
  }
}

async function runMigrationTest() {
  const inputDocument = await createDocument();
  console.log("Input document:");
  printInfo(inputDocument);

  const io = await GltfTransform.getIO();
  const b3dmBuffer = await createB3dmBuffer(inputDocument);
  const outputGlb = await TileFormatsMigrationB3dm.convertB3dmToGlb(b3dmBuffer);
  const outputDocument = await io.readBinary(outputGlb);

  console.log("Output document:");
  printInfo(outputDocument);

  console.log("done");
}

runMigrationTest();
javagl commented 6 months ago

Note: The current build failures are due to the differences in the "golden" reference files. Comparing the current "golden" output to the new output essentially shows that the PR has the desired effect: There is one fewer warning for a "potentially unused accessor" in the new output. The golden reference files will be updated accordingly when the specific tests are added here.