Bug 203060 - WKWebView specific bug: WKWebView as the contents of a SceneKit/ARKit node doesn't work (UIWebView works)
Summary: WKWebView specific bug: WKWebView as the contents of a SceneKit/ARKit node do...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-16 16:08 PDT by Dean Jackson
Modified: 2021-09-08 19:46 PDT (History)
4 users (show)

See Also:


Attachments
Patch (10.74 KB, patch)
2019-10-16 16:23 PDT, Dean Jackson
no flags Details | Formatted Diff | Diff
Patch (15.14 KB, patch)
2021-09-08 17:22 PDT, Tim Horton
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (15.14 KB, patch)
2021-09-08 17:27 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (15.15 KB, patch)
2021-09-08 17:39 PDT, Tim Horton
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (17.15 KB, patch)
2021-09-08 17:48 PDT, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2019-10-16 16:08:01 PDT
WKWebView specific bug: WKWebView as the contents of a SceneKit/ARKit node doesn't work (UIWebView works)
Comment 1 Dean Jackson 2019-10-16 16:08:35 PDT
rdar://47481972
Comment 2 Dean Jackson 2019-10-16 16:23:30 PDT
Created attachment 381131 [details]
Patch
Comment 3 Anders Carlsson 2019-10-16 16:26:27 PDT
Shouldn't this be SPI on WKWebView instead?
Comment 4 Tim Horton 2019-10-16 16:30:03 PDT
Comment on attachment 381131 [details]
Patch

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

> Source/WebKit/ChangeLog:19
> +        SceneKit will still need to adopt this API for the WKWebView to show
> +        up in the rendering.

Kinda sad that it's a pref that needs adopting instead of magic, and not sure that that is actually OK for them?

> Source/WebKit/UIProcess/WebPageProxy.cpp:1970
> +    // If we've swapped to using mapped IOSurfaces, then we need to tell

I think this whole thing should get tucked into RemoteLayerTreeDrawingAreaProxy. Maybe even just add a generic "preferencesChanged" to DrawingAreaProxy and do this in RLTDAProxy's implementation.
WebPageProxy doesn't need to know about crazy things like this.
Comment 5 Tim Horton 2019-10-16 16:30:43 PDT
(In reply to Anders Carlsson from comment #3)
> Shouldn't this be SPI on WKWebView instead?

Also a very reasonable thought. Still would like a version that doesn't need any SPI at all though.
Comment 6 Dean Jackson 2019-10-16 16:31:27 PDT
(In reply to Anders Carlsson from comment #3)
> Shouldn't this be SPI on WKWebView instead?

I don't know. I think this way would allow us to set the preference as we create the view.

But honestly I don't mind.
Comment 7 Simon Fraser (smfr) 2019-10-16 16:40:17 PDT
Comment on attachment 381131 [details]
Patch

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

> Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.mm:92
> +    auto layerContentsType = (m_drawingArea->hasDebugIndicator() || m_drawingArea->page().preferences().useIOSurfaceBackingStoreForRemoteHosting()) ? RemoteLayerBackingStore::LayerContentsType::IOSurface : RemoteLayerBackingStore::LayerContentsType::CAMachPort;

Would be cleaner with m_drawingArea->hasDebugIndicator() || m_drawingArea->page().preferences().useIOSurfaceBackingStoreForRemoteHosting() in a lambda.
Comment 8 Dean Jackson 2019-10-17 11:03:50 PDT
Comment on attachment 381131 [details]
Patch

Holding off on this for the moment in the hope that CoreAnimation can fix it on their side, which would avoid us having to futz with our layers.
Comment 9 Radar WebKit Bug Importer 2021-09-08 17:17:42 PDT
<rdar://problem/82899923>
Comment 10 Tim Horton 2021-09-08 17:22:06 PDT
Created attachment 437688 [details]
Patch
Comment 11 Tim Horton 2021-09-08 17:27:19 PDT
Created attachment 437689 [details]
Patch
Comment 12 Dean Jackson 2021-09-08 17:28:08 PDT
Comment on attachment 437688 [details]
Patch

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

> Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.mm:78
> +    if (m_drawingArea->page().windowKind() == WindowKind::SceneKitSnapshotting)

Would it be better to have a more abstract name for this kind of snapshotting, and only reference SceneKit when you're looking at the class type?

> Source/WebKit/UIProcess/WindowKind.h:33
> +    SceneKitSnapshotting,

e.g. here.
Comment 13 Tim Horton 2021-09-08 17:29:43 PDT
(In reply to Dean Jackson from comment #12)
> Comment on attachment 437688 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=437688&action=review
> 
> > Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.mm:78
> > +    if (m_drawingArea->page().windowKind() == WindowKind::SceneKitSnapshotting)
> 
> Would it be better to have a more abstract name for this kind of
> snapshotting, and only reference SceneKit when you're looking at the class
> type?

Maybe! I should look at what they're actually doing and see if there's a way to name it more generically.
Comment 14 Tim Horton 2021-09-08 17:39:23 PDT
Created attachment 437690 [details]
Patch
Comment 15 Tim Horton 2021-09-08 17:48:09 PDT
Created attachment 437691 [details]
Patch
Comment 16 Simon Fraser (smfr) 2021-09-08 18:39:24 PDT
Comment on attachment 437691 [details]
Patch

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

> Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.mm:201
> +    if ([window isKindOfClass:NSClassFromString(@"_SCNSnapshotWindow")])

Yucky

> Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.mm:202
> +        return WindowKind::InProcessSnapshotting;

Not sure why SceneKit implies snapshotting.
Comment 17 Tim Horton 2021-09-08 19:36:31 PDT
(In reply to Simon Fraser (smfr) from comment #16)
> Comment on attachment 437691 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=437691&action=review
> 
> > Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.mm:201
> > +    if ([window isKindOfClass:NSClassFromString(@"_SCNSnapshotWindow")])
> 
> Yucky

Mmmhmm.

> > Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.mm:202
> > +        return WindowKind::InProcessSnapshotting;
> 
> Not sure why SceneKit implies snapshotting.

When you set a view as the contents of a SceneKit material, they adopt your view tree and use a custom CARenderer to flatten it into a texture. Maybe snapshotting isn't the right word because it's a fresh snapshot every frame? 🤷‍♂️
Comment 18 EWS 2021-09-08 19:46:09 PDT
Committed r282189 (241476@main): <https://commits.webkit.org/241476@main>

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