Bug 225173 – Add support for CanvasRenderingContext2DSettings

• Component: Canvas
• Status: RESOLVED FIXED

[Sam Weinig]

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.

[Sam Weinig]

Created attachment 427312 [details]
Patch

[Sam Weinig]

Created attachment 427322 [details]
Patch

[Chris Dumez]

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).

[Sam Weinig]

(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.

[Chris Dumez]

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.

[Sam Weinig]

Tracking adding support for alpha in https://bugs.webkit.org/show_bug.cgi?id=225191.

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/77324253>

[Brent Fulgham]

*** Bug 193144 has been marked as a duplicate of this bug. ***

[Anne van Kesteren]

*** Bug 196002 has been marked as a duplicate of this bug. ***