Bug 149069 - Should use CARenderServerRenderLayerWithTransform for snapshots on iOS
Summary: Should use CARenderServerRenderLayerWithTransform for snapshots on iOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-11 11:49 PDT by Beth Dakin
Modified: 2015-09-11 16:06 PDT (History)
3 users (show)

See Also:

Attachments
Patch (9.67 KB, patch)
2015-09-11 12:02 PDT, Beth Dakin 		thorton: review+
Details | Formatted Diff | Diff
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 Beth Dakin 2015-09-11 11:49:07 PDT 
We should use CARenderServerRenderLayerWithTransform for snapshots on iOS. This will let us own the backing IOSurface, which will let us make them purgeable and share more code with Mac.
Comment 1 Beth Dakin 2015-09-11 12:02:21 PDT 
Created attachment 261010 [details]
Patch
Comment 2 Tim Horton 2015-09-11 12:04:57 PDT 
Comment on attachment 261010 [details]
Patch

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

> Source/WebKit2/ChangeLog:11
> +        This will let us own the backing IOSurface, which will let us make them 
> +        purgeable and share more code with Mac.

Except we're not going to make them purgeable for now and it's not clear that that is actually the plan so maybe we shouldn't mention that part :D

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1110
> +    auto surface = WebCore::IOSurface::create(WebCore::expandedIntSize(snapshotSize), WebCore::ColorSpaceDeviceRGB);

We should do more perf testing here.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1111
> +    CARenderServerRenderLayerWithTransform(MACH_PORT_NULL, self.layer.context.contextId, reinterpret_cast<uint64_t>(self.layer), surface->surface(), 0, 0, &transform);

I wonder if we have to use some other function on older OSes, like _snapshotWhatever does? Probably.

> Source/WebKit2/UIProcess/mac/ViewSnapshotStore.h:-44
> -#if PLATFORM(MAC)

YAYYYYY
Comment 3 Tim Horton 2015-09-11 12:05:17 PDT 
Also not sure I can really review this :D
Comment 4 Beth Dakin 2015-09-11 14:25:19 PDT 
(In reply to comment #2)
> Comment on attachment 261010 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261010&action=review
> 
> > Source/WebKit2/ChangeLog:11
> > +        This will let us own the backing IOSurface, which will let us make them 
> > +        purgeable and share more code with Mac.
> 
> Except we're not going to make them purgeable for now and it's not clear
> that that is actually the plan so maybe we shouldn't mention that part :D
> 

Fixed this!

> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1110
> > +    auto surface = WebCore::IOSurface::create(WebCore::expandedIntSize(snapshotSize), WebCore::ColorSpaceDeviceRGB);
> 
> We should do more perf testing here.
> 

Perf bot watcher has been alerted! 

> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1111
> > +    CARenderServerRenderLayerWithTransform(MACH_PORT_NULL, self.layer.context.contextId, reinterpret_cast<uint64_t>(self.layer), surface->surface(), 0, 0, &transform);
> 
> I wonder if we have to use some other function on older OSes, like
> _snapshotWhatever does? Probably.
> 

Tim and I considered this, and he changed his mind.

> > Source/WebKit2/UIProcess/mac/ViewSnapshotStore.h:-44
> > -#if PLATFORM(MAC)
> 
> YAYYYYY


YAY. Thanks Tim!

http://trac.webkit.org/changeset/189628
Comment 5 Beth Dakin 2015-09-11 14:29:09 PDT 
Build fix: http://trac.webkit.org/changeset/189630
Comment 6 Beth Dakin 2015-09-11 15:34:15 PDT 
Another build fix: http://trac.webkit.org/changeset/189635
Comment 7 Beth Dakin 2015-09-11 16:06:52 PDT 
More build fix: http://trac.webkit.org/changeset/189637