Bug 202836 - Enable WebGL's ASTC extension all the time
Summary: Enable WebGL's ASTC extension all the time
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kenneth Russell
URL:
Keywords: InRadar
: 194751 (view as bug list)
Depends on: 185272 197900
Blocks: webglangle
  Show dependency treegraph
 
Reported: 2019-10-10 17:51 PDT by Kenneth Russell
Modified: 2020-02-18 16:48 PST (History)
11 users (show)

See Also:


Attachments
Patch (6.36 KB, patch)
2019-11-08 16:11 PST, Kenneth Russell
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2019-10-10 17:51:39 PDT
In Bug 185272 ASTC compressed texture support was added to the WebGL implementation. Currently it's behind a run-time flag. It should be removed from behind this flag and just enabled whenever the underlying OpenGL (ES) extensions are available, so it auto-enables whenever sufficient support is present.
Comment 1 Kenneth Russell 2019-11-07 16:24:08 PST
Blocking on the ETC1/ETC2 bug, which alphabetizes the compressed texture extensions in some of the code.
Comment 2 Kenneth Russell 2019-11-08 16:11:16 PST
Created attachment 383175 [details]
Patch
Comment 3 Kenneth Russell 2019-11-08 16:16:01 PST
Built on macOS. Still building for the iOS Simulator, with WebKit modified to always use an OpenGL ES 3.0 context, to verify the WebGL conformance test passes.
Comment 4 Kenneth Russell 2019-11-09 01:19:58 PST
I built this patch for the iOS Simulator, plus a couple of changes to GraphicsContext3DCocoa.mm to force the use of ES 3.0 contexts:


diff --git a/Source/WebCore/platform/graphics/cocoa/GraphicsContext3DCocoa.mm b/Source/WebCore/platform/graphics/cocoa/GraphicsContext3DCocoa.mm
index d1af803f38d..6a7c5eaa598 100644
--- a/Source/WebCore/platform/graphics/cocoa/GraphicsContext3DCocoa.mm
+++ b/Source/WebCore/platform/graphics/cocoa/GraphicsContext3DCocoa.mm
@@ -192,6 +192,8 @@ GraphicsContext3D::GraphicsContext3D(GraphicsContext3DAttributes attrs, HostWind
     m_powerPreferenceUsedForCreation = GraphicsContext3DPowerPreference::Default;
 #endif
 
+    m_attrs.isWebGL2 = true;
+
 #if !USE(ANGLE)
 #if USE(OPENGL_ES)
     if (m_attrs.isWebGL2)
@@ -215,6 +217,7 @@ GraphicsContext3D::GraphicsContext3D(GraphicsContext3DAttributes attrs, HostWind
 
     if (m_attrs.isWebGL2)
         ::glEnable(GraphicsContext3D::PRIMITIVE_RESTART_FIXED_INDEX);
+    m_isForWebGL2 = m_attrs.isWebGL2;
 #elif USE(OPENGL)
 
     bool useMultisampling = m_attrs.antialias;





Unfortunately https://www.khronos.org/registry/webgl/sdk/tests/conformance/extensions/webgl-compressed-texture-astc.html still reported no support for ASTC textures on the iPhone SE for WebKit Development.

Appreciate others' help getting this patch tested and over the finish line. Thanks.
Comment 5 shrekshao 2019-11-25 16:05:12 PST
I know this patch is for NonAngle case.
The debugging shows it's Extensions3DOpenGLCommon::m_availableExtensions that doesn't include the ASTC extension string. Given that it is initialized by calling glGetStringi, I doubt if the iPhone Simulator on Mac could ever populate such extension if the underlying gpu hardware doesn't support the ASTC extension? If this could be verified I think we could only test this patch on real iPhone. Could Dean or Justin help to verify this? Thanks!
Comment 6 Dean Jackson 2019-12-03 16:11:25 PST
(In reply to shrekshao from comment #5)
> I know this patch is for NonAngle case.
> The debugging shows it's Extensions3DOpenGLCommon::m_availableExtensions
> that doesn't include the ASTC extension string. Given that it is initialized
> by calling glGetStringi, I doubt if the iPhone Simulator on Mac could ever
> populate such extension if the underlying gpu hardware doesn't support the
> ASTC extension? If this could be verified I think we could only test this
> patch on real iPhone. Could Dean or Justin help to verify this? Thanks!

I'll test, but from what I recall, ASTC is only supported on device (and as Ken pointed out, > iPhone SE)
Comment 7 Dean Jackson 2019-12-03 16:13:02 PST
Oh. I also think ASTC was only supported on devices when using a GLES 3 context, not a GLES 2 context :(

That's why Justin put it behind a flag for now.
Comment 8 Dean Jackson 2019-12-03 16:13:59 PST
Comment on attachment 383175 [details]
Patch

r=me

I'll test manually before landing this.
Comment 9 Kenneth Russell 2019-12-03 16:59:42 PST
Thanks Dean for helping with this. Please see the diff in https://bugs.webkit.org/show_bug.cgi?id=202836#c4 which was needed to force WebGL to use an ES3 context by default. Still the simulator didn't appear to support ASTC compressed textures - maybe not surprising since the emulation path would be much more complex than ETC? ETC is supported by the simulator.
Comment 10 Radar WebKit Bug Importer 2019-12-04 10:02:49 PST
<rdar://problem/57627592>
Comment 11 Dean Jackson 2019-12-04 10:03:03 PST
I confirmed this works on devices (that themselves support ASTC).
Comment 12 Radar WebKit Bug Importer 2019-12-04 10:03:09 PST
<rdar://problem/57627600>
Comment 13 Dean Jackson 2019-12-04 10:05:30 PST
Committed r253108: <https://trac.webkit.org/changeset/253108>
Comment 14 Kenneth Russell 2019-12-04 11:07:39 PST
Thank you Dean for testing and landing this!
Comment 15 Kenneth Russell 2020-02-18 16:48:08 PST
*** Bug 194751 has been marked as a duplicate of this bug. ***