Bug 239874

Summary: [GPU Process] NativeImage should share the ShareableBitmap data when it's sent to GPU Process
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: ImagesAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, cdumez, ews-watchlist, gyuyoung.kim, ryuan.choi, sabouhallawa, sergio, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
simon.fraser: review+
Patch none

Description Said Abou-Hallawa 2022-04-28 15:03:44 PDT
Instead of allocating a new memory buffer in GPUProcess and copying the ShareableBitmap data to it, we can create a PlatformImage that directly references this data. This will make all the NativeImages data be attributed to WebProcess.
Comment 1 Said Abou-Hallawa 2022-04-28 15:04:47 PDT
rdar://92031359
Comment 2 Said Abou-Hallawa 2022-04-28 15:41:04 PDT
Created attachment 458550 [details]
Patch
Comment 3 Said Abou-Hallawa 2022-04-29 12:10:26 PDT
Created attachment 458600 [details]
Patch
Comment 4 Said Abou-Hallawa 2022-05-02 02:05:00 PDT
Created attachment 458668 [details]
Patch
Comment 5 Chris Dumez 2022-05-04 16:25:19 PDT
Comment on attachment 458668 [details]
Patch

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

> Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp:350
> +    auto image = NativeImage::create(bitmap->createPlatformImage(DontCopyBackingStore, true), nativeImageResourceIdentifier.object());

The `true` here is not really readable. I suggest using an enum class to replace this boolean to make call sites more readable.

> Source/WebKit/Shared/ShareableBitmap.h:118
> +    RetainPtr<CGImageRef> makeCGImage(bool shouldInterpolate = false);

parameter should be an enum class : bool.

> Source/WebKit/Shared/ShareableBitmap.h:120
> +    WebCore::PlatformImagePtr createPlatformImage(WebCore::BackingStoreCopy = WebCore::CopyBackingStore, bool shouldInterpolate = false);

ditto.

> Source/WebKit/Shared/ShareableBitmap.h:127
> +    WebCore::PlatformImagePtr createPlatformImage(WebCore::BackingStoreCopy = WebCore::CopyBackingStore, bool shouldInterpolate = false) { return createCairoSurface(); }

ditto.

> Source/WebKit/Shared/ShareableBitmap.h:134
> +    RetainPtr<CGImageRef> createCGImage(CGDataProviderRef, bool shouldInterpolate) const;

ditto.

> Source/WebKit/Shared/cg/ShareableBitmapCG.cpp:178
> +    RetainPtr<CGDataProvider> dataProvider = adoptCF(CGDataProviderCreateWithData(this, data(), sizeInBytes(), [](void* typelessBitmap, const void* typelessData, size_t) {

auto

> Source/WebKit/Shared/cg/ShareableBitmapCG.cpp:179
> +        ShareableBitmap* bitmap = static_cast<ShareableBitmap*>(typelessBitmap);

auto*
Comment 6 Said Abou-Hallawa 2022-05-05 00:59:57 PDT
Created attachment 458856 [details]
Patch
Comment 7 EWS 2022-05-05 11:01:29 PDT
Committed r293846 (250317@main): <https://commits.webkit.org/250317@main>

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