Bug 240058 - ImageBuffer subclasses should not be templates
Summary: ImageBuffer subclasses should not be templates
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-05-04 02:20 PDT by Kimmo Kinnunen
Modified: 2022-07-08 01:53 PDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kimmo Kinnunen 2022-05-04 02:20:12 PDT
ImageBuffer subclasses should not be templates. Current ConcreteImageBuffer<ImageBufferBackend> causes the final subclass to be templated. This is overly complicated class hierarchy.

This causes the holders not being able to hold image buffer subclasses generically. This causes all the needed generic subclass properties to leak into main WebCore::ImageBuffer.

Concrete examples:
ImageBuffer::clearBackend only makes sense for RemoteImageBufferProxy<>. However, it must be in ImageBuffer, since RemoteResourceCache cannot hold on to RemoteImageBufferProxy<>

RemoteImageBufferProxy would need IPC messages, but cannot have such as there's no such class RemoteImageBufferProxy, rather RemoteImageBufferProxy<>. IPC system cannot generate messages for templates, and it wouldn't make sense if it could.

ImageBuffer is already doing work with virtual methods.
ImageBufferBackend is already doing work with virtual methods.
It's pointless to provide a mixin ConcreteImageBuffer that just complicates things.

There's no ImageBuffers without ConcreteImageBuffer. 

To fix:
Merge ImageBuffer and ConcreteImageBuffer
The new ImageBuffer calls virtual functions of ImageBufferBackend


imageBuffer = DisplayListAcceleratedImageBuffer::create(size, resolutionScale, colorSpace, pixelFormat, purpose, creationContext);
->
imageBuffer = DisplayListImageBuffer::create(AcceleratedImageBufferBackend::create(size, resolutionScale, colorSpace, pixelFormat, purpose, creationContext));


imageBuffer = AcceleratedImageBuffer::create(size, resolutionScale, colorSpace, pixelFormat, purpose, creationContext);
->
imageBuffer = PlatformImageBuffer::create(AcceleratedImageBufferBackend::create(size, resolutionScale, colorSpace, pixelFormat, purpose, creationContext));


imageBuffer = ConcreteImageBuffer<ImageBufferShareableBitmapBackend>::create(size, resolutionScale, colorSpace, PixelFormat::BGRA8, RenderingPurpose::ShareableSnapshot, { });
->
imageBuffer = ImageBuffer::create(ImageBufferShareableBitmapBackend::create(size, resolutionScale, colorSpace, PixelFormat::BGRA8, RenderingPurpose::ShareableSnapshot, { }));



Ref<RemoteImageBufferProxy> imageBuffer = AcceleratedRemoteImageBufferMappedProxy::create(size, resolutionScale, colorSpace, pixelFormat, purpose, *this);
->
Ref<RemoteImageBufferProxy> imageBuffer = RemoteImageBufferProxy::create(size, resolutionScale, colorSpace, pixelFormat, purpose, renderingMode, *this));
Comment 1 Radar WebKit Bug Importer 2022-05-11 02:21:12 PDT
<rdar://problem/93085945>
Comment 2 Said Abou-Hallawa 2022-07-07 02:42:33 PDT
Pull request: https://github.com/WebKit/WebKit/pull/2161
Comment 3 EWS 2022-07-08 01:53:18 PDT
Committed 252266@main (c5d5c98f4d5c): <https://commits.webkit.org/252266@main>

Reviewed commits have been landed. Closing PR #2161 and removing active labels.