Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-paint-api] Remove alpha flag, in favour of a contextSettings flag? #435

Closed
bfgeek opened this issue Aug 1, 2017 · 3 comments
Closed

Comments

@bfgeek
Copy link
Contributor

bfgeek commented Aug 1, 2017

Currently we have:

registerPaint('foo', class {
  static alpha = false;
});

Do we want this to be:

registerPaint('foo', class {
  static contextSettings = {
    alpha: false;
  };
});

This is to allow other context settings in the future in the same object, e.g. colorSpace.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Remove alpha flag, in favour of a contextSettings flag?, and agreed to the following resolutions:

  • RESOLVED: object called contextAttributes
The full IRC log of that discussion <iank_> Topic: Remove alpha flag, in favour of a contextSettings flag?
<iank_> Github: https://github.com//issues/435
<fantasai> iank_: I noticed recently that the alpha flag is a little silly and not really future-proof
<fantasai> iank_: There's maybe some other arguments appearing here, e.g. color space
<fantasai> iank_: We don't want to keep on adding flags to the class, maybe makes sense to have a dictionary
<fantasai> franremy: Couldn't you just add on top of alpha? Why make the change?
<fantasai> iank_: Mirrors better the ???? API
<fantasai> s/????/getContext
<fantasai> iank_: However, I'm quite fine keeping the static ????
<fantasai> franremy: Not a preference, just wondering why changes
<fantasai> surma: I think mirroring getContext is a good idea. Also see getAlias call being useful here, so makes sense to add the objection
<fantasai> iank_: Do you have a better name than settings?
<fantasai> surma: options? :)
<fantasai> dino: We should reuse ... [looking stuff up]
<fantasai> iank_: canvasContextCreationAttributes?
<fantasai> dino: Yeah, contextCreationAttributes
<fantasai> dino: We know we're in a context, so creationAttributes
<fantasai> dino: But we know its attributes so maybe just creation
<fantasai> dino: I don't think we should use the word settings
<fantasai> dino: creationAttributions
<fantasai> TabAtkins: Can you make it longer, please?
<fantasai> Rossen: canvasContextCreationAttributes
<fantasai> TabAtkins: Yup, thanks
<fantasai> iank_: Let's do a twitter poll, and pick the most popular
<fantasai> surma: Just use contextAttributes. That's how MDN lists it for getAttribute2D
<fantasai> Rossen: Sounds like we're going with the boject, calling it contextAttributes
<fantasai> Rossen: which is nicely in MDN already
<fantasai> Rossen: Opinions or objections?
<fantasai> RESOLVED: object called contextAttributes

@domenic
Copy link
Contributor

domenic commented Aug 27, 2017

Why attributes? We already have enough things on the platform called attributes (IDL and content attributes). With my web dev hat on I'd pick "options".

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 7, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
WPT-Export-Revision: 2154acfa05ba60cb7c9af8ebc5a3c355b604b972
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 7, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
WPT-Export-Revision: 024eb3007fdd2e57f7323e4ba004b04bb9f914a8
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 7, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
WPT-Export-Revision: cf056d79a5d78337c94947cf5dcc0ea680039425
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 7, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
WPT-Export-Revision: 26985404b9aaf5fc52fb6f58603bd039d70ef448
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 7, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
WPT-Export-Revision: 7c098c9bf1f58e3936aed5d948a583c04f6fdb17
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 7, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
WPT-Export-Revision: 73d24beae249f31138080941b98ccfe21ee8ebf5
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 7, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
WPT-Export-Revision: 3e24b31aa0b2d51e0c43af71ee3f0a6f7b03342a
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 8, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
WPT-Export-Revision: d8226bd2c447d966a1cab21d9d87469897bddcd6
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 8, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
WPT-Export-Revision: 7f333b106f7691a99ba5d83bfc5b6e83ea07ba7e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 10, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
WPT-Export-Revision: 3d894952521770e77cef8cac571479b016f0fae9
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 11, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
WPT-Export-Revision: b853a65b2a32a0a5382e0f849a07f4e3009303c8
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 11, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500822}
MXEBot pushed a commit to mirror/chromium that referenced this issue Sep 11, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500822}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 11, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500822}
rachelandrew pushed a commit to rachelandrew/web-platform-tests that referenced this issue Nov 8, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500822}
@bfgeek
Copy link
Contributor Author

bfgeek commented Nov 9, 2017

This has been fixed, and is now contextOptions.

@bfgeek bfgeek closed this as completed Nov 9, 2017
jakearchibald pushed a commit to jakearchibald/web-platform-tests that referenced this issue Nov 16, 2017
It has been resolved here:
w3c/css-houdini-drafts#435
That we should replace the alpha flag with a
PaintRenderingContext2DSettings object.

This CL creates a dictionary item named "PaintRenderingContext2DSettings",
currently this dictionary has only a boolean flag "alpha". This CL
also replaces all the places that uses the boolean with the
context_settings.

This CL also does a tiny refactor in PaintWorkletGlobalScope, where
the part of the code that parses "inputArguments", "inputProperties" etc,
are moved to functions in an anonymous namespace.

This CL should not cause any behavior change. We also added some more
layout tests to make sure that the settings are parsed correctly.

Bug: 578252
Change-Id: I1e01f10250a839cc91632841ee12ecbfa8509faa
Reviewed-on: https://chromium-review.googlesource.com/621746
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500822}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants