Re: [css-paint-api] Spec feedback

On Sun, Aug 23, 2015 at 5:57 AM, Ian Kilpatrick <ikilpatrick@chromium.org>
wrote:

> Hi Benoit, thanks for taking the time to review.
>

Alright so from the feedback I gather this spec it would be impossible to
do immediate rendering if you have tiling. In that case you would use an
intermediate surface and a copy step. Not a big problem given it would
cause less compatibility issues. However not ideal if we want to minimize
the overhead.

I'm still not convinced on the motivation behind the *painting* houdini
spec. The spec says "allows developers to paint a part of an element in
response to geometry / computed style changes". I'm trying to understand if
it's really needed to have the paintCallback execute during the painting
phase of the rendering pipeline. The obvious alternative I see is to
perform the 'painting' using the web content. For example conic-gradient is
polyfilled using CSS SVG image. The biggest limitation the current
conic-gradient polyfill has currently, as I understand it, is that it is
not notified of geometry / computed style changes. I feel that the current
spec is a bit overkill for it's purpose.


> On Fri, Aug 21, 2015 at 5:51 PM, Benoit Girard <bgirard@mozilla.com>
> wrote:
>
> a) The spec says 'paint callbacks should be idempotent'. Should this
> feature allow for, say time based, animated properties? Is Date accessible
> from the PaintGlobalScope? This would interact with implementations such as
> immediate tile based painting where depending on the time API used it could
> drift between the first tile paint and the last. Currently the 'Paint
> Invalidation' section doesn't allow this but it might be a concern for
> forward compatibility.
>
>
> The paint callbacks are called every time the properties they've
> registered as input change. This means that you can use CSS Animations or
> Web Animations to animate custom paint, or just register a custom
> time-value property and modify this from script during rAF.
>
> Date is accessible from PaintGlobalScope, but the idempotency
> recommendation means that authors shouldn't use it.
>

I hadn't considered that CSS Animations could be used with the custom
properties, that would solve all the cases I can think of.


>
> b) What happens if you registerPaint more than once with the given name?
> Does it replace or do the callbacks stack and if so the behavior should be
> specified.
>
>
> Similar to registerApply in the properties and values specification[1],
> this will throw an error. Added Issue.[2] If we allow stacking here we’ll
> suddenly need to provide an API for reordering callbacks which I think is
> unnecessary complexity. If the author needs this type of stacking API this
> would be trivial to implement on top of registerPaint.
>

Conversely it could cause similar problems as onload vs.
addEventListener('load', submitAction). You're right that code that knows
about each other can easily combine their paint callbacks but library code
may conflict. I imagine most web developers will import paint callbacks
from 3rd party libraries. It's conceivable that two libraries may both
supply a polyfill implementation for the same property. addEventListener
allowed event listeners from library not conflict with each other. In this
case we wouldn't want them to stack. If we throw an error however it might
cause the second library to stop initializing if they don't wrap it around
a try catch.


> c) I agree with issue #5, better partial-invalidation (from both side) I
> think would be valuable. With immediate tile based rendering it would
> useful for the user agent to tell large paint callback that it's only
> interested in updating a subrect.
>
>
> I'm now erring on the side that this isn't as useful, primarily from a
> compatibility point of view.
>
>
That's fair, but if we want to allow large and/or complex elements this
will add overhead to smaller paints.


> For UAs that are DisplayList based, would simply require the whole area to
> be re-painted. If an author created two different implementations based on
> if the UA partially invalidated or not, then there could be implementation
> bugs between them which would lead to compat issues.
>

They would only need to provide one implementation that invalidates which
would also covers browser that don't require it.

I don't think invalidation is tied to DisplayList based UAs. It can be
useful for a UA that retains their DisplayList to use spacial invalidation
and updates to their DisplayList instead of reconstructing it all.


>
> If other UAs felt strongly about this, I feel it would be better as an
> optional "advanced" feature that authors would have to explicitly opt-in to.
>
> d) What assumptions can the paint callback make about the canvas and what
> assumptions should it explicitly avoid making? For instance when doing
> immediate tile based rendering, say 256^2 tiles, of a large fragment say
> 1024^2 the user agent may want to fire 4 callbacks for each tile. Can the
> paintCallback make any assumptions about the canvas width/height,
> transformation and clip? This is closely related to partial-invalidation.
>
>
> This sort of highlights why I think partial-invalidation might be a bad
> idea. An author may hard-code their math to assuming 256-tiling, then be
> surprised when a UA changes their tiling scheme or behaviour.
>
> To the question at hand, I would expect that the author is given the whole
> canvas, and additional data that indications the region the author needs to
> write to. If they write to more, the UA is allowed to throw that additional
> data away.
>

I wouldn't expect the author to hard code anything. Even with tiles, the
invalid region may not cover the entire tile. In this case I would just
expect the author to paint blindly and let the canvas clip mask out
unrelated operations, or for more complex operations look at the clip and
self cull.

The important part would be for the author to not assume that their
callback is only called once per frame.


> g) What should the paint callback do when it encounters an error parsing
> the dependent css properties? The spec could say that if a paint callback
> throws a javascript exception with an error message then the user agent can
> report it. I'd imagine it would be useful if the paint callback can easily
> bubble up to the developer tools a parsing error with a particular
> dependent CSS property similar to how dev tools currently highlight errors
> like 'width: SomeText'.
>
>
> This hopefully won’t be an issue if we have a typed OM.
>
>
Typed OM wouldn't cover all the use case so I authors will still use
DOMString to implement unique properties that may still have errors in
them. As well authors may want to reject non-nonsensical values such as one
that would cause a division by zero and that has undefined behavior.

h) Similarly what should the user agent do if there was an exception thrown
> during the paint callback unrelated to parsing. I'd imagine that the user
> agent can simply present the rendering context with whichever operations
> were completed before the error.
>
>
> Added issue. [4] I think that hard failing and presenting no data would be
> the better option here, but would like to see what the wider group thinks.
>

Presenting no data would make it impossible to do immediate rendering
(without a rollback).



> j) PaintCallback is a callback. It looks like it would have access to the
> outer scopes. Web workers specify a script URL where it's clear that they
> don't have access to the outer scope. Should it match web workers?
>
>
> registerPaint is on the PaintGlobalScope. (See the CSS Script API thread I
> wrote [6]). Specifically this would mean that all paint functions run in a
> separate execution context, similar to a worker, and scripts which register
> paint functions are loaded from the main execution context. For example:
>
> // Main execution context
> window.paintExecutionContext.importScripts('my-painter.js');
>
> // Paint execution context
> registerPaint({name: 'paint1', ... });
> registerPaint({name: 'paint2', ... });
>
> This mean that paint callbacks wouldn't have access to the DOM, Storage,
> etc APIs. Blink would not cope well with a execution context per callback
> resulting in large memory usage at the moment. I suspect other JS engines
> would behave similar but would love feedback regarding this.
>
> The Script API explicitly leaves out the number of execution contexts per
> phase, so if an engine would like to parallelize an run multiple contexts
> per phase, they can.
>
>
I'm not sure I fully understand this here. It looks like since
paint1/paint2 were imported in the same script the UA would be required to
have them live in the same PaintGlobalScope. This would mean that the UA
could not paint them in parallel. Or are any importScripts automically
imported for each thread rendering context?

I'm afraid that if we allow global paint state then it opens up a lot of
opportunity for UA compatibility issues. However I've had a chat with the
Mozilla JS team and there does not seem to be any performance way to
prevent this.



> As I mentioned above, for V8 creating a new global scope per callback
> would be high overhead, I suspect it would be for other UAs as well, but
> would love to get feedback regarding this.
>
>
It appears that is the case for us too unfortunately.