Bug 240058

Summary: ImageBuffer subclasses should not be templates
Product: WebKit Reporter: Kimmo Kinnunen <kkinnunen>
Component: CanvasAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   

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.