Jukka Jylänki Reporter
	Description • 7 years ago

The new WebGL draft extension WEBGL_compressed_texture_s3tc_srgb adds support for DXTn/S3TC compressed textures in sRGB color space. Draft spec here:

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

It would be good to have this extension in Firefox, naturally behind the enable-draft-extensions pref until the draft becomes community approved. This support was asked by folks over at Unity, apparently Chrome already supports it.

	Benjamin Cooley
	Comment 1 • 7 years ago

This is the corresponding Chromium bug and discussion:

https://bugs.chromium.org/p/chromium/issues/detail?id=630498

	Kelsey Gilbert [:jgilbert]
	Comment 3 • 7 years ago

Comment on attachment 8872795 [details] [diff] [review]
0001-Bug-1325113-Implement-support-for-WEBGL_compressed_t.patch

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

::: dom/canvas/WebGLFormats.cpp
@@ +332,5 @@
> +    // EXT_texture_compression_s3tc_srgb
> +    AddFormatInfo(FOO(COMPRESSED_SRGB_S3TC_DXT1_EXT ), 0, 1,1,1,0, 0,0, UnsizedFormat::RGB , false, ComponentType::NormUInt);
> +    AddFormatInfo(FOO(COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT), 0, 1,1,1,1, 0,0, UnsizedFormat::RGBA, false, ComponentType::NormUInt);
> +    AddFormatInfo(FOO(COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT), 0, 1,1,1,1, 0,0, UnsizedFormat::RGBA, false, ComponentType::NormUInt);
> +    AddFormatInfo(FOO(COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT), 0, 1,1,1,1, 0,0, UnsizedFormat::RGBA, false, ComponentType::NormUInt);

2nd-to-last arg should be `true` not `false`, since this is an SRGB format.

::: dom/webidl/WebGLRenderingContext.webidl
@@ +827,5 @@
> +    const GLenum COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT  = 0x8C4E;
> +    const GLenum COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT  = 0x8C4F;
> +};
> +
> +

Just one blank newline here, please!

::: gfx/gl/GLContext.cpp
@@ +149,4 @@
>      "GL_EXT_texture3D",
>      "GL_EXT_texture_compression_dxt1",
>      "GL_EXT_texture_compression_s3tc",
> +    "GL_EXT_texture_compression_s3tc_srgb",

You forgot to remove this.

::: gfx/gl/GLContext.h
@@ +414,4 @@
>          EXT_texture3D,
>          EXT_texture_compression_dxt1,
>          EXT_texture_compression_s3tc,
> +        EXT_texture_compression_s3tc_srgb,

You forgot to remove this.

	Kelsey Gilbert [:jgilbert]
	Comment 5 • 7 years ago

Post the link to a try run for this change before I can land this.

	Samuel Vargas [:svargas] Assignee
	Comment 6 • 7 years ago

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

	Kelsey Gilbert [:jgilbert]
	Comment 7 • 7 years ago

Comment on attachment 8872800 [details] [diff] [review]
0001-Bug-1325113-Implement-support-for-WEBGL_compressed_t.patch

Needs DOM peer review for webidl/bindings changes.

	Pulsebot
	Comment 8 • 7 years ago

Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b693eb4b9ef6
Implement support for WEBGL_compressed_texture_s3tc_srgb - r=jgilbert,qdot

	Carsten Book [:Tomcat]
	Comment 9 • 7 years ago
bugherder

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

	Florian Scholz (Open Web Docs)
	Comment 10 • 7 years ago

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

New page:
https://developer.mozilla.org/en-US/docs/Web/API/WEBGL_compressed_texture_s3tc_srgb

Updated pages:
https://developer.mozilla.org/en-US/docs/Web/API/WebGL_API#Extensions
https://developer.mozilla.org/en-US/docs/Web/API/WebGL_API/Using_Extensions
https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/getExtension
https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/getSupportedExtensions
https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/getParameter
https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/compressedTexImage2D
https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/compressedTexSubImage2D

Compat data PR:
https://github.com/mdn/browser-compat-data/pull/246