search

w3ctag / design-reviews

W3C specs and API reviews
Creative Commons Zero v1.0 Universal
328 stars 55 forks source link

Custom Paint API to CR #140

Closed torgo closed 7 years ago

torgo commented 7 years ago

Discussed on the 12 Oct 2016 teleconf https://github.com/w3ctag/meetings/blob/gh-pages/2016/telecons/10-12-agenda.md

torgo commented 7 years ago

https://drafts.css-houdini.org/css-paint-api/

torgo commented 7 years ago

@dbaron @slightlyoff and @travisleithead to review.

slightlyoff commented 7 years ago

Some stream-of-consciousness notes:

Regards

travisleithead commented 7 years ago

Editorial Nits:

dbaron commented 7 years ago

This spec normatively references the WHATWG HTML canvas API (e.g. this). That spec references early drafts from task forces (e.g. this). Worth at least discussing.

I don't think compositing is that early a draft (i.e., I think big parts of it have multiple browser implementations), although there is a more-stable level 1 draft.

property paintWorklet is on the Window interface. Is there a reason it's global? Could there be an alternative home for this?

Maybe on window.CSS ?

I'll try to file comments as issues on css-houdini-drafts, linking back to this one (e.g., like the one above).

travisleithead commented 7 years ago

Thanks @dbaron !

slightlyoff commented 7 years ago

From TAG F2F in Tokyo, we want to re-iterate that we're looking for fuller examples in the Explainer.

We'd like to perhaps revisit on a call to make sure this will get done.

/cc @bfgeek, @rbyers, @shans

bfgeek commented 7 years ago

Thanks for the review!

In the next couple of week I'll add more to the explainer & respond to issues.

bfgeek commented 7 years ago

Still on my queue to address this feedback; I'm planning on having this done before the next Houdini F2F in early Jan.

torgo commented 7 years ago

Taken up at Feb 2017 f2f (day 2). We'll follow up just to check progress on the call on the 22nd.

bfgeek commented 7 years ago

I like the use of ES6 classes for defining worklet code. Is an ES5 syntax also supported? If not, why not?

ES5 syntax is supported, filed: https://github.com/w3c/css-houdini-drafts/issues/361

None of the examples show importing the worklet code. This seems like a major omission.

The examples now import() the worklet code. See: https://drafts.css-houdini.org/css-paint-api/#example-1

The implications for, e.g., drawing shadows outside the main margin box, aren't clear from the prose

This doesn't add any additional complexities on rendering engines, the way to use a paint() function is as a css <image>; this will only paint outside the border box of an element with the border-image-source property and border-image-outset correctly set.

The spec should link to it's companion explainer at the top.

Filed: https://github.com/w3c/css-houdini-drafts/issues/339

This spec normatively references the WHATWG HTML canvas API (e.g. this). That spec references early drafts from task forces (e.g. this). Worth at least discussing.

Acknowledged.

Why does the PaintRenderingContext2D interface not implement CanvasFilters? This deprives custom paint of many interesting effects vs. a Shadow DOM with a background image

We were waiting to see how this matches OffscreenCanvas. One core principle of the paint API is that the invalidation logic is clearly defined. CanvasFilters are able to reference svgs for example which we wouldn't be able to reference (otherwise the invalidation logic because impossible as you have an implicit dependency on something you didn't specify upfront).

One solution here is to accept a subset of CanvasFilters, e.g. blur(2px) which don't reference outside resources.

Another solution modifying CanvasFilters to accept a new css typed om (https://drafts.css-houdini.org/css-typed-om/) CSSFilterValue which we can explicitly invalidate upon as we know its loading state etc.

The Image Placeholder example is seems unsatisfying. How can a developer fade the loaded image in? Or implement any other sort of transition over time?

This is all done with CSS transitions and animations. E.g.

.class {
  background-image: paint(something);
  --some-color-property: red;
  transition: --some-color-property 0.5s ease-out;
}

.class:hover {
  --some-color-property: blue;
}

There is no way to "invalidate" yourself inside a paint class callback, or access timing information. All this type of animation has to use the existing animation APIs.

Related: how does custom paint interact with timelines; e.g. the Web Animation API?

As above, the paint class callback will be invoked each time the computed style changes, so each frame while a Web Animation API is running.

The conic gradient example is non-existent.

Acknowledged we were waiting until we had inputArguments properly defined, it would look something like:

.class {
  background: paint(name, 2px, 4px);
}
registerPaint('name', class {
  static get inputArguments() { return ['<length>', '<length>']; }
  paint(ctx, geom, styleMap, args) {
     console.log(args[0].cssText); // '2px'
     console.log(args[1].cssText); // '4px'
  }
});

We discussed this at the last Houdini F2F and the edits just need to go in.

Why isn't CanvasText supported?

Text is in a "hard problems" bucket at the moment. We need to make sure we explicitly declare out inputs to the paint function (similar to CanvasFilters above). E.g.

paintCtx.font = 'SomeWebFont 2px';

In the above example we don't know ahead of time what font you are going to use, so we can't invalidate the paint function if it loads for example. We were going to introduce a CSSFontFaceValue, and make the font attribute on canvas accept this TypedOM representation.

When the loading state changes, we can then properly invalidate. E.g.

CSS.registerProperty({
  name: '--my-font-face',
  syntax: '<font-face>' // not sure if this is right.
});
.class {
  background-image: paint(name);
  --my-font-face: HappySansFont
}
registerPaint('name', class {
  paint(ctx, geom, styleMap) {
    ctx.fontFace = styleMap.get('--my-font-face');
    ctx.fontSize = 12;
  }
});

property paintWorklet is on the Window interface. Is there a reason it's global? Could there be an alternative home for this?

Resolved in the last Houdini F2F to move this to CSS object.

Not sure the "Note: This is how the class should look" should use WebIDL to describe the expected parameter. I think it would be much clearer to just use a JavaScript example (a class)

This now has a javascript example: https://drafts.css-houdini.org/css-paint-api/#paint-worklet

registerPaint(name, paintCtor):step 2. "NotSupportedError" seems logically wrong. The closest match I found in the WebIDL spec was https://heycam.github.io/webidl/#inuseattributeerror, but that didn't seem quite right either. Is it time for a new "UniqueValueError" or some such (this can't be the first time this case is needed)

Filed: https://github.com/w3c/css-houdini-drafts/issues/362

dbaron commented 7 years ago

I went through the comments above. Here are notes about the ones that are not currently resolved (where I'm counting what seems to me like an adequate response to one of @slightlyoff's or @travisleithead's comments as resolved):

slightlyoff commented 7 years ago

Checking in from the TAG TOK F2F; doesn't look like we can close this yet, pending progress on the issues @dbaron noted.

@bfgeek: should we discuss on an upcoming call?

torgo commented 7 years ago

Waiting on a houdini meeting happening next week. We will circle back on this on 8-8.

torgo commented 7 years ago

Discussed on call 8-8. We need feedback from @bfgeek.

bfgeek commented 7 years ago

w3c/css-houdini-drafts#361 was filed to add an example with ES5 class syntax, but it's so far had negative reaction from @domenic

Fixed.

the issue of paintWorklet being on Window is covered by w3c/css-houdini-drafts#350 (and w3c/css-houdini-drafts#330) but hasn't yet been edited into the spec

Fixed & Fixed.

I wonder if the spec should actually mention border-image-outset in a comment somewhere (as was mentioned in w3ctag#140 (comment)) given that it seems likely to be a common question, although it's not an entirely satisfactory answer given the 9-slice mess you'd have to go through

I just pushed an additional example using border-image to the spec. https://drafts.css-houdini.org/css-paint-api/#example-5

It's not that bad, but would be nice to have a property as simple as background-image to do this. Might be worth exploring a background-image-outset property within css to do this.

I expect we'll also have folks wanting a foreground-image in the future as well.

w3c/css-houdini-drafts#339 is not yet resolved

Fixed.

not sure if more discussion is needed on stability of canvas and its references

Not sure either.

there doesn't seem to be an issue open covering the comments about filters in w3ctag#140 (comment)

Marked https://github.com/w3c/css-houdini-drafts/issues/398 as level 2. As a more meta-comment I think we are lacking a JS representation of filters used in various areas of the platform, as for any sufficiently complex filter, you need to fallback to svg, e.g.

ctx.filter = 'svg(#ref)';

// instead of
ctx.filter = CSS.filterSet([
                         CSS.blur(2, 'px'),
                          CSS.halfTone(...)]);

// note above is a terrible api, but you get the idea.

conic gradient example still isn't edited in

The "arcs" example replaces this now. https://drafts.css-houdini.org/css-paint-api/#example-3

I don't see an open issue representing the future work on CSSFontFaceValue mentioned in w3ctag#140 (comment)

I've marked https://github.com/w3c/css-houdini-drafts/issues/399 as level 2. I think we need the typedOM to settle down a bit, before tackling this.

w3c/css-houdini-drafts#362 is still open

Closed.

w3c/css-houdini-drafts#326 is still open

Closed.

w3c/css-houdini-drafts#327 is still open

Closed.

bfgeek commented 7 years ago

From the F2F we had some minor changes which involved add a devicePixelRatio to the global scope of paintworklet see:

https://github.com/w3c/css-houdini-drafts/pull/446 which is exactly the same as window.devicePixelRatio

Also changed the static alpha = false flag to be a static contextSettings = { alpha: false } to follow the same context creation settings that canvas2d allows in the future. (e.g. color space, etc).

The additional changes from the F2F were some small changes to allow servo to perform speculative evaluation of paint functions. E.g. https://github.com/w3c/css-houdini-drafts/pull/439

torgo commented 7 years ago

Taken up at Nice F2F. On the basis of review of the above, and our understanding of the current statuses of the issued raised, we've decided to close this issue. We thank you for your constructive engagement. This has been a great example of extensibility. Thank you for flying TAG.