Bug 223121 - Simulated WebGL context screen change events should work with GPU process
Summary: Simulated WebGL context screen change events should work with GPU process
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on:
Blocks: webglgpup 219669
  Show dependency treegraph
 
Reported: 2021-03-12 07:50 PST by Kimmo Kinnunen
Modified: 2021-03-15 04:34 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.92 KB, patch)
2021-03-12 07:54 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (4.20 KB, patch)
2021-03-12 10:33 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (3.98 KB, patch)
2021-03-12 11:07 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (4.20 KB, patch)
2021-03-15 03:26 PDT, 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 2021-03-12 07:50:36 PST
Simulated WebGL context screen change events should work with GPU process

Bug 219669 tried to fix it, but did not succeed

fails:
     fast/canvas/webgl/webglcontextchangedevent.html
Comment 1 Kimmo Kinnunen 2021-03-12 07:54:24 PST
Created attachment 423048 [details]
Patch
Comment 2 Kimmo Kinnunen 2021-03-12 10:33:46 PST
Created attachment 423060 [details]
Patch
Comment 3 Kimmo Kinnunen 2021-03-12 11:07:10 PST
Created attachment 423067 [details]
Patch
Comment 4 Darin Adler 2021-03-12 15:10:36 PST
Comment on attachment 423067 [details]
Patch

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

> Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h:118
> +#if ENABLE(WEBGL)
> +    void dispatchDisplayWasReconfiguredForTesting() { dispatchDisplayWasReconfigured(); };
> +#endif

Style-wise, a while back we found that using boolean expressions and separate #if was clearer, and easier to keep correct when refactoring than nested #if. So this would be nice as:

#endif
#if PLATFORM(MAC) && ENABLE(WEBGL)
    // ...
#endif

Please consider that form. I’d suggest that above in the .cpp file too for the same reason.

> Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGL.cpp:281
> +        callOnMainRunLoop([gpuConnectionToWebProcess = m_gpuConnectionToWebProcess]() {
> +            if (auto connectionToWeb = gpuConnectionToWebProcess.get())
> +                connectionToWeb->dispatchDisplayWasReconfiguredForTesting();
> +        });

You don’t have to take my suggestions, but I think the best names for these two would be: weakConnection (the capture) and connection (the local).
Comment 5 Kimmo Kinnunen 2021-03-15 03:26:12 PDT
Created attachment 423152 [details]
Patch
Comment 6 EWS 2021-03-15 04:33:34 PDT
Committed r274412: <https://commits.webkit.org/r274412>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423152 [details].
Comment 7 Radar WebKit Bug Importer 2021-03-15 04:34:16 PDT
<rdar://problem/75425294>