Bug 231062 – SVG images drawn onto display-p3 canvas are flattened to sRGB

• Status: RESOLVED
• Resolution: FIXED
• Component: Canvas

[Cameron McCormack (:heycam)]

.

[Radar WebKit Bug Importer]

<rdar://problem/83752451>

[Cameron McCormack (:heycam)]

We draw the SVG image into a NativeImage so it can be sent to the GPU process.

[Cameron McCormack (:heycam)]

Created attachment 439822 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Simon Fraser (smfr)]

Comment on attachment 439822 [details]
Patch

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

> Source/WebCore/platform/graphics/GraphicsContext.h:433
> +    virtual ImageDrawResult drawImageForCanvas(Image&, const FloatRect& destination, const FloatRect& source, const ImagePaintingOptions&, DestinationColorSpace canvasColorSpace);

Not a fan of "for canvas" down here in GraphicsContext.

Can we just an std::optional<DestinationColorSpace> to an existing function?

> Source/WebCore/svg/graphics/SVGImage.h:102
> +    ImageDrawResult drawForCanvas(GraphicsContext&, const FloatRect& destination, const FloatRect& source, const ImagePaintingOptions&, DestinationColorSpace) final;
> +    ImageDrawResult drawAsNativeImage(GraphicsContext&, const FloatRect& destination, const FloatRect& source, const ImagePaintingOptions&, DestinationColorSpace);
>      ImageDrawResult drawForContainer(GraphicsContext&, const FloatSize containerSize, float containerZoom, const URL& initialFragmentURL, const FloatRect& dstRect, const FloatRect& srcRect, const ImagePaintingOptions& = { });
> +    ImageDrawResult drawForContainerInternal(GraphicsContext&, const FloatSize containerSize, float containerZoom, const URL& initialFragmentURL, const FloatRect& dstRect, const FloatRect& srcRect, const ImagePaintingOptions&, DestinationColorSpace);
> +    ImageDrawResult drawInternal(GraphicsContext&, const FloatRect& destination, const FloatRect& source, const ImagePaintingOptions&, DestinationColorSpace);
> +    ImageDrawResult drawForCanvasForContainer(GraphicsContext&, const FloatSize containerSize, float containerZoom, const URL& initialFragmentURL, const FloatRect& dstRect, const FloatRect& srcRect, const ImagePaintingOptions&, DestinationColorSpace);

That's quite a bit of proliferation too. Can std::optional<DestinationColorSpace> help?

[Cameron McCormack (:heycam)]

(In reply to Simon Fraser (smfr) from comment #5)
> Can we just an std::optional<DestinationColorSpace> to an existing function?

That was my first thought.  There were many subclasses to add that to.  Maybe that's better than adding the separate functions here.

[Cameron McCormack (:heycam)]

Created attachment 439920 [details]
Patch with Image::draw / GraphicsContext::drawImage argument

[Cameron McCormack (:heycam)]

There's a version of the patch with a std::optional<DestinationColorSpace> argument.  And I could be convinced not to make it optional (or std::optional<>) so that all drawImage/draw callers have to pass in an intermediate color space.

[Cameron McCormack (:heycam)]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/31108

[Cameron McCormack (:heycam)]

Created attachment 440139 [details]
Patch

[EWS]

Committed r283531 (242497@main): <https://commits.webkit.org/242497@main>

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