Bug 225286 - Add preliminary support for specifying a color space for 2D canvas
Summary: Add preliminary support for specifying a color space for 2D canvas
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks: 225140
  Show dependency treegraph
 
Reported: 2021-05-02 17:58 PDT by Sam Weinig
Modified: 2021-05-05 11:12 PDT (History)
17 users (show)

See Also:


Attachments
Patch (38.91 KB, patch)
2021-05-03 08:48 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (42.25 KB, patch)
2021-05-03 16:38 PDT, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (38.13 KB, patch)
2021-05-03 16:43 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (38.13 KB, patch)
2021-05-03 16:45 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (38.17 KB, patch)
2021-05-03 18:29 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (38.13 KB, patch)
2021-05-04 09:06 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (39.70 KB, patch)
2021-05-05 08:53 PDT, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2021-05-02 17:58:20 PDT
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.
Comment 1 Sam Weinig 2021-05-03 08:48:15 PDT Comment hidden (obsolete)
Comment 2 Sam Weinig 2021-05-03 16:38:47 PDT Comment hidden (obsolete)
Comment 3 Sam Weinig 2021-05-03 16:43:38 PDT Comment hidden (obsolete)
Comment 4 Sam Weinig 2021-05-03 16:45:12 PDT Comment hidden (obsolete)
Comment 5 Sam Weinig 2021-05-03 18:29:36 PDT Comment hidden (obsolete)
Comment 6 Sam Weinig 2021-05-04 09:06:17 PDT
Created attachment 427674 [details]
Patch
Comment 7 Dean Jackson 2021-05-04 09:16:02 PDT
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 8 Simon Fraser (smfr) 2021-05-04 09:42:48 PDT
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?
Comment 9 Sam Weinig 2021-05-05 08:46:01 PDT
(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?
Comment 10 Sam Weinig 2021-05-05 08:53:47 PDT
Created attachment 427770 [details]
Patch
Comment 11 EWS 2021-05-05 09:37:03 PDT
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].
Comment 12 Radar WebKit Bug Importer 2021-05-05 09:38:31 PDT
<rdar://problem/77561169>