917505 - Add ETC2 compressed texture format for WebGL

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

No WebGL ext spec yet, but core in WebGL 2

http://www.khronos.org/registry/webgl/specs/latest/2.0/

See this wiki page about our WebGL 2 impl (and in particular, how to turn on webgl2)

https://wiki.mozilla.org/Platform/GFX/WebGL2

General wiki page about impl webgl extensions:

https://wiki.mozilla.org/Platform/GFX/WebGL/Contribute/Extensions

Comment 1

[Kelsey Gilbert [:jgilbert]]

We should spec this for WebGL 1.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Absolutely. That is sort of what I had unconsciously in mind when I linked to the wiki page on Extensions above ;-)

Comment 3

[u480271]

How do we go about spec'ing this for WebGL 1? Do I take https://www.khronos.org/registry/webgl/extensions/proposals/WEBGL_compressed_texture_etc1/ as an example?

Comment 4

[u480271]

Attachment: Add ETC2 compressed texture format to WebGL.

Comment 5

[u480271]

Attachment: Add ETC2 compressed texture format to WebGL.

Jeff, I'm looking for feedback on this patch to check whether it's heading in
the right direction. I've spent a lot time working on ETC2 block decompressor
in JS since that is what the S3TC conformance test does.

There is still a lot of work to be done. Off the top of my head:
 * Check sRGB texture interaction
 * "punch through" and ETC2_EAC alpha
 * single and dual channel textures

Comment 6

[Kelsey Gilbert [:jgilbert]]

(In reply to Dan Glastonbury :djg :kamidphish from comment #5)
> Created attachment 8341350 [details] [diff] [review]
> Add ETC2 compressed texture format to WebGL.
> 
> Jeff, I'm looking for feedback on this patch to check whether it's heading in
> the right direction. I've spent a lot time working on ETC2 block decompressor
> in JS since that is what the S3TC conformance test does.
> 
> There is still a lot of work to be done. Off the top of my head:
>  * Check sRGB texture interaction
>  * "punch through" and ETC2_EAC alpha
>  * single and dual channel textures

Honestly, we should probably just include an array containing the decompressed values in the test. If it passes on different drivers, we can be sure that the decompressed values we include match what we would get from completing the decompression algorithm.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Jeff, Dan, do you think one of you could pick this up again? Modern Android devices support ETC2 via GLES3 (and GLES2 with extensions), so we could really improve the games situation there without requiring them to use webgl2.

Comment 8

[Kelsey Gilbert [:jgilbert]]

We can find someone to do this.
This is why I proposed https://www.khronos.org/registry/webgl/extensions/WEBGL_compressed_texture_es3/

Comment 9

[Ethan Lin[:ethlin]]

Is there any work need to be done?

Comment 10

[Kelsey Gilbert [:jgilbert]]

(In reply to Ethan Lin[:ethlin] from comment #9)
> Is there any work need to be done?

Yes, implement at your leisure.

Comment 11

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: add etc2 support.

Comment 12

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8702479 [details] [diff] [review]
add etc2 support.

Review of attachment 8702479 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/canvas/WebGLContextExtensions.cpp
@@ +127,5 @@
>          return WebGLExtensionColorBufferFloat::IsSupported(this);
>      case WebGLExtensionID::WEBGL_compressed_texture_atc:
>          return gl->IsExtensionSupported(gl::GLContext::AMD_compressed_ATC_texture);
> +    case WebGLExtensionID::WEBGL_compressed_texture_es3:
> +        return gl->IsExtensionSupported(gl::GLContext::ARB_ES3_compatibility);

You have to add this to the 'drafts' switch, not this main one. (r-)

::: dom/canvas/test/webgl-conformance/conformance/extensions/00_test_list.txt
@@ +3,5 @@
>  oes-texture-float.html
>  oes-vertex-array-object.html
>  webgl-debug-renderer-info.html
>  webgl-debug-shaders.html
> +webgl-compressed-texture-es3.html

Add this to webgl-mochitest until we can upstream it and pull it down.

::: dom/canvas/test/webgl-conformance/conformance/extensions/webgl-compressed-texture-es3.html
@@ +92,5 @@
> +
> +debug("");
> +
> +var wtu = WebGLTestUtils;
> +var canvas = document.getElementById("canvas");

Don't bother attaching to an existing canvas unless you want it to be seen for some reason. Just use:
var canvas = document.createElement('canvas')

@@ +98,5 @@
> +var program = wtu.setupTexturedQuad(gl);
> +var ext = null;
> +var vao = null;
> +var validFormats = {
> +    COMPRESSED_R11_EAC_WEBGL                        : 0x9270,

No _WEBGL suffix.

@@ +443,5 @@
> +function copyRect(data, srcX, srcY, dstX, dstY, width, height, stride) {
> +  var bytesPerLine = width * 4;
> +  var srcOffset = srcX * 4 + srcY * stride;
> +  var dstOffset = dstX * 4 + dstY * stride;
> +  for (; height > 0; --height) {

Just use var jj like normal.

@@ +591,5 @@
> +        glErrorShouldBe(gl, gl.INVALID_OPERATION, "invalid offset");
> +    }
> +
> +    if (width < 32 && height < 32)
> +    {

) {

@@ +636,5 @@
> +    img.src = c.toDataURL();
> +    return img;
> +}
> +function compareRect(
> +        actualWidth, actualHeight, actualChannels,

function compareRect(actualWidth, actualHeight, actualChannels,

Also newline between } and function

@@ +638,5 @@
> +}
> +function compareRect(
> +        actualWidth, actualHeight, actualChannels,
> +        dataWidth, dataHeight, expectedData,
> +        testData, testFormat) {

|                  testData, testFormat)
|{

@@ +646,5 @@
> +
> +    var div = document.createElement("div");
> +    div.className = "testimages";
> +    insertImg(div, "expected", makeImage(
> +            actualWidth, actualHeight, dataWidth, expectedData,

insertImg(div, "expected", makeImage(actualWidth, actualHeight,
                                     dataWidth, expectedData,...

Really just pull the makeImage retval into a temporary variable. This will keep things clear. There no reason to put this all in the same statement.

@@ +666,5 @@
> +            switch (testFormat) {
> +              case ext.COMPRESSED_SRGB8_ETC2_WEBGL:
> +              case ext.COMPRESSED_SRGB8_PUNCHTHROUGH_ALPHA1_ETC2_WEBGL:
> +              case ext.COMPRESSED_SRGB8_ALPHA8_ETC2_EAC_WEBGL:
> +                  var convertSRGB = val => {

convertFrom/ToSRGB is common enough that it should be pulled out into a proper function, not embedded as a lambda.

@@ +700,5 @@
> +            for (var channel = 0; channel < actualChannels; ++channel) {
> +                diff[channel] = Math.abs(expected[channel] - actual[actualOffset + channel]);
> +            }
> +
> +            if(diff.some(element => element > maxDiffPixel)) {

if<space>(

But there's no need for this block to exist. Just integrate this code into the `for (var channel` block. You won't need the diff array either.

::: dom/canvas/test/webgl-conformance/conformance/resources/etc2-data.js
@@ +1,1 @@
> +var img_4x4_r11_eac = {

Include a comment header that says where you sourced this data. Also call this 'es3' not 'etc2', since the EAC ones sorta aren't ETC2.

::: dom/webidl/WebGLRenderingContext.webidl
@@ +829,5 @@
>  
>  [NoInterfaceObject]
> +interface WEBGL_compressed_texture_es3
> +{
> +    const GLenum COMPRESSED_R11_EAC_WEBGL                           = 0x9270;

These don't have _WEBGL suffixes according to the spec:
https://www.khronos.org/registry/webgl/extensions/WEBGL_compressed_texture_es3/

Comment 13

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

(In reply to Jeff Gilbert [:jgilbert] from comment #12)
> ::: dom/canvas/test/webgl-conformance/conformance/resources/etc2-data.js
> @@ +1,1 @@
> > +var img_4x4_r11_eac = {
> 
> Include a comment header that says where you sourced this data. Also call
> this 'es3' not 'etc2', since the EAC ones sorta aren't ETC2.
All data includes both compressed and decompressed are almost created by myself (I only took the original data from Dan's patch). So I won't leave comment here.

Comment 14

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: add etc2 support v2.

Comment 15

[Kelsey Gilbert [:jgilbert]]

(In reply to Morris Tseng [:mtseng] from comment #13)
> (In reply to Jeff Gilbert [:jgilbert] from comment #12)
> > ::: dom/canvas/test/webgl-conformance/conformance/resources/etc2-data.js
> > @@ +1,1 @@
> > > +var img_4x4_r11_eac = {
> > 
> > Include a comment header that says where you sourced this data. Also call
> > this 'es3' not 'etc2', since the EAC ones sorta aren't ETC2.
> All data includes both compressed and decompressed are almost created by
> myself (I only took the original data from Dan's patch). So I won't leave
> comment here.

What, no! Definitely leave a comment saying this. We need to know where this comes from, even if it's from one of us.

Ideally you would even include a high-level guide for how you pulled the data out. (Name of program or which website you got it from)

Comment 16

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

(In reply to Jeff Gilbert [:jgilbert] from comment #15)
> What, no! Definitely leave a comment saying this. We need to know where this
> comes from, even if it's from one of us.
> 
> Ideally you would even include a high-level guide for how you pulled the
> data out. (Name of program or which website you got it from)
Ok, I got it. I'll add comment in my next patch.

Comment 17

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Add WEBGL_compressed_texture_es3 support.

Add comment in es3-data.js.

Comment 18

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd3c2d9646ec

Comment 19

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=240c148d8cac

Comment 20

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Add WEBGL_compressed_texture_es3 support v2. (carry r+: jgilbert)

:smaug, would you review the webidl changes here? Thanks.

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8704952 [details] [diff] [review]
Add WEBGL_compressed_texture_es3 support v2. (carry r+: jgilbert)

r+ webidl part.

Comment 22

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c9694372b25e

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c9694372b25e

Comment 25

[Florian Scholz (Open Web Docs)]

Developer release notes:
https://developer.mozilla.org/en-US/Firefox/Releases/46#WebGL

Docs:
https://developer.mozilla.org/en-US/docs/Web/API/WEBGL_compressed_texture_es3
https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/compressedTexImage2D
https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/compressedTexSubImage2D