Closed
Bug 917505
Opened 11 years ago
Closed 9 years ago
Add ETC2 compressed texture format for WebGL
Categories
(Core :: Graphics: CanvasWebGL, defect)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: bjacob, Assigned: mtseng)
References
()
Details
(Keywords: dev-doc-complete, Whiteboard: webgl-extension)
Attachments
(1 file, 5 obsolete files)
(deleted),
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
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•11 years ago
|
||
We should spec this for WebGL 1.
Reporter | ||
Comment 2•11 years ago
|
||
Absolutely. That is sort of what I had unconsciously in mind when I linked to the wiki page on Extensions above ;-)
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?
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
Attachment #8341350 -
Flags: review?(jgilbert)
Attachment #8340978 -
Attachment is obsolete: true
Comment 6•11 years ago
|
||
(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.
Attachment #8341350 -
Flags: review?(jgilbert)
Updated•10 years ago
|
Keywords: dev-doc-needed
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.
Flags: needinfo?(jgilbert)
Flags: needinfo?(dglastonbury)
Comment 8•9 years ago
|
||
We can find someone to do this.
This is why I proposed https://www.khronos.org/registry/webgl/extensions/WEBGL_compressed_texture_es3/
Flags: needinfo?(dglastonbury)
Updated•9 years ago
|
Flags: needinfo?(jgilbert)
Updated•9 years ago
|
Flags: needinfo?(jgilbert)
Updated•9 years ago
|
Flags: needinfo?(jgilbert)
Comment 10•9 years ago
|
||
(In reply to Ethan Lin[:ethlin] from comment #9)
> Is there any work need to be done?
Yes, implement at your leisure.
Assignee: dglastonbury → nobody
Flags: needinfo?(jgilbert)
Assignee | ||
Comment 11•9 years ago
|
||
Attachment #8702479 -
Flags: review?(jgilbert)
Assignee | ||
Updated•9 years ago
|
Attachment #8341350 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → mtseng
Assignee | ||
Updated•9 years ago
|
Status: NEW → ASSIGNED
Comment 12•9 years ago
|
||
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/
Attachment #8702479 -
Flags: review?(jgilbert) → review-
Assignee | ||
Comment 13•9 years ago
|
||
(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.
Assignee | ||
Comment 14•9 years ago
|
||
Attachment #8703511 -
Flags: review?(jgilbert)
Assignee | ||
Updated•9 years ago
|
Attachment #8702479 -
Attachment is obsolete: true
Comment 15•9 years ago
|
||
(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)
Assignee | ||
Comment 16•9 years ago
|
||
(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.
Assignee | ||
Comment 17•9 years ago
|
||
Add comment in es3-data.js.
Attachment #8703926 -
Flags: review?(jgilbert)
Assignee | ||
Updated•9 years ago
|
Attachment #8703511 -
Attachment is obsolete: true
Attachment #8703511 -
Flags: review?(jgilbert)
Updated•9 years ago
|
Attachment #8703926 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 18•9 years ago
|
||
Assignee | ||
Comment 19•9 years ago
|
||
Assignee | ||
Comment 20•9 years ago
|
||
:smaug, would you review the webidl changes here? Thanks.
Attachment #8704952 -
Flags: review?(bugs)
Assignee | ||
Updated•9 years ago
|
Attachment #8703926 -
Attachment is obsolete: true
Comment 21•9 years ago
|
||
Comment on attachment 8704952 [details] [diff] [review]
Add WEBGL_compressed_texture_es3 support v2. (carry r+: jgilbert)
r+ webidl part.
Attachment #8704952 -
Flags: review?(bugs) → review+
Comment 22•9 years ago
|
||
Comment 23•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Comment 25•9 years ago
|
||
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
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•