Merge pull request #113 from CesiumGS/fix-combine-with-external-trans…

…forms

Fix combine with external transforms

specs/data/combineTilesets/externalsWithTransform/README.md

@@ -0,0 +1,10 @@
+A test for the `combine` functionality involving external tilesets
+that have a root transform that is not identity.
+
+A regression test for https://github.com/CesiumGS/3d-tiles-tools/issues/112
+
+Each external tileset refers to a unit cube (0,0,0)-(1,1,1).
+
+The `externalR` refers to a red cube and has a translation of 3 in x-direction.
+The `externalG` refers to a green cube and has a translation of 4 in y-direction.
+The `externalB` refers to a blue cube and has a translation of 5 in z-direction.

specs/data/combineTilesets/externalsWithTransform/externalB.json

@@ -0,0 +1,21 @@
+{
+  "asset" : {
+    "version" : "1.1"
+  },
+  "geometricError" : 4.0,
+  "root" : {
+    "boundingVolume" : {
+      "box" : [ 0.5,-0.5,0.5,0.5,0,0,0,-0.5,0,0,0,0.5 ]
+    },
+    "geometricError" : 2.0,
+    "content": {
+      "uri": "unitCubeB.glb"
+    },
+    "transform": [ 
+      1, 0, 0, 0,
+      0, 1, 0, 0,
+      0, 0, 1, 0,
+      0, 0, 5, 1
+    ]
+  }
+}

specs/data/combineTilesets/externalsWithTransform/externalG.json

@@ -0,0 +1,21 @@
+{
+  "asset" : {
+    "version" : "1.1"
+  },
+  "geometricError" : 4.0,
+  "root" : {
+    "boundingVolume" : {
+      "box" : [ 0.5,-0.5,0.5,0.5,0,0,0,-0.5,0,0,0,0.5 ]
+    },
+    "geometricError" : 2.0,
+    "content": {
+      "uri": "unitCubeG.glb"
+    },
+    "transform": [ 
+      1, 0, 0, 0,
+      0, 1, 0, 0,
+      0, 0, 1, 0,
+      0, 4, 0, 1
+    ]
+  }
+}

specs/data/combineTilesets/externalsWithTransform/externalR.json

@@ -0,0 +1,21 @@
+{
+  "asset" : {
+    "version" : "1.1"
+  },
+  "geometricError" : 4.0,
+  "root" : {
+    "boundingVolume" : {
+      "box" : [ 0.5,-0.5,0.5,0.5,0,0,0,-0.5,0,0,0,0.5 ]
+    },
+    "geometricError" : 2.0,
+    "content": {
+      "uri": "unitCubeR.glb"
+    },
+    "transform": [ 
+      1, 0, 0, 0,
+      0, 1, 0, 0,
+      0, 0, 1, 0,
+      3, 0, 0, 1
+    ]
+  }
+}

specs/data/combineTilesets/externalsWithTransform/tileset.json

@@ -0,0 +1,58 @@
+{
+  "asset" : {
+    "version" : "1.1"
+  },
+  "geometricError" : 4.0,
+  "root" : {
+    "boundingVolume" : {
+      "box" : [ 
+        2.0, 1.5, 3.0,
+        2.0, 0, 0,
+        0, -2.5, 0,
+        0, 0, 3.0 
+      ]
+    },
+    "geometricError" : 2.0,
+    "children": [
+      {
+        "boundingVolume" : {
+          "box" : [ 
+            3.5, -0.5, 0.5,
+            0.5, 0, 0,
+            0, -0.5, 0,
+            0, 0, 0.5 ]
+        },
+        "geometricError" : 1.0,
+        "content": {
+          "uri": "externalR.json"
+        }
+      },  
+      {
+        "boundingVolume" : {
+          "box" : [ 
+            0.5, 3.5, 0.5,
+            0.5, 0, 0,
+            0, -0.5, 0,
+            0, 0, 0.5 ]
+        },
+        "geometricError" : 1.0,
+        "content": {
+          "uri": "externalG.json"
+        }
+      },  
+      {
+        "boundingVolume" : {
+          "box" : [ 
+            0.5, -0.5, 5.5,
+            0.5, 0, 0,
+            0, -0.5, 0,
+            0, 0, 0.5 ]
+        },
+        "geometricError" : 1.0,
+        "content": {
+          "uri": "externalB.json"
+        }
+      }      
+    ]
+  }
+}

specs/tools/tilesetProcessing/TilesetCombinerSpec.ts

@@ -8,10 +8,16 @@ import { SpecHelpers } from "../../SpecHelpers";
 
 const SPECS_DATA_BASE_DIRECTORY = SpecHelpers.getSpecsDataBaseDirectory();
 
-const basicInput =
+const nestedExteralInput =
   SPECS_DATA_BASE_DIRECTORY + "/combineTilesets/nestedExternal";
-const basicOutput =
+const nestedExteralOutput =
   SPECS_DATA_BASE_DIRECTORY + "/output/combineTilesets/nestedExternal";
+
+const externalsWithTransformInput =
+  SPECS_DATA_BASE_DIRECTORY + "/combineTilesets/externalsWithTransform";
+const externalsWithTransformOutput =
+  SPECS_DATA_BASE_DIRECTORY + "/output/combineTilesets/externalsWithTransform";
+
 const overwrite = true;
 
 describe("TilesetCombiner", function () {
@@ -22,12 +28,16 @@ describe("TilesetCombiner", function () {
   });
 
   it("combines external tilesets into a single tileset", async function () {
-    await TilesetOperations.combine(basicInput, basicOutput, overwrite);
+    await TilesetOperations.combine(
+      nestedExteralInput,
+      nestedExteralOutput,
+      overwrite
+    );
 
     // Ensure that the output directory contains the expected files:
     // All files of the input, except for the external tileset JSON files
     const actualRelativeFiles =
-      SpecHelpers.collectRelativeFileNames(basicOutput);
+      SpecHelpers.collectRelativeFileNames(nestedExteralOutput);
     actualRelativeFiles.sort();
     const expectedRelativeFiles = [
       "README.md",
@@ -44,7 +54,7 @@ describe("TilesetCombiner", function () {
     // Ensure that the single 'tileset.json' contains the
     // proper content URIs for the combined output
     const tilesetJsonBuffer = fs.readFileSync(
-      Paths.join(basicOutput, "tileset.json")
+      Paths.join(nestedExteralOutput, "tileset.json")
     );
     const tileset = JSON.parse(tilesetJsonBuffer.toString());
     const actualContentUris = await SpecHelpers.collectExplicitContentUris(
@@ -62,4 +72,54 @@ describe("TilesetCombiner", function () {
     ];
     expect(actualContentUris).toEqual(expectedContentUris);
   });
+
+  it("retains the transforms of root nodes of external tilesets", async function () {
+    await TilesetOperations.combine(
+      externalsWithTransformInput,
+      externalsWithTransformOutput,
+      overwrite
+    );
+
+    // Ensure that the resulting tileset JSON contains the
+    // proper content transforms and bounding volumes
+    const tilesetJsonBuffer = fs.readFileSync(
+      Paths.join(externalsWithTransformOutput, "tileset.json")
+    );
+    const tileset = JSON.parse(tilesetJsonBuffer.toString());
+
+    // The expected bounding box for the root is
+    // the bounding box of (0,0,0)-(4,5,6)
+    const expectedRootBox = [2, 1.5, 3, 2, 0, 0, 0, -2.5, 0, 0, 0, 3];
+    const rootBox = tileset.root.boundingVolume.box;
+    expect(rootBox).toEqual(expectedRootBox);
+
+    // The expected bounding box for each child is the unit
+    // cube (it will be transformed into place by the
+    // transform of the children!
+    const expectedChildBox = [0.5, -0.5, 0.5, 0.5, 0, 0, 0, -0.5, 0, 0, 0, 0.5];
+
+    const childBox0 = tileset.root.children[0].boundingVolume.box;
+    const childBox1 = tileset.root.children[1].boundingVolume.box;
+    const childBox2 = tileset.root.children[2].boundingVolume.box;
+    expect(childBox0).toEqual(expectedChildBox);
+    expect(childBox1).toEqual(expectedChildBox);
+    expect(childBox2).toEqual(expectedChildBox);
+
+    // The transforms of the nodes of the children that are created for the
+    // external tilesets are expected to be their original transforms, namely,
+    // - a translation of x=3 for the red cube
+    // - a translation of y=4 for the green cube
+    // - a translation of z=5 for the blue cube
+    const expectedTransformR = [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 3, 0, 0, 1];
+    const childTransformR = tileset.root.children[0].transform;
+    expect(childTransformR).toEqual(expectedTransformR);
+
+    const expectedTransformG = [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 4, 0, 1];
+    const childTransformG = tileset.root.children[1].transform;
+    expect(childTransformG).toEqual(expectedTransformG);
+
+    const expectedTransformB = [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 5, 1];
+    const childTransformB = tileset.root.children[2].transform;
+    expect(childTransformB).toEqual(expectedTransformB);
+  });
 });

src/tools/tilesetProcessing/TilesetCombiner.ts

@@ -142,7 +142,7 @@ export class TilesetCombiner {
     this.externalTilesetFileNames.length = 0;
     await this.combineTilesetsInternal(".", tileset, undefined);
 
-    this.copyResources();
+    this.copyResources(tilesetTargetJsonFileName);
 
     const combinedTilesetJsonString = JSON.stringify(tileset, null, 2);
     const combinedTilesetJsonBuffer = Buffer.from(combinedTilesetJsonString);
@@ -176,6 +176,8 @@ export class TilesetCombiner {
       parentTile.content = root.content;
       parentTile.contents = root.contents;
       parentTile.children = root.children;
+      parentTile.boundingVolume = root.boundingVolume;
+      parentTile.transform = root.transform;
     }
     await Tiles.traverseExplicit(root, async (tilePath: Tile[]) => {
       const tile = tilePath[tilePath.length - 1];
@@ -278,20 +280,26 @@ export class TilesetCombiner {
   /**
    * Copy all elements from the tileset source to the tileset target,
    * except for the ones that have been determined to be external
-   * tilesets.
+   * tilesets, and the one that has the given target name.
    *
    * This is supposed to be called when the `tilesetSource` and
    * `tilesetTarget` are defined, and BEFORE the entry for the
-   * combined tileset JSON is added to the target, because that
-   * entry might overwrite an existing one.
+   * combined tileset JSON (with the given name) is added
+   * to the target.
+   *
+   * @param tilesetTargetJsonFileName The name of the target file
+   * that will contain the combined tileset JSON
    */
-  private copyResources(): void {
+  private copyResources(tilesetTargetJsonFileName: string): void {
     if (!this.tilesetSource || !this.tilesetTarget) {
       throw new DeveloperError("The source and target must be defined");
     }
     const entries = TilesetSources.getEntries(this.tilesetSource);
     for (const entry of entries) {
       const key = entry.key;
+      if (key === tilesetTargetJsonFileName) {
+        continue;
+      }
       if (this.externalTilesetFileNames.includes(key)) {
         continue;
       }