Bug 225173 - Add support for CanvasRenderingContext2DSettings
Summary: Add support for CanvasRenderingContext2DSettings
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 225180
  Show dependency treegraph
 
Reported: 2021-04-28 16:57 PDT by Sam Weinig
Modified: 2021-04-29 09:10 PDT (History)
12 users (show)

See Also:


Attachments
Patch (22.00 KB, patch)
2021-04-28 17:18 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (40.25 KB, patch)
2021-04-28 18:27 PDT, Sam Weinig
no flags 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-04-28 16:57:43 PDT
We should support for CanvasRenderingContext2DSettings from https://html.spec.whatwg.org/multipage/canvas.html#canvasrenderingcontext2dsettings.

These allow disabling alpha and giving a hint to the engine about desynchronization. It is also the mechanism we can use to pass color space information in for bug 225140.
Comment 1 Sam Weinig 2021-04-28 17:18:06 PDT Comment hidden (obsolete)
Comment 2 Sam Weinig 2021-04-28 18:27:59 PDT
Created attachment 427322 [details]
Patch
Comment 3 Chris Dumez 2021-04-29 08:32:54 PDT
Comment on attachment 427322 [details]
Patch

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

> Source/WebCore/html/canvas/CanvasRenderingContext2DSettings.idl:31
> +    // boolean alpha = true;

Not sure why we are commenting this out. I don't see the drawback for fully matching the spec on IDL side (and even in the implementation struct). It's not like the JS can feature-detect if we support a particular dictionary member of not.
I personally think we should have alpha in the dictionary and the implementation struct, and add a FIXME comment at the spot where the alpha struct member should be used (but isn't yet).
Comment 4 Sam Weinig 2021-04-29 08:37:04 PDT
(In reply to Chris Dumez from comment #3)
> Comment on attachment 427322 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=427322&action=review
> 
> > Source/WebCore/html/canvas/CanvasRenderingContext2DSettings.idl:31
> > +    // boolean alpha = true;
> 
> Not sure why we are commenting this out. I don't see the drawback for fully
> matching the spec on IDL side (and even in the implementation struct). It's
> not like the JS can feature-detect if we support a particular dictionary
> member of not.
> I personally think we should have alpha in the dictionary and the
> implementation struct, and add a FIXME comment at the spot where the alpha
> struct member should be used (but isn't yet).

I originally didn't have it commented out on the same principle, but it turns out you can feature detect it since HTMLCanvasElement.getContextAttributes() returns one, you can check if the returned object has an 'alpha' property.
Comment 5 Chris Dumez 2021-04-29 08:38:02 PDT
Comment on attachment 427322 [details]
Patch

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

>>> Source/WebCore/html/canvas/CanvasRenderingContext2DSettings.idl:31
>>> +    // boolean alpha = true;
>> 
>> Not sure why we are commenting this out. I don't see the drawback for fully matching the spec on IDL side (and even in the implementation struct). It's not like the JS can feature-detect if we support a particular dictionary member of not.
>> I personally think we should have alpha in the dictionary and the implementation struct, and add a FIXME comment at the spot where the alpha struct member should be used (but isn't yet).
> 
> I originally didn't have it commented out on the same principle, but it turns out you can feature detect it since HTMLCanvasElement.getContextAttributes() returns one, you can check if the returned object has an 'alpha' property.

Oh, I missed that. Please disregard my comment then.
Comment 6 Sam Weinig 2021-04-29 08:41:48 PDT
Tracking adding support for alpha in https://bugs.webkit.org/show_bug.cgi?id=225191.
Comment 7 EWS 2021-04-29 09:09:27 PDT
Committed r276777 (237161@main): <https://commits.webkit.org/237161@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427322 [details].
Comment 8 Radar WebKit Bug Importer 2021-04-29 09:10:18 PDT
<rdar://problem/77324253>