CanvasFilter objects as input to CanvasRenderingContext2D.filter attribute

[mysteryDate]

• At least two implementers are interested (and none opposed):
  • Chromium
  • Webkit: see litherum's comment here and the webkit-dev thread
• Tests are written and can be reviewed and commented upon at:
  • https://github.com/web-platform-tests/wpt/tree/master/html/canvas/element/manual/filters
  • https://github.com/web-platform-tests/wpt/tree/master/html/canvas/element/filters
  • https://github.com/web-platform-tests/wpt/tree/master/html/canvas/offscreen/filters
• Implementation bugs are filed:
  • Chrome: crbug.com/1169216
  • Firefox: (https://bugzilla.mozilla.org/show_bug.cgi?id=1745995)
  • Safari: https://bugs.webkit.org/show_bug.cgi?id=234298

WhatWG bug: #5621
Original propsal: https://github.com/fserb/canvas2D/blob/master/spec/filters.md
TAG review thread: w3ctag/design-reviews#627
Webkit-dev thread: https://lists.webkit.org/pipermail/webkit-dev/2021-May/031840.html
Mozilla standards thread: mozilla/standards-positions#519

---

/canvas.html ( diff )
/infrastructure.html ( diff )
/references.html ( diff )

[annevk]

@whatwg/canvas please review as well.

[domenic]

Pulling out a separate thread from the data model to discuss some input questions:

Currently the input is defined as CanvasFilterDictionary which has four properties. I'm a bit unclear: is the intent to support only those four types of filters? Do blur, colorMatrix, convolveMatrix, and componentTransfer corresponding directly to specific filters from https://drafts.fxtf.org/filter-effects-1/#FilterPrimitivesOverview ? If so we need a translation table since you seem to not be using the same names, e.g. I think blur corresponds to feGaussianBlur?

If you want to support arbitrary filters using the same names as the SVG spec, then record<> is probably better than CanvasFilterDictionary, so that you can operate generically. But if you want to specifically opt in only some filters, with new names, then yeah, you need a custom dictionary.

---

What happens if I fill in more than one member of the dictionary? All the examples seem to only fill in a single one.

---

I assume you want to allow a pipeline to contain multiple of the same type of filter, right? E.g.

new CanvasFilter([
  { blur: ... },
  { colorMatrix: ... },
  { blur: ... }
]);

Otherwise we could get rid of the array, i.e. transform

const f = new CanvasFilter([
  { blur: ... },
  { colorMatrix: ... }
]);

into

const f = new CanvasFilter({
  blur: ...,
  colorMatrix: ...
});

(taking advantage of the fact that JavaScript objects are ordered.)

---

You can probably get more type-safety, and get the bindings layer to do more work for you, by using record<> instead of object for the value. E.g. instead of object blur you could use record<DOMString, DOMString> blur or maybe record<DOMString, (DOMString or long long or sequence<long long> or boolean)>.

Of course, you'd get maximal type-safety---and I think it would also benefit ecosystems like TypeScript, which generate IDE autocomplete from the IDL---if you created separate dictionaries for each filter type, which contained exactly the right properties with exactly the right types. But that would require serious maintenance work to keep in sync with the SVG filters spec. (Probably we'd want to define those dictionaries in the SVG filter spec and just reference them from HTML...)

[kdashg]

I don't think this is agreed upon

[mysteryDate]

I was basing this off of the conclusions of the TAG review (w3ctag/design-reviews#627) I'm okay with leaving this note out for the time being if it makes other implementers uncomfortable and TAG is okay with it.

[mysteryDate]

is the intent to support only those four types of filters?

No, these four were the top requested by our users/partners. The plan is to eventually support all, though some make little sense in canvas world. For example, feFlood is just fillRect, but if we have a simple and extensible way of supporting ALL filters I don't see why not have things like feFlood, for the weirdos who hate fillRect for some reason. (Insert something about Hyrum's law here).

Also, there was a preference expressed by Apple to keep things modest initially in this thread: "In practice, most content uses only a fairly small set of filter types. If we're going to add filter support, we should start with a small set of popular filters, and only expand the set if compelling needs arise."

If so we need a translation table since you seem to not be using the same names, e.g. I think blur corresponds to feGaussianBlur?

To be honest that was an arbitrary decision made by me when writing the initial proof-of-concept. I'd still love to drop the "fe" prefix though.

If you want to support arbitrary filters using the same names as the SVG spec, then record<> is probably better than CanvasFilterDictionary, so that you can operate generically.

Spec-wise that sounds good, but given Apple's reluctance to bite this all off at once that feels unwise. Each one of these does require some work on the user-agent side.

What happens if I fill in more than one member of the dictionary? All the examples seem to only fill in a single one.

Good question. In my head these dictionaries were really designed as individual stages, but having multiple members filled out will simply apply those filter stages in the order they appear. Perhaps this is another point in favor of record.

pipeline to contain multiple of the same type of filter,

Yeah, totally necessary.

You can probably get more type-safety, and get the bindings layer to do more work for you, by using record<> instead of object for the value. E.g. instead of object blur you could use record<DOMString, DOMString> blur or maybe record<DOMString, (DOMString or long long or sequence or boolean)>.

I like this, though the idea with using plain-old dictionaries was one put for by @fserb to reduce the amount of changes to IDL files that would be needed. Thoughts @fserb?

[domenic]

Based on what you've said so far, my recommendation is that you do something like the following. It takes into account the following desires:

• Easy to extend to other filter types in the future; just expand the "supported filter names" list.
• Leverage the existing filter spec as much as possible, instead of having to create per-filter spec or IDL.
• Make these APIs behave like "strongly typed" parts of the platform for the attribute values, instead of just saying the values match XML and are always strings.

typedef record<DOMString, record<DOMString, any>> CanvasFilterInput;

[Exposed=(Window,Worker)]
interface CanvasFilter {
  constructor(CanvasFilterInput or sequence<CanvasFilterInput>) filters);
};

Define the supported filter names to be « "gaussianBlur", "colorMatrix", "convolveMatrix", "componentTransfer" ».

Define an XML filter as a struct whose items are a name, a string, and attributes, an ordered map of strings to strings.

Define the IDL type for the canvas filter attribute attrName using an algorithm that looks at the grammar definition and returns one of boolean, long long, sequence<long long>, or DOMString.

Then your code to assemble the normalized form as part of the constructor looks something like:

1. Let xmlFilters be an empty list.
2. If filters is a CanvasFilterInput, then set filters to « filters ».
3. For each filterDict of filters:
   1. For each name → attributes of filterDict:
      1. If name is not one of the [supported filter names], then continue. (Or throw?)
      2. Let xmlName be the concatenation of "fe", the first code unit of name converted to ASCII uppercase, and all code units of name after the first one.
      3. Let xmlFilter be a new [XML filter] whose [name] is xmlName and whose [attributes] is an empty ordered map.
      4. Append xmlFilter to xmlFilters.
      5. For each attrName → attrValue of attributes:
         1. If any of the following are true:
            • attrName is not listed as an attribute for the filter primitive given by xmlName
            • attrName is a core attribute
            • attrName is a presentation attribute
            • attrName is a filter primitive attribute
            • attrName is "class", "style", or "in"
              then continue. (Or throw?)
         2. Let type be the [IDL type for the canvas filter attribute] attrName.
         3. Let attrIDLValue be the result of converting attrValue to type. (This step does things like convert "false" to true for boolean-types, or converting document.documentElement into "[object HTMLHtmlElement]" for string types, just like other places on the platform that accept booleans or strings.)
         4. Let attrXMLValue be the result of converting attrIDLValue to an ECMAScript value, and then converting that result to a DOMString. (This step just gets us back into strings since that's what XML operates on.)
         5. Set xmlFilter's attributes[attrName] to attrXMLValue.

Some notes:

• I went with record<> for the top-level type instead of dictionary because dictionary keys are always lexicographically ordered, which might be confusing. That is, if we used dictionary, then new CanvasFilter({ gaussianBlur: ..., convolveMatrix: ... }) would get normalized by the bindings layer (before your code/spec steps ever saw it) into new CanvasFilter({ convolveMatrix: ..., gaussianBlur: ... }). Using record<> preserves the ordering.

• I went with any instead of the union type we discussed above because I realized that you need to do a conversion that depends on the specific attribute in question. So doing a generic bindings-layer conversion into the union type (boolean or long long or sequence<long long> or DOMString) doesn't buy us anything; someone could still put in a string (which is allowed by the union) for a sequence-of-numers attribute. We need to do the conversion in prose.

[Kaiido]

For each name → attributes of filterDict:

Is there a precedent where the order of the keys is preserved in such a meaningful manner?
As a web author I find this very surprising, and I really wouldn't expect anyone to understand why they have completely different graphic results depending on when in their code they did insert a property to a JS object.

I feel it would be a lot more simpler to not wrap the attributes inside a filterDict, but instead have its name be one of the members of this attribute.

So something like

const f = new CanvasFilter([
   { name: "convolveMatrix", kernelMatrix: [ ... ] },
   { name: "gaussianBlur", stdDeviation: 2 }
 ]);

This way no ambiguity about the order (though if graph is coming, I guess ins and outs attribute's members would be able to change that order).

Note: name was an attribute used on some SVG elements (<font-face-name> and <color-profile> both deprecated in SVG2), I doubt it would ever become one of the official attributes on any future SVG filter, but of course there may be a better name for this.

[mysteryDate]

I feel it would be a lot more simpler to not wrap the attributes inside a filterDict, but instead have its name be one of the members of this attribute.

@Kaiido I like this. Unless @fserb objects I vote we go with that. I also think leveraging the ordering of javascript objects is surprising.

Note: name was an attribute used on some SVG elements ( and both deprecated in SVG2), I doubt it would ever become one of the official attributes on any future SVG filter, but of course there may be a better name for this.

Unless I'm missing something, it looks like SVG itself uses "name": https://drafts.fxtf.org/filter-effects/#elementdef-fecolormatrix

[Kaiido]

Unless I'm missing something, it looks like SVG itself uses "name": https://drafts.fxtf.org/filter-effects/#elementdef-fecolormatrix

Yes, they use the word "name" to describe the filters in the specs, my concern was about the fact that there could theoretically have been an attribute named name on one of these filters.

<feFooBar name="meaningful-value"/>

In this case that would have conflicted with this model, since we could not set both the CanvasFilter's name and the SVGFilter's attribute.

So I checked for such attributes, and the two only I found were on unrelated elements, both deprecated now.
Still I think it's a point to keep in mind. Maybe there is a way to let SVG folks know we have this case that forbids the addition of this attribute on future filters, but I'm not aware of the procedure.

[annevk]

This needs a description of how "XML filter" is mapped to a filter to be used for rendering. I guess it pretty straightforwardly maps to a filter primitive (why is that not its name?), but that needs to be spelled out somewhere.

[mysteryDate]

@annevk

This needs a description of how "XML filter" is mapped to a filter to be used for rendering. I guess it pretty straightforwardly maps to a filter primitive (why is that not its name?), but that needs to be spelled out somewhere.

I added some minimal language to 4.12.5.1.21 Drawing model. Thoughts?

[domenic]

Looks good with only a couple conceptual issues remaining before we can move on to editorial tweaks:

• Make sure each CanvasFilter has a defined XML filter list associated with it.
• Do you want to change to the structure @Kaiido mentioned where instead of records we use { name: "x", attr1: "y", attr2: "z" } structure? In that case I think CanvasFilterInput becomes just record<DOMString, any>, and then the constructor code needs to be updated to special-case name (before the generic attributes loop).

[domenic]

OK, great! The overall model looks good so we're at the point of lots of editorial tweaks :)

[mysteryDate]

@Kaiido

I feel it would be a lot more simpler to not wrap the attributes inside a filterDict, but instead have its name be one of the members of this attribute.

I spoke with @fserb about this and he suggested the key "filter" in order to be even less ambiguous. With this kind of use "name" is a little funny:

const f1 = {
  name: "gaussianBlur", 
  stdDeviation: 2 
 };
const f2 = {
  name: "convolveMatrix", 
  kernelMatrix: [ ... ]
 };
const f = new CanvasFilter([f1, f2]);

As opposed to:

const f1 = {
 filter: "gaussianBlur", 
 stdDeviation: 2 
};
const f2 = {
 filter: "convolveMatrix", 
 kernelMatrix: [ ... ]
};
const f = new CanvasFilter([f1, f2]);

Thoughts?

[Kaiido]

I spoke with @fserb about this and he suggested the key "filter" in order to be even less ambiguous. With this kind of use "name" is a little funny

I personally do like filter a bit more too, there is probably even less risk of collision with an hypothetic future attribute¹, and indeed, name made sense because we were talking about CanvasFilters, but in user code these are just objects, so it looses its meaning.
That being said, this becomes more of an esthetic question than a real practical one as it was before, so if anyone has any objection or an even better proposition, I wouldn't mind either way.

1. Technically, the global filter attribute is applicable even on <filter> elements, but I don't think this would ever have any visible effect.

[annevk]

Reference tests are fine for successful conversions (although you could use getImageData() presumably?). However, to ensure he conversions are implemented correctly we should also test the cases where they end up throwing (as it seems these exceptions will end up rethrowing and abort the constructor steps).

And yeah, merging this PR makes this feature part of the standard and the standard ought to have good test coverage, so we should have those tests. The process is a bit quicker than in other standards organizations, but we still have some prerequisites.

[mysteryDate]

@annevk Here's the PR with the tests crrev.com/c/3320960

[domenic]

LGTM. @annevk?

[annevk]

Thanks, looks good to me as well. Will you file the remaining implementation bugs @mysteryDate? See instructions at https://github.com/whatwg/meta/blob/main/MAINTAINERS.md.

I might push another fixup for some indentation issue I just noticed.

[mysteryDate]

Yes, I'll file the remaining implementation bugs this week. Glad to be moving on! 🙌

[domenic]

Woohoo, thanks for sticking with us through this very long (323 comment!) review. I think the resulting spec is really nice.

[mysteryDate]

Yay! Happy holidays everyone. So glad to get this in.