Bug 217216 - Implement RemoteGraphicsContextGL
Summary: Implement RemoteGraphicsContextGL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Mac Unspecified
: P1 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on: 217213 219431 219486 219487
Blocks: webglgpup
  Show dependency treegraph
 
Reported: 2020-10-02 04:30 PDT by Kimmo Kinnunen
Modified: 2020-12-09 10:32 PST (History)
23 users (show)

See Also:


Attachments
Patch (315.83 KB, patch)
2020-11-03 05:26 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (353.51 KB, patch)
2020-11-22 05:10 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (374.34 KB, patch)
2020-11-23 02:23 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (510.23 KB, patch)
2020-11-30 08:06 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (549.54 KB, patch)
2020-12-01 02:25 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (547.54 KB, patch)
2020-12-01 03:20 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (551.13 KB, patch)
2020-12-01 03:33 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (551.12 KB, patch)
2020-12-01 03:48 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (551.12 KB, patch)
2020-12-01 03:52 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (551.48 KB, patch)
2020-12-01 04:05 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (554.69 KB, patch)
2020-12-01 04:20 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (554.89 KB, patch)
2020-12-01 04:22 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (554.76 KB, patch)
2020-12-01 04:33 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (555.36 KB, patch)
2020-12-01 04:44 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (568.52 KB, patch)
2020-12-01 05:57 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (568.66 KB, patch)
2020-12-01 08:17 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (551.98 KB, patch)
2020-12-02 03:10 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (555.53 KB, patch)
2020-12-02 06:06 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (556.13 KB, patch)
2020-12-02 06:11 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (556.37 KB, patch)
2020-12-02 06:46 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (567.83 KB, patch)
2020-12-02 07:03 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (567.88 KB, patch)
2020-12-02 07:13 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (567.90 KB, patch)
2020-12-02 07:20 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (571.17 KB, patch)
2020-12-03 01:44 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (402.85 KB, patch)
2020-12-03 05:44 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (403.75 KB, patch)
2020-12-04 01:42 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (403.41 KB, patch)
2020-12-09 00:05 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (404.37 KB, patch)
2020-12-09 00:34 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (405.19 KB, patch)
2020-12-09 01:30 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (406.15 KB, patch)
2020-12-09 04:07 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kimmo Kinnunen 2020-10-02 04:30:36 PDT
Implement RemoteWebGLBackend that creates RemoteGraphicsContextsGL instances
Comment 1 Radar WebKit Bug Importer 2020-10-02 04:30:50 PDT
<rdar://problem/69876258>
Comment 2 Kimmo Kinnunen 2020-11-03 05:26:38 PST
Created attachment 413049 [details]
Patch
Comment 3 Kimmo Kinnunen 2020-11-22 05:10:26 PST
Created attachment 414787 [details]
Patch
Comment 4 Kimmo Kinnunen 2020-11-23 02:23:31 PST
Created attachment 414797 [details]
Patch
Comment 5 Dean Jackson 2020-11-25 01:24:56 PST
Comment on attachment 414797 [details]
Patch

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

This looks great to me!

> Source/WebCore/platform/graphics/GraphicsContextGL.h:675
> +        REQUESTABLE_EXTENSIONS_ANGLE = 0x93A8, // NOLINT

What does the NOLINT mean?

> Source/WebKit/WebProcess/GPU/graphics/RemoteGraphicsContextGLProxyFunctionsGenerated.h:30
> +    void activeTexture(GCGLenum texture) final

People might complain about the indentation here.

> Source/WebKit/WebProcess/GPU/graphics/RemoteGraphicsContextGLProxyFunctionsGenerated.h:147
> +        sendSync(Messages::RemoteGraphicsContextGL::CreateFramebuffer(), Messages::RemoteGraphicsContextGL::CreateFramebuffer::Reply(returnValue), m_graphicsContextGLIdentifier, 1_s);

I wonder if we should have a local sendSync form that calls sendSync adding the m_graphicsContextGLIdentifier and timeout?
Comment 6 Kimmo Kinnunen 2020-11-30 08:06:23 PST
Created attachment 415024 [details]
Patch
Comment 7 Kimmo Kinnunen 2020-12-01 02:25:11 PST
Created attachment 415117 [details]
Patch
Comment 8 Kimmo Kinnunen 2020-12-01 03:20:23 PST
Created attachment 415121 [details]
Patch
Comment 9 Kimmo Kinnunen 2020-12-01 03:33:51 PST
Created attachment 415122 [details]
Patch
Comment 10 Kimmo Kinnunen 2020-12-01 03:48:10 PST
Created attachment 415123 [details]
Patch
Comment 11 Kimmo Kinnunen 2020-12-01 03:52:59 PST
Created attachment 415124 [details]
Patch
Comment 12 Kimmo Kinnunen 2020-12-01 04:05:15 PST
Created attachment 415125 [details]
Patch
Comment 13 Kimmo Kinnunen 2020-12-01 04:20:09 PST
Created attachment 415126 [details]
Patch
Comment 14 Kimmo Kinnunen 2020-12-01 04:22:17 PST
Created attachment 415127 [details]
Patch
Comment 15 Kimmo Kinnunen 2020-12-01 04:33:16 PST
Created attachment 415128 [details]
Patch
Comment 16 Kimmo Kinnunen 2020-12-01 04:44:26 PST
Created attachment 415129 [details]
Patch
Comment 17 Kimmo Kinnunen 2020-12-01 05:57:04 PST
Created attachment 415131 [details]
Patch
Comment 18 Kimmo Kinnunen 2020-12-01 08:17:46 PST
Created attachment 415135 [details]
Patch
Comment 19 Kimmo Kinnunen 2020-12-02 03:10:52 PST
Created attachment 415202 [details]
Patch
Comment 20 Kimmo Kinnunen 2020-12-02 06:06:26 PST
Created attachment 415216 [details]
Patch
Comment 21 Kimmo Kinnunen 2020-12-02 06:11:35 PST
Created attachment 415218 [details]
Patch
Comment 22 Kimmo Kinnunen 2020-12-02 06:46:43 PST
Created attachment 415219 [details]
Patch
Comment 23 Kimmo Kinnunen 2020-12-02 07:03:28 PST
Created attachment 415220 [details]
Patch
Comment 24 Kimmo Kinnunen 2020-12-02 07:13:39 PST
Created attachment 415221 [details]
Patch
Comment 25 Kimmo Kinnunen 2020-12-02 07:20:38 PST
Created attachment 415223 [details]
Patch
Comment 26 Kimmo Kinnunen 2020-12-03 01:44:08 PST
Created attachment 415285 [details]
Patch
Comment 27 Kimmo Kinnunen 2020-12-03 05:44:45 PST
Created attachment 415297 [details]
Patch
Comment 28 Kimmo Kinnunen 2020-12-04 01:42:04 PST
Created attachment 415394 [details]
Patch
Comment 29 Simon Fraser (smfr) 2020-12-08 14:22:02 PST
Comment on attachment 415394 [details]
Patch

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

> Source/WebCore/page/Chrome.cpp:553
> +    // TODO: GPU process. Change to GCGLManager, make it work, return null for others.

Maybe file a bug and reference it here to track the work.

> Source/WebCore/platform/graphics/RemoteGraphicsContextGLProxyBase.cpp:48
> +void RemoteGraphicsContextGLProxyBase::setContextVisibility(bool)
> +{
> +}

Should these implementations ASSERT_NOT_REACHED?

> Source/WebCore/platform/graphics/cocoa/GraphicsContextGLIOSurfaceSwapChain.h:49
> +        void* handle { nullptr }; // Producer specific metadata handle (such as EGLSurface).

It's unclear what the ownership model is for this handle.

> Source/WebKit/WebProcess/GPU/graphics/RemoteGraphicsContextGLProxy.h:379
> +    GraphicsContextGLIdentifier m_graphicsContextGLIdentifier { GraphicsContextGLIdentifier::generate() };

This object identifier does not guarantee uniqueness between WebContent processes, but it must.
Comment 30 Kimmo Kinnunen 2020-12-09 00:05:07 PST
Created attachment 415724 [details]
Patch
Comment 31 Kimmo Kinnunen 2020-12-09 00:34:04 PST
Created attachment 415727 [details]
Patch
Comment 32 Kimmo Kinnunen 2020-12-09 01:30:45 PST
Created attachment 415729 [details]
Patch
Comment 33 Kimmo Kinnunen 2020-12-09 04:07:02 PST
Created attachment 415739 [details]
Patch
Comment 34 EWS 2020-12-09 10:32:57 PST
Committed r270587: <https://trac.webkit.org/changeset/270587>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 415739 [details].