Compress textures stage #204

lilleyse · 2017-01-04T16:45:35Z

To-do / notes:

Further testing in Cesium on multiple devices and with multiple models. So far textured box and Cesium Air work correctly.

The rest of the items are moved to #210

pjcozzi · 2017-01-06T00:10:26Z

Support mip-mapping?...

Likely, but this will depend on the final spec for glTF. Will keep you posted, but it may end up in a roadmap tasklist.

pjcozzi · 2017-01-06T00:13:26Z

Support loading target/dds/ktx as input images?

This means support loading glTF models that reference dds or ktx images (what is "target?"), right?

Why dds? I don't expect it will be part of glTF.

As for KTX, is it possible to support it but just disable some pipeline optimizations to start? Otherwise, we may have to decompress in software to resize, texture atlas, etc.

pjcozzi · 2017-01-06T00:14:40Z

Is it better to scale to the lower power-of-two or high power-of-two. Right now it scales to lower.

I don't know what is common. Perhaps we warn if it needs to be scaled at all, and suggest that users resize their textures beforehand.

Longer-term, we could round, or have an option to floor/ceil/round.

pjcozzi · 2017-01-06T00:17:25Z

Support for EAC formats or not a priority?

Is this a matter of finding the right tools to integrate? I think once this part of gltf-pipeline has some traction, folks may contribute here.

pjcozzi · 2017-01-06T00:19:40Z

README.md

 |`--quantize`, `-q`|Quantize the attributes of this glTF asset using the WEB3D_quantized_attributes extension.|No, default `false`|
 |`--encodeNormals`, `-n`|Oct-encode the normals of this glTF asset.|No, default `false`|
 |`--compressTextureCoordinates`, `-c`|Compress the testure coordinates of this glTF asset.|No, default `false`|
 |`--removeNormals`, `-r`|Strips off existing normals, allowing them to be regenerated.|No, default `false`|
 |`--faceNormals`, `-f`|If normals are missing, they should be generated using the face normal.|No, default `false`|
-|`--cesium`, `-c`|Optimize the glTF for Cesium by using the sun as a default light source.|No, default `false`|


Just curious, why did you remove this?

The -c flag belongs to compressTextureCoordinates, I don't think this one ever had a short flag.

pjcozzi · 2017-01-06T00:29:25Z

README.md

@@ -50,6 +50,12 @@ node ./bin/gltf-pipeline.js -i ./specs/data/boxTexturedUnoptimized/CesiumTexture
 |`--ao.groundPlane`|Simulate a groundplane at the lowest point of the model when baking AO.|No, default `false`|
 |`--ao.ambientShadowContribution`|Amount of AO to show when blending between shader computed lighting and AO. 1.0 is full AO, 0.5 is a 50/50 blend.|No, default `0.5`|
 |`--ao.quality`|Quality to use when baking AO. Valid settings are high, medium, and low.|No, default `low`|
+|`--texcomp.enable`|Compress textures.|No, default `false`|
+|`--texcomp.format`|The compressed texture format.|No, unless `texcomp.enable` is defined.|


What are the valid values?

pjcozzi · 2017-01-06T00:29:45Z

README.md

@@ -94,6 +100,10 @@ The documentation will be placed in the `doc` folder.

 Pull requests are appreciated!  Please use the same [Contributor License Agreement (CLA)](https://github.com/AnalyticalGraphicsInc/cesium/blob/master/CONTRIBUTING.md) and [Coding Guide](https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Documentation/Contributors/CodingGuide/README.md) used for [Cesium](http://cesiumjs.org/).

+## Attribution
+
+This product includes components of the PowerVR Tools Software from Imagination Technologies Limited.


Include a link to the directory or LICENSE.md file.

pjcozzi · 2017-01-06T00:30:46Z

README.md

+|`--texcomp.quality`|The compressed texture quality from 0 to 10.|No, default `5`|
+|`--texcomp.bitrate`|The bitrate when using the pvrtc or astc formats|No, default `2.0`|
+|`--texcomp.blockSize`|The block size for astc compression. Smaller block sizes result in higher bitrates. This value is ignored if options.bitrate is also set.|No, default `8x8`|
+|`--texcomp.alphaBit`|Store a single bit for alpha. Only supported for etc2.|No, default `false`|

 ## Build Instructions


Include a section on where the binaries for the texture compressions came from and how to build any that you built yourself (links to build instructions elsewhere is OK).

pjcozzi · 2017-01-06T00:35:51Z

lib/parseArguments.js

+                group: 'Options: Texture Compression',
+                type: 'boolean'
+            },
+            'texcomp.format': {


For the texcomp.* options, are there any links we can point folks to, e.g., the doc from the vendor who wrote the tool we are forwarding the parameters to.

These links could be in the README.md.

We'll also link to @bagnell's upcoming blog post.

pjcozzi · 2017-01-06T00:39:43Z

I know you already tested with Mac, but all tests pass on my Mac. 🥇

pjcozzi · 2017-01-06T00:41:06Z

specs/lib/compressTexturesSpec.js

+function verifyCrunch(gltfPath, imagePath, options) {
+    return getCompressedTexture(gltfPath, imagePath, options)
+        .then(function(uri) {
+            // TODO : Do more verification: width, height, and expectedFormat - this requires a .crn reader


Move to tasklist?

pjcozzi · 2017-01-06T00:42:43Z

specs/lib/compressTexturesSpec.js

+}
+
+function verifyKTX(gltfPath, imagePath, options, expectedFormat) {
+    return getCompressedTexture(gltfPath, imagePath, options)


Should getCompressedTexture be named compressTexture, e.g., doThis instead of getDoThis?

pjcozzi · 2017-01-06T00:43:13Z

Update LICENSE.md for all the third-party compression libraries.

pjcozzi · 2017-01-06T00:46:56Z

specs/lib/compressTexturesSpec.js

@@ -0,0 +1,444 @@
+'use strict';


Very nice job with these tests. Clean.

pjcozzi · 2017-01-06T00:50:33Z

lib/compressTextures.js

+    var flipY = false;
+
+    if (format === 'pvrtc1' || format === 'pvrtc2') {
+        // PVRTC hardware support rectangular power-of-two, but iOS software requires square power-of-two.


Should there be some warnings for the caveats here and below?

pjcozzi · 2017-01-06T00:53:08Z

lib/compressTextures.js

+    };
+}
+
+function getTempDirectory() {


Perhaps @private in its own file.

pjcozzi · 2017-01-06T00:55:22Z

lib/compressTextures.js

+    }
+
+    var outputPath = getTempImagePath(tempDirectory, extension);
+    var cpuCount = os.cpus().length;


Add a tasklist item (that will go into the roadmap) to make this an option, which we'll want for server-side.

pjcozzi · 2017-01-06T00:55:50Z

Looks great, just those comments.

lilleyse · 2017-01-06T19:01:13Z

This means support loading glTF models that reference dds or ktx images (what is "target?"), right?

Why dds? I don't expect it will be part of glTF.

As for KTX, is it possible to support it but just disable some pipeline optimizations to start? Otherwise, we may have to decompress in software to resize, texture atlas, etc.

Ah, meant Targa instead of target.
Yeah I'm thinking these changes aren't really needed for now. Images of any type will pass through the pipeline fine as long as stages like AO-to-texture of compressTextures aren't run.

I don't know what is common. Perhaps we warn if it needs to be scaled at all, and suggest that users resize their textures beforehand.

Longer-term, we could round, or have an option to floor/ceil/round.

Ok I can log some warnings to the console. Adding additional options for floor/ceil/round is super easy but can be done later.

Is this a matter of finding the right tools to integrate? I think once this part of gltf-pipeline has some traction, folks may contribute here.

EAC is supported by PVRTexTool. It's only a 1/2 channel format so definitely not a priority now.

bagnell · 2017-01-06T20:08:29Z

KTX files may contain textures of formats other than the compressed formats. It can be any format accepted by glTexImage2D. We may want to read some of the variants, like GL_RGB8, GL_RGBA8, etc.

lilleyse · 2017-01-06T20:30:14Z

@pjcozzi Address your comments. I will test on Linux tonight.

pjcozzi · 2017-01-26T13:10:14Z

@pjcozzi not sure if this was your intention, but could the compressed formats point to other image id's?

Not that was not my intention since, technically, a glTF 2.0 image can't point to a .ktx file. So the example you wrote above, #204 (comment), would be:

"images": {
    "image-id": {
        "name": "image-name",
        "extras" : {
            "compressedImage3DTiles" : { // with one or more of the following properties:
                "s3tc": {
                    "bufferView" : // ...,
                    "mimeType" : "image/crn",
                    "height" : 256,  // if these become core glTF spec for "image"
                    "width" : 256
                },
                "pvrtc": {
                    "uri" : "image.ktx" // KTX must be PVRTC compressed
                },
                "etc": {
                    "uri" : "image.ktx" // KTX must be ETC compressed
                },
                "astc": {
                    "uri" : "image.ktx" // KTX must be ASTC compressed
                }
            }
        },
        "uri": // image when compression is not supported, could be a data uri for a 1x1 white pixel
    }
}

Everything is inside extras.compressedImage3DTiles. For it to be valid glTF, image.uri must still be defined, but could be a 1x1 white pixel data uri.

pjcozzi · 2017-01-26T13:13:50Z

node bin/gltf-pipeline.js specs/data/boxTexturedUnoptimized/CesiumTexturedBoxTest.gltf compressed.gltf --texcomp.format=dxt1 --texcomp.format=etc1 --texcomp.quality=5

To keep the CLI simple all formats use the same quality/other settings. When using the API each format can take its own options object.

Couldn't this be as simple as renaming all texcomp.* arguments, except for texcomp.format to be texcomp.[format].whatever so the above would be:

node bin/gltf-pipeline.js specs/data/boxTexturedUnoptimized/CesiumTexturedBoxTest.gltf compressed.gltf --texcomp.format=dxt1 --texcomp.dxt1.quality=5 --texcomp.format=etc1 --texcomp.etc1.quality=9

pjcozzi · 2017-01-26T13:38:11Z

Can we also document in the README.md, CLI help, and reference doc that this is Cesium and 3D Tiles specific extra metadata; compressed textures are not yet part of the glTF spec, and a link to KhronosGroup/glTF#739?

pjcozzi · 2017-01-26T13:38:47Z

@lilleyse is this item in the tasklist complete?

Further testing in Cesium on multiple devices and with multiple models. So far textured box and Cesium Air work correctly.

pjcozzi · 2017-01-26T13:40:51Z

@bagnell please update Cesium for the above schema change? In the order of preference of compressed textures, I guess load etc last in case there are software implementations on desktop.

mramato · 2017-01-26T14:02:39Z

Why are we submitting binaries directly to git and including them directly in the npm package? This bloats repo size, is non-standard, and considerably limits cross-system compatibility immediately.

We should be using https://github.com/mapbox/node-pre-gyp to serve pre-built binaries and https://github.com/nodejs/node-gyp to build them on the fly if needed. (Do we build these binaries ourselves?)

lilleyse · 2017-01-30T20:16:08Z

Updated to write all compressed images inside extras. Also the CLI now supports different options per format, the command is like below:

node ./bin/gltf-pipeline.js -i ./specs/data/boxTexturedUnoptimized/CesiumTexturedBoxTest.gltf -o output.gltf --texcomp.dxt1.enable --texcomp.dxt1.quality=10 --texcomp.etc1.enable

Next I'll look into serving the pre-built binaries differently. @mramato we do build them ourselves.

pjcozzi · 2017-02-02T12:10:35Z

Next I'll look into serving the pre-built binaries differently

If this is non-trivial, please submit an issue for it; otherwise, a separate PR is OK.

pjcozzi · 2017-02-02T12:11:35Z

[ ] Further testing in Cesium on multiple devices and with multiple models. So far textured box and Cesium Air work correctly.

Is this still needed?

pjcozzi · 2017-02-02T12:17:05Z

On Windows with a fresh npm install, running the example failed for me:

$ node ./bin/gltf-pipeline.js -i ./specs/data/boxTexturedUnoptimized/CesiumText
uredBoxTest.gltf -o output.gltf --texcomp.dxt1.enable --texcomp.dxt1.quality=10
 --texcomp.etc1.enable
Warning: Image0001 will be resized to the lower power-of-two for texture compres
sion. To prevent this warning in the future resize any images beforehand.
Warning: Image0001 will be resized to the lower power-of-two for texture compres
sion. To prevent this warning in the future resize any images beforehand.
Unhandled rejection TypeError: base64 is not a function
    at Function.from (native)
    at Function.from (native)
    at compressTextures (c:\source\gltf-pipeline\lib\compressTextures.js:83:44)
    at c:\source\gltf-pipeline\lib\Pipeline.js:164:24
    at tryCatcher (c:\source\gltf-pipeline\node_modules\bluebird\js\release\util
.js:16:23)
    at Promise._settlePromiseFromHandler (c:\source\gltf-pipeline\node_modules\b
luebird\js\release\promise.js:510:31)
    at Promise._settlePromise (c:\source\gltf-pipeline\node_modules\bluebird\js\
release\promise.js:567:18)
    at Promise._settlePromise0 (c:\source\gltf-pipeline\node_modules\bluebird\js
\release\promise.js:612:10)
    at Promise._settlePromises (c:\source\gltf-pipeline\node_modules\bluebird\js
\release\promise.js:691:18)
    at Promise._fulfill (c:\source\gltf-pipeline\node_modules\bluebird\js\releas
e\promise.js:636:18)
    at PromiseArray._resolve (c:\source\gltf-pipeline\node_modules\bluebird\js\r
elease\promise_array.js:125:19)
    at PromiseArray._promiseFulfilled (c:\source\gltf-pipeline\node_modules\blue
bird\js\release\promise_array.js:143:14)
    at Promise._settlePromise (c:\source\gltf-pipeline\node_modules\bluebird\js\
release\promise.js:572:26)
    at Promise._settlePromise0 (c:\source\gltf-pipeline\node_modules\bluebird\js
\release\promise.js:612:10)
    at Promise._settlePromises (c:\source\gltf-pipeline\node_modules\bluebird\js
\release\promise.js:691:18)
    at Async._drainQueue (c:\source\gltf-pipeline\node_modules\bluebird\js\relea
se\async.js:133:16)

Unhandled rejection Error
    at new DeveloperError (c:\source\gltf-pipeline\node_modules\cesium\Source\Co
re\DeveloperError.js:44:19)
    at ChildProcess.<anonymous> (c:\source\gltf-pipeline\lib\compressTexture.js:
220:24)
    at ChildProcess.g (events.js:260:16)
    at emitTwo (events.js:87:13)
    at ChildProcess.emit (events.js:172:7)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:200:12)

There are also 28 related test failures.

I updated to Cesium 1.30, 41aed06, but Linux CI still passes.

pjcozzi · 2017-02-02T12:45:57Z

Does this spawn a process per texture up to a limit?

lilleyse · 2017-02-02T18:37:58Z

What version of node are you using?

lilleyse · 2017-02-02T18:40:36Z

Does this spawn a process per texture up to a limit?

Yes it spawns a process per texture. There is no limit.

lilleyse · 2017-02-02T19:09:10Z

This should be ready. For the time being node-pre-gyp may not be the solution as that is intended for C++ node modules whereas the tools here are CLI based. I'm going to open an issue so we don't forget about eventually serving the binaries differently.

lilleyse · 2017-02-02T19:16:13Z

[ ] Further testing in Cesium on multiple devices and with multiple models. So far textured box and Cesium Air work correctly.
Is this still needed?

Crossed it out. @bagnell has been using this PR to generate models for cesium and no problems have come up lately.

pjcozzi · 2017-02-03T01:58:34Z

What version of node are you using?

The right version now. :)

lilleyse added 3 commits January 4, 2017 11:26

Added compressTextures stage

8b215bd

Merge branch 'master' into compress-textures

82b35ed

Clarify alphaBit

dca8fd4

lilleyse mentioned this pull request Jan 4, 2017

Texture compression CesiumGS/cesium#4758

Merged

5 tasks

lilleyse added 4 commits January 4, 2017 14:17

Fix glTypeSize from crunch

9506993

astc to ktx

c8b6eca

astc flipy and cleanup

6e0044f

Make mac/linux binaries executable

90f52f1

pjcozzi reviewed Jan 6, 2017

View reviewed changes

specs/lib/compressTexturesSpec.js

@@ -0,0 +1,444 @@

'use strict';

Copy link

Contributor

pjcozzi Jan 6, 2017

Choose a reason for hiding this comment

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

Very nice job with these tests. Clean.

pjcozzi reviewed Jan 6, 2017

View reviewed changes

lib/compressTextures.js Outdated

};

}

function getTempDirectory() {

Copy link

Contributor

pjcozzi Jan 6, 2017

Choose a reason for hiding this comment

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

Perhaps @private in its own file.

pjcozzi reviewed Jan 6, 2017

View reviewed changes

License, readme, warnings, cleanup

21d43bd

pjcozzi mentioned this pull request Jan 26, 2017

Document texture compression CesiumGS/3d-tiles#173

Closed

lilleyse added 3 commits January 30, 2017 13:56

Save compressed textures as extras in the image

0e59dca

Fix

eb0b3d5

More flexible CLI for compressing textures

05fdbb9

lilleyse and others added 4 commits January 30, 2017 15:26

Merge branch 'master' into compress-textures

68a99a4

Renaming

d7e4e97

Treat crunch as a separate format from s3tc in compressedImage3DTiles

07669b9

Merge branch 'master' into compress-textures

41aed06

pjcozzi merged commit 795e73e into master Feb 3, 2017

pjcozzi deleted the compress-textures branch February 3, 2017 01:58

This was referenced Feb 3, 2017

BIM roadmap CesiumGS/cesium#4678

Open

Texture compression for 3D Tiles and glTF CesiumGS/cesium#4679

Closed

shehzan10 mentioned this pull request Feb 3, 2017

Add spec and readme example for using compression with optimizeGlb CesiumGS/3d-tiles-validator#48

Merged

pierreglibert mentioned this pull request Mar 13, 2020

png & jpeg to KTX GPU texture #530

Closed

Compress textures stage #204

Compress textures stage #204

Conversation

lilleyse commented Jan 4, 2017 • edited Loading

pjcozzi commented Jan 6, 2017

pjcozzi commented Jan 6, 2017

pjcozzi commented Jan 6, 2017

pjcozzi commented Jan 6, 2017

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

pjcozzi commented Jan 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Jan 6, 2017

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

pjcozzi commented Jan 6, 2017

lilleyse commented Jan 6, 2017

bagnell commented Jan 6, 2017

lilleyse commented Jan 6, 2017

pjcozzi commented Jan 26, 2017

pjcozzi commented Jan 26, 2017

pjcozzi commented Jan 26, 2017

pjcozzi commented Jan 26, 2017

pjcozzi commented Jan 26, 2017

mramato commented Jan 26, 2017

lilleyse commented Jan 30, 2017

pjcozzi commented Feb 2, 2017

pjcozzi commented Feb 2, 2017

pjcozzi commented Feb 2, 2017

pjcozzi commented Feb 2, 2017

lilleyse commented Feb 2, 2017

lilleyse commented Feb 2, 2017

lilleyse commented Feb 2, 2017

lilleyse commented Feb 2, 2017

pjcozzi commented Feb 3, 2017

lilleyse commented Jan 4, 2017 •

edited

Loading