Texture compression #4758

bagnell · 2016-12-15T00:50:02Z

Adds support for:

DXT, PVR, and ECT1 texture compression when the extensions are available.
loading KTX files for compressed textures.
models that link to an external KTX file or embed the KTX file binary.

There are a few features of KTX files that I didn't implement, but if they are important I can add support in this PR:

Switch to little endian if the file is big endian
Make file metadata available (key-value pairs)
3D textures
Texture arrays
Cubemaps
Mipmaps

TODO:

Add test for models with an embedded KTX image.
Add test for binary glTF models with an embedded KTX image.
Update CHANGES.md
Squash commits
Test with Natural Earth

pjcozzi · 2016-12-15T02:20:19Z

There are a few features of KTX files that I didn't implement, but if they are important I can add support in this PR...

Just document that these are not supported in the reference doc.

Later, as the glTF spec for this stabilizes, we'll add whatever features we need to be fully conformant.

pjcozzi · 2016-12-15T02:22:24Z

Is the plan to also add ASTC to this PR?

pjcozzi · 2016-12-15T02:24:24Z

Source/Renderer/Context.js

@@ -271,7 +271,10 @@ define([
        this._depthTexture = !!getExtension(gl, ['WEBGL_depth_texture', 'WEBKIT_WEBGL_depth_texture']);
        this._textureFloat = !!getExtension(gl, ['OES_texture_float']);
        this._fragDepth = !!getExtension(gl, ['EXT_frag_depth']);
-        this._debugShaders = getExtension(gl, ['WEBGL_debug_shaders']);
+        this._debugShaders = !!getExtension(gl, ['WEBGL_debug_shaders']);
+        this._s3tc = !!getExtension(gl, ['WEBGL_compressed_s3tc', 'MOZ_WEBGL_compressed_texture_s3tc', 'WEBKIT_WEBGL_compressed_texture_s3tc']);


Here and below, do you know if the 'MOZ_and WEBKIT_ prefixes are in widespread use? If this is just for older browsers, we can drop them.

I'm not sure about the MOZ_ prefix, but the WEBKIT_ prefix was on the iPad I tested.

What the WEBKIT_ prefix the only option? If so, just leave these.

Yes, it was the only one.

pjcozzi · 2016-12-15T02:30:28Z

Source/Scene/Model.js

@@ -1395,6 +1399,21 @@ define([
        };
    }

+    function ktxLoad(model, id) {


Why is this needed? Because of internalFormat? Could this be combined with imageLoad?

pjcozzi · 2016-12-15T02:31:16Z

Can you please provide a summary of the current glTF changes you made, e.g., can now reference .ktx, how it works with binary glTF, how it works when embedded, etc.?

pjcozzi · 2016-12-15T02:35:03Z

Source/Core/loadKTX.js

+        var endianness = view.getUint32(byteOffset, true);
+        byteOffset += sizeOfUint32;
+        if (endianness !== endiannessTest) {
+            // TODO: Switch endianness?


Actually given how trivial this will be, I would just do it now.

I'm going to ignore this for now. I couldn't find a big endian file to test with without generating one from the same code. The KTX library has an issue to add tests as well: KhronosGroup/KTX-Software#26

pjcozzi · 2016-12-15T02:37:50Z

Specs/Core/loadKTXSpec.js

+    var fakeXHR;
+
+    beforeEach(function() {
+        fakeXHR = jasmine.createSpyObj('XMLHttpRequest', ['send', 'open', 'setRequestHeader', 'abort', 'getAllResponseHeaders']);


Is this copy and pasted a few times in the tests? Perhaps it should be a utility in the Specs root directory?

Yes, all of the load* functions have similar tests. Since many of them are slightly different, I'm not going to do it in this PR.

pjcozzi · 2016-12-15T02:39:59Z

@bagnell wow, this looks really good.

Once we have the summary, #4758 (comment), I can start discussing with the glTF folks.

pjcozzi · 2016-12-15T02:40:42Z

Part of #4679 / #4678

bagnell · 2016-12-15T18:33:28Z

Is the plan to also add ASTC to this PR?

I added it but couldn't test it. I have the extension on my Galaxy S6 but it isn't reported by WebGL Report. It also isn't on WebGL stats. Is it a draft extension?

https://www.khronos.org/registry/webgl/extensions/WEBGL_compressed_texture_astc/

bagnell · 2016-12-15T20:02:52Z

Can you please provide a summary of the current glTF changes you made, e.g., can now reference .ktx, how it works with binary glTF, how it works when embedded, etc.?

The image is loaded as a KTX is the extension is .ktx.
Binary images are parsed as a KTX file when the mime type is image/ktx
Embedded KTX files as a data uri do not currently work. I'm looking into it.

The KTX files must be a single 2D texture. The texture can be compressed or uncompressed.
If the texture is compressed and the format is unsupported, an exception will be thrown. The app is responsible for requesting a glTF with supported compressed texture formats. If the KTX has mipmaps, only the level 0 texture is loaded.

… them from KTX files.

bagnell · 2016-12-19T22:47:12Z

@pjcozzi This is ready for another look. I squashed the commits to remove all of the binary test data from the history during implementation.

pjcozzi · 2016-12-20T13:56:07Z

These three ShaderProgram tests fail in this branch and pass in master:

Renderer/ShaderProgram fails vertex shader compile
Expected function to throw RuntimeError.
Error: Expected function to throw RuntimeError.
    at stack (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1475:17)
    at buildExpectationResult (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1445:14)
    at Spec.expectationResultFactory (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:584:18)
    at Spec.addExpectationResult (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:332:34)
    at Expectation.addExpectationResult (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:528:21)
    at Expectation.toThrowRuntimeError (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1399:12)
    at http://localhost:8080/Specs/Renderer/ShaderProgramSpec.js:523:12
    at Object.<anonymous> (http://localhost:8080/Specs/spec-main.js:139:30)
    at attemptAsync (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1793:24)
    at QueueRunner.run (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1746:26)
Renderer/ShaderProgram fails fragment shader compile
Expected function to throw RuntimeError.
Error: Expected function to throw RuntimeError.
    at stack (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1475:17)
    at buildExpectationResult (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1445:14)
    at Spec.expectationResultFactory (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:584:18)
    at Spec.addExpectationResult (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:332:34)
    at Expectation.addExpectationResult (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:528:21)
    at Expectation.toThrowRuntimeError (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1399:12)
    at http://localhost:8080/Specs/Renderer/ShaderProgramSpec.js:537:12
    at Object.<anonymous> (http://localhost:8080/Specs/spec-main.js:139:30)
    at attemptAsync (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1793:24)
    at QueueRunner.run (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1746:26)
Renderer/ShaderProgram fails to link
Expected function to throw RuntimeError.
Error: Expected function to throw RuntimeError.
    at stack (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1475:17)
    at buildExpectationResult (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1445:14)
    at Spec.expectationResultFactory (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:584:18)
    at Spec.addExpectationResult (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:332:34)
    at Expectation.addExpectationResult (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:528:21)
    at Expectation.toThrowRuntimeError (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1399:12)
    at http://localhost:8080/Specs/Renderer/ShaderProgramSpec.js:551:12
    at Object.<anonymous> (http://localhost:8080/Specs/spec-main.js:139:30)
    at attemptAsync (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1793:24)
    at QueueRunner.run (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1746:26)

pjcozzi · 2016-12-20T14:04:58Z

Source/Core/loadKTX.js

+     * @see {@link http://www.w3.org/TR/cors/|Cross-Origin Resource Sharing}
+     * @see {@link http://wiki.commonjs.org/wiki/Promises/A|CommonJS Promises/A}
+     */
+    function loadKTX(urlOrBuffer, headers) {


@bagnell we should encourage users to use this broadly. What do you think about adding this to a Sandcastle example? Perhaps the materials one? Just use a small simple KTX file like the Cesium logo.

pjcozzi · 2016-12-20T14:07:31Z

Source/Core/loadKTX.js

+            texture = new Uint8Array(texture.buffer, 0, levelSize);
+        }
+
+        return {


Can we document this type? Even if it is a trivial constructor + readonly properties?

I don't know if there is an easier way to document this (other than making the type) like how we can document callback functions.

pjcozzi · 2016-12-20T14:09:30Z

Source/Core/loadKTX.js

+        }
+
+        if (!PixelFormat.validate(glInternalFormat)) {
+            throw new RuntimeError('glInternalFormat is not a valid format.');


Document all exceptions. Are you sure these are runtime errors? I think they are developer errors. For example: https://github.com/AnalyticalGraphicsInc/cesium/blob/3d-tiles/Source/Scene/Batched3DModel3DTileContent.js#L215

Wouldn't all file loading/parsing be a runtime error? The developer might not have control over the file. KML and GeoJSON work this way. Shouldn't parsing a 3D tile also throw runtime errors when they fail to parse?

pjcozzi · 2016-12-20T14:09:56Z

Source/Core/loadKTX.js

+            throw new RuntimeError('3D textures are unsupported.');
+        }
+
+        // TODO: support texture arrays and cubemaps


Remove the TODO here and below, and just add bullets for the limitations to the reference doc.

pjcozzi · 2016-12-20T14:10:17Z

Source/Core/loadKTX.js

+        var bytesOfKeyValueByteSize = view.getUint32(byteOffset, true);
+        byteOffset += sizeOfUint32;
+
+        // TODO: read metadata? At least need to check for KTXorientation


This TODO as well.

pjcozzi · 2016-12-20T14:27:27Z

@bagnell just those comments.

Can you please also open a separate PR to merge this into 3d-tiles? I am comfortable doing that today even before the glTF spec/extension work.

lilleyse · 2016-12-21T16:18:53Z

I'm having trouble loading the samples on my machine.

Box-textured-ktx-binary

Box-textured-ktx-embedded

Box-textured-ktx

I have these extensions:

WEBGL_compressed_texture_etc1
WEBGL_compressed_texture_s3tc
WEBKIT_WEBGL_compressed_texture_s3tc

…he file format.

Add crunch compressed texture support

pjcozzi · 2017-01-10T01:22:37Z

Source/Core/loadCRN.js

+     * @see {@link http://www.w3.org/TR/cors/|Cross-Origin Resource Sharing}
+     * @see {@link http://wiki.commonjs.org/wiki/Promises/A|CommonJS Promises/A}
+     */
+    function loadCRN(urlOrBuffer, headers) {


Just want to bump my question in the other PR about if we can implement some of these in terms of a generic @private load* function to avoid some duplication?

I'd rather keep them separate. The only thing that is the same is checking if the parameter is a url or array buffer.

pjcozzi · 2017-01-10T01:24:12Z

Apps/Sandcastle/gallery/Materials.html

+    Sandcastle.declare(applyCompressedTextureMaterial); // For highlighting in Sandcastle.
+
+    var compressedImageUrl;
+    var context = scene.context;


Context is not part of the public API. You could expose these on Scene or perhaps a scene.getTextureFormatSupported(String) function. Whatever you think is clean.

pjcozzi · 2017-01-10T01:27:11Z

Really nice job, just those comments @bagnell.

We'll also have to park this PR while we work on the glTF spec, but this branch should come into master well before 3d-tiles.

pjcozzi · 2017-01-26T13:43:31Z

Related glTF Pipeline changes: CesiumGS/gltf-pipeline#204

pjcozzi · 2017-01-26T13:44:06Z

@bagnell given that this is cleanly going into a glTF extras property, it will be OK to merge this into master when ready; we'll also update the online model converter.

bagnell · 2017-01-26T21:11:04Z

@pjcozzi This has been updated. Just waiting of the gltf-pipeline changes to generate the binary and embedded test models.

bagnell · 2017-01-30T21:21:15Z

@pjcozzi This is ready for another look.

pjcozzi · 2017-02-02T12:33:21Z

Source/Scene/Model.js

@@ -1419,8 +1419,8 @@ define([
                var gltfImage = images[textures[id].source];
                var extras = gltfImage.extras;

-                var binary;
-                var uri;
+                var binary = undefined;


Good catch!

pjcozzi · 2017-02-02T12:43:12Z

Also merged into 3d-tiles, e05898a

pjcozzi reviewed Dec 15, 2016

View reviewed changes

pjcozzi mentioned this pull request Dec 19, 2016

Texture compression for 3D Tiles and glTF #4679

Closed

8 tasks

Add support for s3tc, pvrtc, and etc1 compressed textures and loading…

4cc7559

… them from KTX files.

bagnell force-pushed the texture-compression branch from 6020a01 to 4cc7559 Compare December 19, 2016 22:45

Move exported function to the top

cd3b02a

pjcozzi reviewed Dec 20, 2016

View reviewed changes

pjcozzi added 2 commits December 20, 2016 09:16

Style

797e43b

Update CHANGES.md

a9fc4f9

bagnell added 2 commits December 20, 2016 16:09

Merge branch 'master' into texture-compression

0235cec

Fix failing tests.

ca72704

bagnell added 2 commits December 21, 2016 14:18

Add ktx loading support to materials. Update Sandcastle and tests.

64dd9b4

Document type returned from loadKTX and doc unsupported features of t…

da6d1c6

…he file format.

bagnell and others added 2 commits January 9, 2017 14:56

Update crunch.js and update licenses.

73fc589

Merge pull request #4814 from AnalyticalGraphicsInc/crunch

304d32a

Add crunch compressed texture support

bagnell mentioned this pull request Jan 10, 2017

Add texture compression to 3d-tiles #4837

Merged

pjcozzi reviewed Jan 10, 2017

View reviewed changes

lilleyse mentioned this pull request Jan 12, 2017

Texture Compression Roadmap CesiumGS/gltf-pipeline#210

Closed

11 tasks

pjcozzi mentioned this pull request Jan 24, 2017

Notes for ETC2/EAC sample WebGLSamples/WebGL2Samples#60

Closed

bagnell added 3 commits January 26, 2017 13:50

Merge branch 'master' into texture-compression

40f8bcd

Update parsing glTF compressed textures.

ee85f5c

Add getCompressedTextureFormatSupported to Scene.

d7d82d0

bagnell added 3 commits January 30, 2017 14:52

Merge branch 'master' into texture-compression

d1270e1

Update test models with compressed textures. Fix binary glTF.

7b9c700

Update texture specs after merge.

3bd7004

bagnell and others added 7 commits January 30, 2017 16:30

Fix loading multiple textures.

324200c

Update ETC1 test.

f6ee997

Fix failing CI tests.

9574914

Merge branch 'master' into texture-compression

4b713b9

Update CHANGES.md.

697877c

Merge branch 'master' into texture-compression

3938f7b

Tweak CHANGES.md

ffa2554

pjcozzi reviewed Feb 2, 2017

View reviewed changes

pjcozzi merged commit b6b3ad2 into master Feb 2, 2017

pjcozzi deleted the texture-compression branch February 2, 2017 12:33

pjcozzi mentioned this pull request Feb 3, 2017

BIM roadmap #4678

Open

53 tasks

Texture compression #4758

Texture compression #4758

Conversation

bagnell commented Dec 15, 2016 • edited Loading

pjcozzi commented Dec 15, 2016

pjcozzi commented Dec 15, 2016

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 Dec 15, 2016

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 Dec 15, 2016

pjcozzi commented Dec 15, 2016

bagnell commented Dec 15, 2016

bagnell commented Dec 15, 2016

bagnell commented Dec 19, 2016

pjcozzi commented Dec 20, 2016

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 Dec 20, 2016

lilleyse commented Dec 21, 2016

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 10, 2017

pjcozzi commented Jan 26, 2017

pjcozzi commented Jan 26, 2017

bagnell commented Jan 26, 2017

bagnell commented Jan 30, 2017

Choose a reason for hiding this comment

pjcozzi commented Feb 2, 2017

bagnell commented Dec 15, 2016 •

edited

Loading