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.

Sign up for GitHub

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

Jump to bottom

CSSColorValue as input for CanvasRenderingContext2D fill and stroke styles #6609

Closed
wants to merge 10 commits into from
Closed

CSSColorValue as input for CanvasRenderingContext2D fill and stroke styles #6609

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
68a353c
Add CSSColorValue as input for BaseRenderingContext2D fill and stroke…
mysteryDate Apr 22, 2021
8a61c7a
Use dfn and change idl too
mysteryDate Apr 23, 2021
250f21e
x
mysteryDate Apr 23, 2021
cc5f61a
Put span around interface classes
mysteryDate Apr 27, 2021
02d3953
Link to srgb conversion
mysteryDate Apr 27, 2021
315cf9d
Merge branch 'main' into CSSColorValue-input
mysteryDate May 10, 2021
ca06187
Add setter step for CSSColorValues
mysteryDate May 10, 2021
4925731
Formatting
mysteryDate May 11, 2021
768954a
Maybe I finally got the wrapping right
mysteryDate May 11, 2021
1089e54
proper citation
mysteryDate May 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62810,8 +62810,8 @@ try {

mysteryDate marked this conversation as resolved.
Show resolved Hide resolved
<p>Can be set, to change the fill style.</p>

<p>The style can be either a string containing a CSS color, or a <code>CanvasGradient</code> or
<code>CanvasPattern</code> object. Invalid values are ignored.</p>
<p>The style can be either a string containing a CSS color, a <code>CanvasGradient</code>,
<code>CanvasPattern</code> or <a href="https://drafts.css-houdini.org/css-typed-om-1/#colorvalue-objects">CSSColorValue</a> object. Invalid values are ignored.</p>
mysteryDate marked this conversation as resolved.
Show resolved Hide resolved
</dd>

<dt><var>context</var> . <code subdfn data-x="dom-context-2d-strokeStyle">strokeStyle</code> [ = <var>value</var> ]</dt>
Expand All @@ -62821,8 +62821,8 @@ try {

<p>Can be set, to change the stroke style.</p>

<p>The style can be either a string containing a CSS color, or a <code>CanvasGradient</code> or
<code>CanvasPattern</code> object. Invalid values are ignored.</p>
<p>The style can be either a string containing a CSS color, a <code>CanvasGradient</code>,
<code>CanvasPattern</code> or <a href="https://drafts.css-houdini.org/css-typed-om-1/#colorvalue-objects">CSSColorValue</a> object. Invalid values are ignored.</p>
</dd>
</dl>

Expand Down Expand Up @@ -62853,26 +62853,29 @@ try {
<p>Objects that implement the <code>CanvasFillStrokeStyles</code> interface have attributes and
methods (defined in this section) that control how shapes are treated by the object.</p>

<p>The <dfn attribute for="CanvasFillStrokeStyles"><code
data-x="dom-context-2d-fillStyle">fillStyle</code></dfn> attribute represents the color or style
to use inside shapes, and the <dfn attribute for="CanvasFillStrokeStyles"><code
data-x="dom-context-2d-strokeStyle">strokeStyle</code></dfn> attribute represents the color or
style to use for the lines around the shapes.</p>
<p>The <dfn><code data-x="dom-context-2d-fillStyle">fillStyle</code></dfn> attribute represents the
color or style to use inside shapes, and the <dfn><code data-x="dom-context-2d-strokeStyle">strokeStyle</code></dfn> attribute represents the color
or style to use for the lines around the shapes.</p>

<p>Both attributes can be either strings, <code>CanvasGradient</code>s, or
<code>CanvasPattern</code>s. On setting, strings must be <span data-x="parse a CSS &lt;color>
<p>Both attributes can be either strings, <code>CanvasGradient</code>s,
<code>CanvasPattern</code>s, or <a href="https://drafts.css-houdini.org/css-typed-om-1/#colorvalue-objects">CSSColorValue</a>s. On setting, strings must be <span data-x="parse a CSS &lt;color>
value">parsed</span> with this <code>canvas</code> element and the color assigned, and
<code>CanvasGradient</code> and <code>CanvasPattern</code> objects must be assigned themselves. If
parsing the value results in failure, then it must be ignored, and the attribute must retain its
previous value. If the new value is a <code>CanvasPattern</code> object that is marked as <span
data-x="concept-canvas-pattern-not-origin-clean">not origin-clean</span>, then the
<code>CanvasGradient</code>, <code>CanvasPattern</code> and <a href="https://drafts.css-houdini.org/css-typed-om-1/#colorvalue-objects">CSSColorValue</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What CSS context are the color values evaluated in? E.g. if the color value references a variable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can't, luckily - that would make it a CSSUnparsedValue, rather than a CSSColorValue. With the exception of CSSColor and CSSCMYK (which can reference a user-defined color space), all the CSSColorValue subclasses are self-contained and eagerly evaluatable into whatever color format you want.

That does still mean we need to specify how those two deal with user-defined color spaces, tho. Easy route is to say "it can't" and you're stuck with just the predefined color spaces (and the naive CMYK space). That's probably how we want to do it for now; making it better would be a bit difficult with shadows and such, and we shouldn't hang this on that issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about currentcolor?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a CSSKeywordValue, so similarly you don't need to worry about that here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also jumping back a sec, I'm reviewing Typed OM and see that, currently, it doesn't try to apply user-defined colorspaces in its own color conversion anyway yet. So if you have a new CSSColor("--foo", [0,1,0]), calling .toRGB() just throws currently. So doing the same thing here (eagerly checking for a custom colorspace and rejecting such CSSColor objects) is totally consistent with current practice, not just the easy way out. 👍

I anticipate we'll add a way to ref such custom colorspaces in the future, I'm just punting for now. When that happens it should end up working here as well.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about CSSMathValue coordinates? Not all math can be eagerly evaluatable, I think. E.g. what about things that represent colors like lch(0 calc(100vw / 10em) 50%)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That precise concern already exists, since you can provide that exact value as a string today. I haven't checked the spec to see if it actually addresses it (or just assumes that you're only passing simple values), but regardless, it's either undefined or already-solved for both cases, so the Typed OM object doesn't introduce anything new.

objects must be assigned themselves. If parsing the value results in failure,
then it must be ignored, and the attribute must retain its previous value. If
the new value is a <code>CanvasPattern</code> object that is marked as
<span data-x="concept-canvas-pattern-not-origin-clean">not origin-clean</span>, then the
<code>CanvasRenderingContext2D</code>'s <span
data-x="concept-canvas-origin-clean">origin-clean</span> flag must be set to false.</p>


<p>When set to a <code>CanvasPattern</code> or <code>CanvasGradient</code> object, the assignment
is <span>live</span>, meaning that changes made to the object after the assignment do affect
subsequent stroking or filling of shapes.</p>

<p>When set to a <a href="https://drafts.css-houdini.org/css-typed-om-1/#colorvalue-objects">CSSColorValue</a> object, the assignment is not live and instead
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would it mean for the assignment to be live? Aren't CSSColorValue objects immutable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're not immutable, but they are "dead". It's intended that you can reuse TypedOM values to set a property multiple times, tweaking the values on the same object rather than constantly creating new objects.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I don't quite understand the distinction. How would you mutate a CSSColorValue?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By just setting the attributes? They're not readonly.

The point is that if you, say, get the value of the 'color' property and get a CSSColorValue out of it, you can fiddle with the values and it doesn't do anything to the page's style. You have to set it back to the proeprty for it to have an effect.

(And then you can do animations more cheaply by just reusing that color object and setting styles with it in each frame, rather than getting a fresh object on each frame and adjusting them.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find any attributes on either CSSColorValue or CSSStyleValue...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what's happening now. Inputting CSSColorValue is the same as inputting the equivalent color string.

What do you mean by "processing model work"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I misunderstood the intent, because you said "objects must be assigned themselves", which implies to me that the property must have its value set to the object.

If you want to convert it into a string, then you need to define or reference the algorithm for converting a CSSColorValue into a string, and make sure that is run upon setting.

Partially this is annoying because the existing infrastructure is not using proper setter/getter steps and associated values. I might be able to work on an editorial PR to make that easier for you...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each color space needs have it's own conversion: https://drafts.css-houdini.org/css-typed-om-1/#dom-csscolorvalue-torgb
Is it enough to reference the houdini, or do they need to be defined explicitly here?

Partially this is annoying because the existing infrastructure is not using proper setter/getter steps and associated values. I might be able to work on an editorial PR to make that easier for you...

I'm not sure I understand. What is the "existing infrastructure"?

Copy link
Member

@domenic domenic Apr 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By "existing infrastructure" I mean the existing spec text. See my message below at #6609 (comment).

After the refactoring in #6614, the data model will be that the fill/stroke style are either a CSS color (not a string), a CanvasPattern, or a CanvasGradient. Note that on getting the CSS color is serialized into a string using the HTML spec's existing "serialization of a color" algorithm, which always produces either #123456 or rgba(, , ,) format values.

After this PR, what would you like the getter to return? I can see a few options:

  • In the setter. store the CSS color in RGBA format using the same algorithm that Houdini's toRGB() uses. (Houdini basically just says "convert this into an sRGB color"; I'm not sure if that's enough for an interoperable implementation.) Then, use the HTML spec's existing "serialization of a color" algorithm on the result in the getter.

  • In the setter, store the CSS color in some sort of unconverted original format. Then in the setter, serialize it using some sort of CSS serialization of that color, defined somewhere.

I think the main difference here is in cases where your CSSColorValue is outside the sRGB gamut. If you take the first option I mention, you'd convert into sRGB and lose fidelity, but fit very nicely into the existing infrastructure. Whereas if you take the second one, you get more power, but we have to do more work.

Concretely, consider:

const cssColor = CSSColorValue.parse("lch(59% 50 224)");
context.fillStyle = cssColor;

console.log(context.fillStyle);

should this log lch(59% 50 224) or #009cbc? (They are different colors.) And which should be used for drawing?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Houdini basically just says "convert this into an sRGB color"; I'm not sure if that's enough for an interoperable implementation.)

It's enough; all the algorithms are well-defined and precise, and Color 4 references them more explicitly to help implementors. (They're just from the literature, rather than being reproduced in our spec.)

AFAIK, canvas is still sRGB; there's no way to get access to values outside that space yet, right? So it should eagerly convert to an internal RGB color (and then use the existing serialization logic to always produce a hex string).

the object is converted to a color up assignment.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "converted to a color up assignment" mean? Does the fillStyle property stop returning a CSSColorValue object and start returning ... "a color"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! That's weird, I think some auto-formatting mangled my words.
I meant converted to a color. Basically:

ctx.fillstyle = new CSSRGB(1, 0, 1);
console.log(ctx.fillstyle):
> "#ff00ff"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, great, converted to a color string. That helps with some of the mutability problems discussed in the other thread.

Make sure to define the algorithm for that, instead of using the "objects must be assigned themselves" clause.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don't know what I was thinking when I wrote that. 🤦‍♀️

So right now each CSSColorValue type has a ToColor() function that returns an RGB color. Should I define the colorspace -> RGB conversions for each colorspace? Where should I do this?

Copy link
Contributor

@tabatkins tabatkins Apr 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I haven't written the serializers for the color classes yet (there's a TODO issue there in the spec), but you should be able to assume they exist if necessary.

You should be able to just say "convert to an sRGB color", tho; all the built-in spaces know how to be converted to sRGB (see https://drafts.css-houdini.org/css-typed-om/#dom-csscolorvalue-torgb for example). The internal value the spec works with is an abstract CSS color; you don't need to pretend you're working with JS objects.

So right now each CSSColorValue type has a ToColor() function that returns an RGB color. Should I define the colorspace -> RGB conversions for each colorspace? Where should I do this?

No need there, that function is specifically to turn it into a CSSColor object (similar to toRGB() turning it into CSSRGB).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a link to https://drafts.css-houdini.org/css-typed-om/#dom-csscolorvalue-torgb and explicitly mentioned sRGB.


<p>On getting, if the value is a color, then the <span data-x="serialization of a
color">serialization of the color</span> must be returned. Otherwise, if it is not a color but a
<code>CanvasGradient</code> or <code>CanvasPattern</code>, then the respective object must be
Expand Down