Bug 219551

Summary: Remove ENABLE_GRAPHICS_CONTEXT_GL by replacing it with ENABLE(WEBGL)
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: WebGLAssignee: Fujii Hironori <Hironori.Fujii>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, benjamin, berto, cdumez, cgarcia, cmarcelo, dino, don.olmstead, ews-watchlist, graouts, gustavo, gyuyoung.kim, holymonson, jdarpinian, kbr, kkinnunen, kondapallykalyan, luiz, ryuan.choi, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Fujii Hironori 2020-12-04 12:43:19 PST 
Remove ENABLE_GRAPHICS_CONTEXT_GL by replacing it with ENABLE(WEBGL)

GraphicsContextGL is a module only for WebGL.
ENABLE_WEBGL should be removed if all ports enable it.
Comment 1 Fujii Hironori 2020-12-04 12:49:19 PST 
Created attachment 415451 [details]
Patch
Comment 2 EWS Watchlist 2020-12-04 12:49:57 PST 
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Kenneth Russell 2020-12-04 18:16:15 PST 
Comment on attachment 415451 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=415451&action=review

Looks like a nice cleanup. As long as this builds everywhere, r+.

> Source/WebCore/platform/graphics/GLContext.cpp:21
> +#if USE(OPENGL) || USE(OPENGL_ES)

Wasn't immediately obvious to me that these two #if statements are functionally equivalent.

> Source/WebCore/platform/graphics/opengl/ExtensionsGLOpenGLCommon.cpp:438
> +#endif // ENABLE(WEBGL) &&  || !USE(ANGLE)

Typo - please fix.
Comment 4 Fujii Hironori 2020-12-05 12:56:25 PST 
Comment on attachment 415451 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=415451&action=review

Thank you very much for taking time to review my patch.

>> Source/WebCore/platform/graphics/GLContext.cpp:21
>> +#if USE(OPENGL) || USE(OPENGL_ES)
> 
> Wasn't immediately obvious to me that these two #if statements are functionally equivalent.

Yup, non-cocoa ports need to enable either of the macros to use OpenGL.

>> Source/WebCore/platform/graphics/opengl/ExtensionsGLOpenGLCommon.cpp:438
>> +#endif // ENABLE(WEBGL) &&  || !USE(ANGLE)
> 
> Typo - please fix.

Will fix.
Comment 5 Fujii Hironori 2020-12-05 13:01:24 PST 
Created attachment 415496 [details]
Patch for landing
Comment 6 Fujii Hironori 2020-12-05 13:29:39 PST 
Committed r270477: <https://trac.webkit.org/changeset/270477>
Comment 7 Radar WebKit Bug Importer 2020-12-05 13:30:22 PST 
<rdar://problem/72012254>
Comment 8 monson 2020-12-27 23:26:27 PST 
Comment on attachment 415496 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=415496&action=review

> Source/cmake/OptionsGTK.cmake:-113
> -WEBKIT_OPTION_DEPEND(ENABLE_3D_TRANSFORMS ENABLE_GRAPHICS_CONTEXT_GL)
> -WEBKIT_OPTION_DEPEND(ENABLE_ASYNC_SCROLLING ENABLE_GRAPHICS_CONTEXT_GL)
> -WEBKIT_OPTION_DEPEND(ENABLE_GLES2 ENABLE_GRAPHICS_CONTEXT_GL)
> -WEBKIT_OPTION_DEPEND(ENABLE_WEBGL ENABLE_GRAPHICS_CONTEXT_GL)
>  WEBKIT_OPTION_DEPEND(USE_ANGLE_WEBGL ENABLE_WEBGL)
> -WEBKIT_OPTION_DEPEND(USE_WPE_RENDERER ENABLE_GRAPHICS_CONTEXT_GL)

I'm OK with killing ENABLE_GRAPHICS_CONTEXT_GL, but here (and somethere else) are some options depend on it. Would you consider a situation where we have neither OPENGL nor GLES2, and bring back some dependencies? I have hit the wrong assertion of USE_GSTREAMER_GL, ENABLE_3D_TRANSFORMS and ENABLE_ASYNC_SCROLLING already.
Comment 9 Fujii Hironori 2020-12-28 00:19:43 PST 
Could you open a new bug or reopen bug 219916?