As a first step toward implementing https://github.com/WICG/canvas-color-space/blob/main/CanvasColorSpaceProposal.md, let's start with adding support for specifying a color space via the settings object and rendering using that color space. To keep things small, we will leave out ImageData access from this first change.
Created attachment 427564 [details] Patch
Created attachment 427614 [details] Patch
Created attachment 427615 [details] Patch
Created attachment 427617 [details] Patch
Created attachment 427628 [details] Patch
Created attachment 427674 [details] Patch
Comment on attachment 427674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427674&action=review > LayoutTests/fast/canvas/CanvasRenderingContext2DSettings-colorSpace-disabled.html:11 > + let canvas = document.createElement("canvas"); > + let context = canvas.getContext("2d", settings); Nit: const (and elsewhere) > LayoutTests/fast/canvas/canvas-color-space-display-p3.html:5 > + <p>This test passes if the square below looks to be all one color</p> Nice test!
Comment on attachment 427674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427674&action=review > Source/WebCore/html/HTMLCanvasElement.cpp:951 > + else > + return std::pair { DestinationColorSpace::SRGB, PixelFormat::BGRA8 }; No else after return. > Source/WebCore/html/canvas/PredefinedColorSpace.h:34 > +enum class PredefinedColorSpace { Maybe this should be CanvasPredefinedColorSpace?
(In reply to Simon Fraser (smfr) from comment #8) > Comment on attachment 427674 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=427674&action=review > > > Source/WebCore/html/HTMLCanvasElement.cpp:951 > > + else > > + return std::pair { DestinationColorSpace::SRGB, PixelFormat::BGRA8 }; > > No else after return. Yeah. (I kind of hate this rule. The symmetry seems so much nicer in cases like this, but if I want to change this rule I will bring it up separately). Fixed. > > > Source/WebCore/html/canvas/PredefinedColorSpace.h:34 > > +enum class PredefinedColorSpace { > > Maybe this should be CanvasPredefinedColorSpace? I'd like to stick to the spec name if I can. Perhaps we should suggest changing it HTML?
Created attachment 427770 [details] Patch
Committed r277024 (237338@main): <https://commits.webkit.org/237338@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 427770 [details].
<rdar://problem/77561169>