Bug 217374 - Mac: Remove OpenGL and OpenGL ES backends
Summary: Mac: Remove OpenGL and OpenGL ES backends
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Local Build
Hardware: Mac All
: P2 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on:
Blocks: 217213
  Show dependency tree / graph
 
Reported: 2020-10-06 03:51 PDT by Kimmo Kinnunen
Modified: 2020-10-08 17:14 PDT (History)
20 users (show)

See Also:

Attachments
Patch (148.00 KB, patch)
2020-10-07 05:25 PDT, Kimmo Kinnunen 		no flags Details | Formatted Diff | Diff
Patch (148.27 KB, patch)
2020-10-08 02:00 PDT, Kimmo Kinnunen 		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 Kimmo Kinnunen 2020-10-06 03:51:03 PDT 
Remove OpenGL (CGL) and OpenGL ES (EAGL) backends. Make ANGLE the only backend.
Current layering violations and technical dept makes it too laboursome to implement the blocking bugs.
Comment 1 Kimmo Kinnunen 2020-10-07 05:25:02 PDT 
Created attachment 410745 [details]
Patch
Comment 2 Darin Adler 2020-10-07 08:06:53 PDT 
Comment on attachment 410745 [details]
Patch

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

Looks great.

> Source/WebCore/platform/graphics/GraphicsContextGL.h:780
> +

Should not leave this extra blank line.

> Source/WebCore/platform/graphics/cocoa/WebGLLayer.h:38
> +

Should not add this extra blank line here.

> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:52
> +namespace {

We mostly still use static rather than anonymous namespace for this kind of thing in WebKit.

> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:53
> +// TODO: Fix style to allow multi-line raw strings.

Not an important follow-up so I would omit this comment. Also. WebKit usies FIXME, not TODO.

> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:54
> +#define SHADER(text) #text##_s

Why do we need this macro at all? It does not seem to add any helpful abstraction.

> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:56
> +const ASCIILiteral s_rgbVertexShader { SHADER(

constexpr auto or static constexpr auto?
Comment 3 Dean Jackson 2020-10-07 09:42:51 PDT 
Comment on attachment 410745 [details]
Patch

Awesome!
Comment 4 Kimmo Kinnunen 2020-10-08 02:00:10 PDT 
Created attachment 410819 [details]
Patch
Comment 5 EWS 2020-10-08 11:56:50 PDT 
Committed r268198: <https://trac.webkit.org/changeset/268198>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 410819 [details].
Comment 6 Radar WebKit Bug Importer 2020-10-08 11:57:29 PDT 
<rdar://problem/70103808>
Comment 7 Alex Christensen 2020-10-08 17:14:30 PDT 
Has this day really finally come?  Exciting!