Make composite tile #4

lasalvavida · 2016-08-19T15:26:24Z

Adds a tool for generating composite tilesets.

pjcozzi · 2016-08-19T18:24:29Z

generator/bin/makeCompositeTile.js

+var defined = Cesium.defined;
+
+var argv = require('yargs')
+    .usage('Usage: $0 \<tilesets\> [options]')


"tilesets" -> "tiles", right?

Yup! Thank you

pjcozzi · 2016-08-19T18:25:03Z

Thanks @lasalvavida! Please add doc to the README.md, and add unit tests.

@lilleyse please review and merge when this is ready.

pjcozzi · 2016-08-19T18:26:05Z

@lasalvavida @lilleyse also please keep the roadmap, CesiumGS/3d-tiles-tools#9, updated when things are merged. I'll leave this 100% up to you guys.

lasalvavida · 2016-08-24T21:35:11Z

Updated

pjcozzi · 2016-08-24T21:57:40Z

generator/README.md

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


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

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

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.

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.

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.

pjcozzi · 2016-08-24T22:01:06Z

Looks great at a quick glance! @lilleyse review and merge please.

added tests

lasalvavida · 2016-08-25T15:37:22Z

Updated

pjcozzi · 2016-08-25T16:24:50Z

Thanks @lasalvavida. @lilleyse can you please do the final review and merge?

lilleyse · 2016-08-25T17:39:33Z

Sure, I haven't reviewed this at all yet but I'm going to take some time later to review this and finish up #3

lilleyse

This utility fits better in the tools project instead. The generator project will be more for constructing sample tilesets. My comments below assume that it's in the tools repo.

Overall things look good here. Just some organizational comments now that #3 is in.

lilleyse · 2016-10-03T15:41:30Z

generator/bin/makeCompositeTile.js

+            options.gzip = true;
+        }
+        return writeTile(outputPath, makeCompositeTile(tiles), options);
+    });


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

lilleyse · 2016-10-03T15:41:32Z

generator/lib/isGzipped.js

+ * @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.


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

lilleyse · 2016-10-03T15:41:39Z

generator/lib/isGzipped.js

+        throw new DeveloperError('data must be defined.');
+    }
+    return data[0] === 0x1f && data[1] === 0x8b;
+}


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.

lilleyse · 2016-10-03T15:41:43Z

generator/lib/makeCompositeTile.js

+/**
+ * Combines an array of tile buffers into a single composite tile.
+ *
+ * @param {Array.<Buffer>} tileBuffers An array of buffers holding tile data.


At least in Cesium, we go with the Buffer[] version.

lilleyse · 2016-10-03T15:41:49Z

generator/lib/readTile.js

+            }
+            return buffer;
+        });
+}


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

lilleyse · 2016-10-03T16:09:30Z

generator/lib/writeTile.js

+            options = defaultValue(options, {});
+            var gzip = defaultValue(options.gzip, false);
+            if (gzip) {
+                return zlibGzip(tileData, undefined);


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?

lilleyse · 2016-10-03T16:09:42Z

generator/specs/lib/isGzippedSpec.js

+    it('throws DeveloperError if data is undefined', function() {
+        expect(function() {
+            isGzipped(undefined);
+        }).toThrowError();


Change to toThrowDeveloperError

lilleyse · 2016-10-03T16:09:46Z

generator/specs/lib/isGzippedSpec.js

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


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

lilleyse · 2016-10-03T16:09:51Z

generator/specs/lib/writeTileSpec.js

+            .then(function(tileData) {
+                var magic = tileData.toString('utf8', 0, 4);
+                expect(magic).toEqual('i3dm');
+            }),done).toResolve();


, done spacing

lilleyse · 2016-10-03T16:09:53Z

generator/specs/lib/writeTileSpec.js

+            })
+            .then(function(tileData) {
+                expect(isGzipped(tileData)).toBeTruthy();
+            }),done).toResolve();


lilleyse · 2016-10-04T17:52:52Z

Add makeCompositeTile to index.js.

…ols into make-composite-tile

lasalvavida · 2016-10-07T16:56:07Z

@lilleyse This is ready for a look.

lilleyse · 2016-10-07T17:45:28Z

tools/lib/getDefaultWriteCallback.js

-        return fsExtraOutputFile(outputFile, data);
+function getDefaultWriteCallback() {
+    return function(file, data, options) {
+        return writeTile(file, data, options);


I think it's best to just keep this really simple like it was before where it just gets data and write to a file. Gzipping or anything else should be handled by the calling code. Maybe this makes writeTile not needed anymore.

lilleyse · 2016-10-07T17:45:32Z

tools/lib/gzipTileset.js

-            .on('data', function (item) {
-                if (!item.stats.isDirectory()) {
-                    ++numberOfFiles;
+    return getFilesInDirectory(inputDirectory, {


I used to do a similar approach here, but changed it for this comment: #3 (comment). I'm fine with merging the changes in this file in though.

I have a branch that cleans up gzipTileset and the end result looks similar to this, but walks the directory without buffering the file names up front (through a helper function). And really that too is probably going to get blown away again at some later point.

lilleyse · 2016-10-07T17:45:42Z

tools/lib/isTileFile.js

+
+module.exports = isTileFile;
+
+function isTileFile(file) {


Mark as @Private

lilleyse · 2016-10-07T17:46:32Z

tools/specs/lib/gzipTilesetSpec.js

        };
        var gzipOptions = {
            inputDirectory : tilesetDirectory,
+            outputDirectory : gzippedDirectory,


The idea behind custom write callbacks was that you didn't need to supply an output directory to the stage because that information only needs to be known by the callback. I think the old way with using a closure for getDefaultWriteCallback was more flexible.

lilleyse · 2016-10-07T17:58:24Z

Also add makeCompositeTile to the README and 3d-tiles-tools.js Just make sure it's clearly defined that its not a pipeline stage and doesn't operate over the whole tileset.

lilleyse · 2017-07-02T23:45:51Z

Brought the makeCompositeTile file into #86

lasalvavida added 4 commits August 19, 2016 11:22

Added make composite tile utility

7cb47ac

Updated packages

180365a

Use Cesium.defined

e19651f

Comment formatting

69ba157

pjcozzi reviewed Aug 19, 2016
View reviewed changes

lasalvavida added 2 commits August 24, 2016 17:32

Fleshed out makeCompositeTile with tests and README

dc410fc

jsHint fix

5554957

pjcozzi reviewed Aug 24, 2016
View reviewed changes

lasalvavida added 2 commits August 25, 2016 11:36

Separated makeCompositeTile into a lib function of the same name, and

36cb38e

added tests

Aligned comments

5fb2426

Include order fixes

d3f8ae8

pjcozzi mentioned this pull request Aug 25, 2016

Added glbToPlainB3dm to generate basic b3dm from glb #6

Merged

lilleyse reviewed Oct 3, 2016

View reviewed changes

lilleyse mentioned this pull request Oct 4, 2016

Validator setup #8

Merged

lasalvavida added 7 commits October 7, 2016 09:28

Merge branch 'master' of github.com:AnalyticalGraphicsInc/3d-tiles-to…

68fd781

…ols into make-composite-tile

Migrated writeCompositeTile from generator to tools

31bde95

Removed files from generator

6e0f9c7

Fixed a few errors

434ed9a

Throw errors outside of promise

ca69ae6

Reverted README changes

7187ece

Fixed fsExtra usage

1634430

lasalvavida added 5 commits October 7, 2016 12:51

Removed samples from generator data

aad3cdb

Added makeCompositeTile to index

2eb4dea

Updated packages

b63c911

Removed rimraf

dec3c9e

Reverted package.json

d7a15aa

lilleyse reviewed Oct 7, 2016

View reviewed changes

lilleyse closed this Jul 2, 2017

lilleyse mentioned this pull request Jul 2, 2017

Upgrade tileset stage #86

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make composite tile #4

Make composite tile #4

lasalvavida commented Aug 19, 2016

pjcozzi Aug 19, 2016

lasalvavida Aug 19, 2016

pjcozzi commented Aug 19, 2016

pjcozzi commented Aug 19, 2016

lasalvavida commented Aug 24, 2016

pjcozzi Aug 24, 2016

pjcozzi Aug 24, 2016

lasalvavida Aug 25, 2016

lasalvavida Aug 25, 2016 •

edited

Loading

lasalvavida Aug 25, 2016

pjcozzi commented Aug 24, 2016

lasalvavida commented Aug 25, 2016

pjcozzi commented Aug 25, 2016

lilleyse commented Aug 25, 2016

lilleyse left a comment

lilleyse Oct 3, 2016

lilleyse Oct 3, 2016

lilleyse Oct 3, 2016

lilleyse Oct 3, 2016

lilleyse Oct 3, 2016 •

edited

Loading

lilleyse Oct 3, 2016

lilleyse Oct 3, 2016

lilleyse Oct 3, 2016

lilleyse Oct 3, 2016

lilleyse Oct 3, 2016

lilleyse commented Oct 4, 2016

lasalvavida commented Oct 7, 2016

lilleyse Oct 7, 2016 •

edited

Loading

lilleyse Oct 7, 2016 •

edited

Loading

lilleyse Oct 7, 2016

lilleyse Oct 7, 2016

lilleyse commented Oct 7, 2016

lilleyse commented Jul 2, 2017 •

edited

Loading


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

Make composite tile #4

Make composite tile #4

Conversation

lasalvavida commented Aug 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Aug 19, 2016

pjcozzi commented Aug 19, 2016

lasalvavida commented Aug 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lasalvavida Aug 25, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Aug 24, 2016

lasalvavida commented Aug 25, 2016

pjcozzi commented Aug 25, 2016

lilleyse commented Aug 25, 2016

lilleyse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilleyse Oct 3, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilleyse commented Oct 4, 2016

lasalvavida commented Oct 7, 2016

lilleyse Oct 7, 2016 • edited Loading

Choose a reason for hiding this comment

lilleyse Oct 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilleyse commented Oct 7, 2016

lilleyse commented Jul 2, 2017 • edited Loading

lasalvavida Aug 25, 2016 •

edited

Loading

lilleyse Oct 3, 2016 •

edited

Loading

lilleyse Oct 7, 2016 •

edited

Loading

lilleyse Oct 7, 2016 •

edited

Loading

lilleyse commented Jul 2, 2017 •

edited

Loading