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

Make composite tile #4

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion generator/README.md
Original file line number Diff line number Diff line change
@@ -1 +1,17 @@
Tools for generating sample 3D Tiles tilesets.
Tools for generating sample 3D Tiles tilesets.

# Make Composite Tile

Creates a composite tile from multiple source tiles.

## Example
`node ./bin/makeCompositeTile.js batched.b3dm instanced.i3dm points.pnts -o output/composite.cmpt`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work with input wildcards? Not a huge deal, but would be fine if it is trivial to implement.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the output directory created if it doesn't exist? I am not familiar with all the Node IO routines.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work with input wildcards? Not a huge deal, but would be fine if it is trivial to implement.

It does not, I'll see how hard that is to do.

Copy link
Contributor Author

@lasalvavida lasalvavida Aug 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the output directory created if it doesn't exist? I am not familiar with all the Node IO routines.

It is because writeTile uses fs-extra.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work with input wildcards? Not a huge deal, but would be fine if it is trivial to implement.

It does not, I'll see how hard that is to do.

Actually I just tried it and it does work. Your shell handles the wildcards and passes the resolved arguments.


## Command-Line Flags

| Flag | Description | Required |
| --- | --- | --- |
| `-o`, `--output` | Output path where the composite tile should be written. | No, default `output/composite.cmpt` |
| `-z`, `--gzip` | Gzip the output composite tile. | No, default `false` |

All unflagged arguments are treated as source tiles to place in the composite tile.
56 changes: 56 additions & 0 deletions generator/bin/makeCompositeTile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#!/usr/bin/env node
'use strict';
var Promise = require('bluebird');

var argv = require('yargs')
.usage('Usage: $0 \<tiles\> [options]')
.example('$0 batched.b3dm instanced.i3dm points.pnts -o output/composite.cmpt')
.alias('o', 'output')
.default('o', 'output/composite.cmpt')
.nargs('o', 1)
.describe('o', 'Output path where the composite tile should be written.')
.boolean('z')
.alias('z', 'gzip')
.default('z', false)
.describe('z', 'Flag to gzip the output composite tile.')
.help('h')
.alias('h', 'help')
.demand(1)
.argv;

var readTile = require('../lib/readTile');
var writeTile = require('../lib/writeTile');

var tilePaths = argv._;
var outputPath = argv.o;

Promise.map(tilePaths, function(tilePath) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this logic to a function in lib so it could be part of an API if we decide to expose one later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which part? The Promise.map part is pretty trivial.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all of it so it is a function that takes an array of input paths and an output path.

Copy link
Contributor Author

@lasalvavida lasalvavida Aug 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just separating out the composite logic? Make it a function that takes an array of buffers and outputs the composite tile buffer. I'd rather not force it to be to/from the filesystem and handle the I/O in the script.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

return readTile(tilePath);
})
.then(function(tiles) {
var header = new Buffer(16);
var buffers = [];
buffers.push(header);
var byteLength = header.length;
for (var i = 0; i < tiles.length; i++) {
var tile = tiles[i];
// Byte align all tiles to 4 bytes
var tilePadding = tile.length % 4;
if (tilePadding !== 0) {
tile = Buffer.concat([tile, new Buffer(4 - tilePadding)]);
}
tile.writeUInt32LE(tile.length, 8); // byteLength
byteLength += tile.length;
buffers.push(tile);
}
header.write('cmpt', 0); // magic
header.writeUInt32LE(1, 4); // version
header.writeUInt32LE(byteLength, 8); // byteLength
header.writeUInt32LE(tiles.length, 12); // tilesLength
var buffer = Buffer.concat(buffers);
var options = {};
if (argv.z) {
options.gzip = true;
}
return writeTile(outputPath, buffer, options);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integrate this into the 3d-tiles-tools.js file.

22 changes: 22 additions & 0 deletions generator/lib/isGzipped.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict';
var Cesium = require('cesium');

var DeveloperError = Cesium.DeveloperError;
var defined = Cesium.defined;

module.exports = isGzipped;

/**
* Test if the provided data is gzipped.
*
* @param {Buffer} data A buffer containing the data to test.
* @returns {Boolean} True if the data is gzipped, False if not.
*
* @throws {DeveloperError} Will throw an error if data is undefined.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically we don't mention Errors that are thrown because of missing arguments.

*/
function isGzipped(data) {
if (!defined(data)) {
throw new DeveloperError('data must be defined.');
}
return data[0] === 0x1f && data[1] === 0x8b;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another file called isGzipped exists that checks if a file is gzipped, so maybe that one can be renamed to isGzippedFile.

I can see several areas where gzipTileset can improve by using this one as well as using readTile and writeTile. I'll make those changes after this PR is in.

39 changes: 39 additions & 0 deletions generator/lib/readTile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
'use strict';
var Cesium = require('cesium');
var Promise = require('bluebird');
var fs = require('fs-extra');
var zlib = require('zlib');

var DeveloperError = Cesium.DeveloperError;
var defined = Cesium.defined;

var isGzipped = require('./isGzipped');

var fsReadFile = Promise.promisify(fs.readFile);
var zlibGunzip = Promise.promisify(zlib.gunzip);

module.exports = readTile;

/**
* Reads tile data from a file.
*
* @param {String} filePath The file path to read from.
* @returns {Promise} A promise that resolves with the data when the read operation completes.
*
* @throws {DeveloperError} Will throw an error if filePath is undefined.
*/
function readTile(filePath) {
return Promise.resolve()
.then(function() {
if (!defined(filePath)) {
throw new DeveloperError('filePath must be defined');
}
return fsReadFile(filePath);
})
.then(function(buffer) {
if (isGzipped(buffer)) {
return zlibGunzip(buffer, undefined);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to pass in undefined.

}
return buffer;
});
}
Copy link
Contributor

@lilleyse lilleyse Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file and writeTile can probable be condensed. See this comment: #3 (comment)

47 changes: 47 additions & 0 deletions generator/lib/writeTile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
'use strict';
var Cesium = require('cesium');
var Promise = require('bluebird');
var fs = require('fs-extra');
var zlib = require('zlib');

var DeveloperError = Cesium.DeveloperError;
var defaultValue = Cesium.defaultValue;
var defined = Cesium.defined;

var fsOutputFile = Promise.promisify(fs.outputFile);
var zlibGzip = Promise.promisify(zlib.gzip);

module.exports = writeTile;

/**
* Writes the tile data to a file.
*
* @param {String} filePath The file path where the tile should be written.
* @param {Buffer} tileData A buffer containing the tile data to write.
* @param {Object} [options] Defines custom behavior for writing.
* @param {Boolean} [options.gzip=false] Flag to gzip the buffer data before writing.
* @returns {Promise} A promise that resolves when the write operation completes.
*
* @throws {DeveloperError} Throws an error if filePath is undefined.
* @throws {DeveloperError} Throws an error if tileData is undefined.
*/
function writeTile(filePath, tileData, options) {
return Promise.resolve()
.then(function() {
if (!defined(filePath)) {
throw new DeveloperError('filePath must be defined.');
}
if (!defined(tileData)) {
throw new DeveloperError('tileData must be defined.');
}
options = defaultValue(options, {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options = defaultValue(options, defaultValue.EMPTY_OBJECT);

var gzip = defaultValue(options.gzip, false);
if (gzip) {
return zlibGzip(tileData, undefined);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there was some talk that gzip is faster as a sync operation because it's not actually doing any file reading. @mramato is this correct?

}
return Promise.resolve(tileData);
})
.then(function(buffer) {
return fsOutputFile(filePath, buffer);
});
}
7 changes: 4 additions & 3 deletions generator/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@
},
"dependencies": {
"bluebird": "3.4.1",
"cesium": "1.24"
"cesium": "1.24",
"fs-extra": "0.30.0",
"yargs": "5.0.0"
},
"devDependencies": {
"fs-extra": "0.30.0",
"gulp": "3.9.1",
"gulp-jshint": "2.0.1",
"istanbul": "0.4.4",
Expand All @@ -37,7 +38,7 @@
"open": "0.0.5",
"request": "2.74.0",
"requirejs": "2.2.0",
"yargs": "4.8.1"
"rimraf": "2.5.4"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use fsExtra.remove instead.

},
"scripts": {
"jsHint": "gulp jsHint",
Expand Down
82 changes: 82 additions & 0 deletions generator/specs/bin/makeCompositeTileSpec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
'use strict';
var Promise = require('bluebird');
var childProcess = require('child_process');
var fs = require('fs-extra');
var rimraf = require('rimraf');

var isGzipped = require('../../lib/isGzipped');

var fsReadFile = Promise.promisify(fs.readFile);
var rimrafAsync = Promise.promisify(rimraf);

var justHeaderI3dmPath = './specs/data/justHeader.i3dm';
var justHeaderGzippedI3dmPath = './specs/data/justHeaderGzipped.i3dm';
var testOutputPath = './specs/data/.test/';

describe('makeCompositeTile', function() {
afterAll(function(done) {
rimrafAsync(testOutputPath, {})
.then(done);
});

it('makes a composite tile', function(done) {
var outputPath = testOutputPath + 'composite.cmpt';
expect(makeCompositeTile([justHeaderI3dmPath, justHeaderI3dmPath, '-o', outputPath])
.then(function() {
return fsReadFile(outputPath);
})
.then(function(tileData) {
var magic = tileData.toString('utf8', 0, 4);
var byteLength = 16 + 32 + 32;
expect(tileData.length).toBe(byteLength);
expect(magic).toEqual('cmpt'); // magic
expect(tileData.readUInt32LE(4)).toBe(1); // version
expect(tileData.readUInt32LE(8)).toBe(byteLength); // byteLength
expect(tileData.readUInt32LE(12)).toBe(2); // tilesLength
var tileMagic = tileData.toString('utf8', 16, 20);
expect(tileMagic).toEqual('i3dm');
}), done).toResolve();
});

it('makes a composite tile from gzipped tiles', function(done) {
var outputPath = testOutputPath + 'compositeFromGzipped.cmpt';
expect(makeCompositeTile([justHeaderGzippedI3dmPath, justHeaderGzippedI3dmPath, '-o', outputPath])
.then(function() {
return fsReadFile(outputPath);
})
.then(function(tileData) {
var magic = tileData.toString('utf8', 0, 4);
var byteLength = 16 + 32 + 32;
expect(tileData.length).toBe(byteLength);
expect(magic).toEqual('cmpt'); // magic
expect(tileData.readUInt32LE(4)).toBe(1); // version
expect(tileData.readUInt32LE(8)).toBe(byteLength); // byteLength
expect(tileData.readUInt32LE(12)).toBe(2); // tilesLength
var tileMagic = tileData.toString('utf8', 16, 20);
expect(tileMagic).toEqual('i3dm');
}), done).toResolve();
});

it('makes a gzipped composite tile', function(done) {
var outputPath = testOutputPath + 'compositeGzipped.cmpt';
expect(makeCompositeTile([justHeaderI3dmPath, justHeaderI3dmPath, '-o', outputPath, '-z'])
.then(function() {
return fsReadFile(outputPath);
})
.then(function(tileData) {
expect(isGzipped(tileData)).toBeTruthy();
}), done).toResolve();
});
});

function makeCompositeTile(args) {
var makeCompositeTileProcess = childProcess.fork('./bin/makeCompositeTile', args);
return new Promise(function(resolve, reject) {
makeCompositeTileProcess.on('close', function() {
resolve();
});
makeCompositeTileProcess.on('error', function() {
reject();
});
});
}
Copy link
Contributor

@lilleyse lilleyse Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a similar comment on #6, we aren't running tests on the bin folder right now. However if you want to write tests for 3d-tiles-tools.js and include this code there feel free to do so.

Binary file added generator/specs/data/justHeader.i3dm
Binary file not shown.
Binary file added generator/specs/data/justHeaderGzipped.i3dm
Binary file not shown.
24 changes: 24 additions & 0 deletions generator/specs/lib/isGzippedSpec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';
var Promise = require('bluebird');
var zlib = require('zlib');

var isGzipped = require('../../lib/isGzipped');

var zlibGzip = Promise.promisify(zlib.gzip);

describe('isGzipped', function() {
it('throws DeveloperError if data is undefined', function() {
expect(function() {
isGzipped(undefined);
}).toThrowError();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to toThrowDeveloperError

});

it('detects when data is gzipped', function(done) {
var data = new Buffer(40);
expect(isGzipped(data)).toBeFalsy();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toBe(false) and toBe(true) are more precise.

expect(zlibGzip(data, undefined)
.then(function(zippedData) {
expect(isGzipped(zippedData)).toBeTruthy();
}), done).toResolve();
});
});
27 changes: 27 additions & 0 deletions generator/specs/lib/readTileSpec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';
var readTile = require('../../lib/readTile');

describe('readTile', function() {
it('throws DeveloperError if filePath is undefined', function(done) {
expect(readTile(undefined)
.catch(function(error) {
expect(error).toBeDefined();
}), done).toResolve();
});

it('reads a tile', function(done) {
expect(readTile('./specs/data/justHeader.i3dm')
.then(function(tileData) {
var magic = tileData.toString('utf8', 0, 4);
expect(magic).toEqual('i3dm');
}), done).toResolve();
});

it('reads a gzipped tile', function(done) {
expect(readTile('./specs/data/justHeaderGzipped.i3dm')
.then(function(tileData) {
var magic = tileData.toString('utf8', 0, 4);
expect(magic).toEqual('i3dm');
}), done).toResolve();
});
});
Loading