Bug 219551 - Remove ENABLE_GRAPHICS_CONTEXT_GL by replacing it with ENABLE(WEBGL)
Summary: Remove ENABLE_GRAPHICS_CONTEXT_GL by replacing it with ENABLE(WEBGL)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-12-04 12:43 PST by Fujii Hironori
Modified: 2020-12-28 00:19 PST (History)
21 users (show)

See Also:

Attachments
Patch (48.60 KB, patch)
2020-12-04 12:49 PST, Fujii Hironori 		no flags Details | Formatted Diff | Diff
Patch for landing (48.55 KB, patch)
2020-12-05 13:01 PST, Fujii Hironori 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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?