Reverts https://bugs.webkit.org/show_bug.cgi?id=227519
<rdar://problem/80582699>
We need to revert all these changes: r279722 r284740 r283531
The intermediate ImageBuffers of the SVGImage need to be "really" compatible with the underlying ImageBuffer of the GraphicsContext. This is tracked by bug 235758.
Created attachment 451181 [details] Patch
Created attachment 451502 [details] Patch
Created attachment 451869 [details] Patch
Comment on attachment 451869 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=451869&action=review > Source/WebCore/svg/graphics/SVGImage.cpp:206 > + auto imageBuffer = ImageBuffer::create(size(), RenderingMode::Unaccelerated, 1, DestinationColorSpace::SRGB(), PixelFormat::BGRA8); Why is this being hard coded to SRGB?
Created attachment 452221 [details] Patch
Comment on attachment 452221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452221&action=review > Source/WebCore/svg/graphics/SVGImage.cpp:213 > + auto imageBuffer = ImageBuffer::create(size(), renderingMode, ShouldUseDisplayList::No, RenderingPurpose::DOM, 1, colorSpace, PixelFormat::BGRA8, hostWindow); Why isn't this createCompatibleBuffer()?
Comment on attachment 452221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452221&action=review >> Source/WebCore/svg/graphics/SVGImage.cpp:213 >> + auto imageBuffer = ImageBuffer::create(size(), renderingMode, ShouldUseDisplayList::No, RenderingPurpose::DOM, 1, colorSpace, PixelFormat::BGRA8, hostWindow); > > Why isn't this createCompatibleBuffer()? Because it is hard to a GraphicsContext for all the callers. Here are a few examples: - (void)getPreviewSnapshotImage:(CGImageRef*)cgImage andRects:(NSArray **)rects + (CGImageRef)imageForURL:(NSURL *)url - (WebUITextIndicatorData *)initWithImage:(CGImageRef)image textIndicatorData:(const WebCore::TextIndicatorData&)indicatorData scale:(CGFloat)scale static RetainPtr<UIImage> uiImageForImage(Image* image)
Comment on attachment 452221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452221&action=review >>> Source/WebCore/svg/graphics/SVGImage.cpp:213 >>> + auto imageBuffer = ImageBuffer::create(size(), renderingMode, ShouldUseDisplayList::No, RenderingPurpose::DOM, 1, colorSpace, PixelFormat::BGRA8, hostWindow); >> >> Why isn't this createCompatibleBuffer()? > > Because it is hard to a GraphicsContext for all the callers. Here are a few examples: > > - (void)getPreviewSnapshotImage:(CGImageRef*)cgImage andRects:(NSArray **)rects > + (CGImageRef)imageForURL:(NSURL *)url > - (WebUITextIndicatorData *)initWithImage:(CGImageRef)image textIndicatorData:(const WebCore::TextIndicatorData&)indicatorData scale:(CGFloat)scale > static RetainPtr<UIImage> uiImageForImage(Image* image) Because it is hard to *get* a GraphicsContext ...
Comment on attachment 452221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452221&action=review >>>> Source/WebCore/svg/graphics/SVGImage.cpp:213 >>>> + auto imageBuffer = ImageBuffer::create(size(), renderingMode, ShouldUseDisplayList::No, RenderingPurpose::DOM, 1, colorSpace, PixelFormat::BGRA8, hostWindow); >>> >>> Why isn't this createCompatibleBuffer()? >> >> Because it is hard to a GraphicsContext for all the callers. Here are a few examples: >> >> - (void)getPreviewSnapshotImage:(CGImageRef*)cgImage andRects:(NSArray **)rects >> + (CGImageRef)imageForURL:(NSURL *)url >> - (WebUITextIndicatorData *)initWithImage:(CGImageRef)image textIndicatorData:(const WebCore::TextIndicatorData&)indicatorData scale:(CGFloat)scale >> static RetainPtr<UIImage> uiImageForImage(Image* image) > > Because it is hard to *get* a GraphicsContext ... I see. This is just getting a NativeImage from an SVGImage without any notion of what the caller will do with it. In that case it might be risky to sometimes return an Accelerated image. For the code paths involving the GPUP, it might make more sense to create a version of this that takes the destination context.
Comment on attachment 452221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452221&action=review >>>>> Source/WebCore/svg/graphics/SVGImage.cpp:213 >>>>> + auto imageBuffer = ImageBuffer::create(size(), renderingMode, ShouldUseDisplayList::No, RenderingPurpose::DOM, 1, colorSpace, PixelFormat::BGRA8, hostWindow); >>>> >>>> Why isn't this createCompatibleBuffer()? >>> >>> Because it is hard to a GraphicsContext for all the callers. Here are a few examples: >>> >>> - (void)getPreviewSnapshotImage:(CGImageRef*)cgImage andRects:(NSArray **)rects >>> + (CGImageRef)imageForURL:(NSURL *)url >>> - (WebUITextIndicatorData *)initWithImage:(CGImageRef)image textIndicatorData:(const WebCore::TextIndicatorData&)indicatorData scale:(CGFloat)scale >>> static RetainPtr<UIImage> uiImageForImage(Image* image) >> >> Because it is hard to *get* a GraphicsContext ... > > I see. This is just getting a NativeImage from an SVGImage without any notion of what the caller will do with it. In that case it might be risky to sometimes return an Accelerated image. > > For the code paths involving the GPUP, it might make more sense to create a version of this that takes the destination context. I can add the new version for nativeImage in a separate patch. Or maybe I can pass an optional pointer to GraphicsContext to the existing one.
Committed r289981 (247367@main): <https://commits.webkit.org/247367@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452221 [details].