| Summary: | ImageBuffer subclasses should not be templates | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Kimmo Kinnunen <kkinnunen> |
| Component: | Canvas | Assignee: | 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 | ||
Pull request: https://github.com/WebKit/WebKit/pull/2161 Committed 252266@main (c5d5c98f4d5c): <https://commits.webkit.org/252266@main> Reviewed commits have been landed. Closing PR #2161 and removing active labels. |
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));