[css-paint-api] Cycle possible using inputProperties()

stephenmcgruer commented 5 years ago

Consider the following:

index.html

<!-- index.html -->
<!doctype html>
<style>
  textarea {
    background-image: paint(recursivePainter);
  }
</style>
<textarea></textarea>
<script>
  CSS.paintWorklet.addModule('recursive.js');
</script>

recursive.js

class RecursivePainter {
  static get inputProperties() { return ['background-image']; }
  paint(ctx, geom, properties) {
    var myself = properties.get('background-image');
    ctx.drawImage(myself, 0, 0);
  }
}

registerPaint('recursivePainter', RecursivePainter);

This forms a cycle, where RecursivePainter depends on RecursivePainter in order to draw, which is rather impossible. More generally, you could have a chain of PaintWorklets, such that PW1 <-- PW2 <-- PW3 ... <-- PWn <-- PW1. Such a chain is finite (as the number of css properties which accept <image> is finite), but detecting cycles would be painful for the browser - and it's unclear how to resolve them properly.

I think the best the spec can do is declare PaintWorklet-generated <image>s to be invalid in the input properties. This is a little tricky since some properties such as border-image can contain multiple images and it seems reasonable to allow the non-PaintWorklet ones to be used.

FremyCompany commented 5 years ago

I am not sure forbidding recursion is desirable. Recursion could be useful in some cases (like if you want to apply an effect multiple times with different parameters to the same image) but maybe we need to define what happens when such a recursion occurs. Eventually, if you are truly looping, you will get a stack overflow exception thrown, which seems like the right thing to do.

stephenmcgruer commented 5 years ago

if you want to apply an effect multiple times with different parameters to the same image

Where do the different parameters come from? The CSS Paint API is stateless, so as far as I can tell the developer cannot detect where they are in the recursion anyway, if they wished to do something recursively.

Offhand I cannot see any effect that could be created this way; could you give a (high level) suggestion of such an effect?

FremyCompany commented 5 years ago

@stephenmcgruer I guess I was handling a more general situation than your issue. In what I was envisioning, you would use parameters, like paint(workletname, argumentA) and depend on another paint(worklet, argumentB) in a different property.

I guess you are right that a pure loop (depending on the exact paint(worklet, *) which you are currently painting wouldn't be very useful. I just don't know if that is worth specifying, or if that's better to let it throw a stack overflow and move on, rather than risking to over-restrict things. I suspect if someone carefully takes the time to sort it out, it should be possible to write a specific enough spec text to cover that case, I was just afraid we might draw a too-broad trait.

stephenmcgruer commented 5 years ago

It seems like we could spec something that would leave the door open in the future to the upgrade?

I.e. something like: if any of the \<image> values in the styleMap that would be passed to paint() is itself a <paint()>, then log an error to the console and produce an invalid image.

This would (I think) avoid backwards compatibility issues, as developers could not rely on it accidentally today (they would get a clearly broken image), and would leave the door open for us to spec a tractable dependency model in the future.

FremyCompany commented 5 years ago

@stephenmcgruer I still think you're drawing a too broad trait. A paint() should be able to use another paint() as an input, the only think it should not be able to take as an input is itself. If you have blur painter, and a conic gradient painter, you should be able to use the blur painter on top of the conic gradient painter.

Maybe something like that instead?

if any of the values in the styleMap that would be passed to paint() is currently being painted in a worklet, then log an error to the console and put an invalid image in the stylemap in its place.

AmeliaBR commented 5 years ago

I agree that the reasonable reaction to circular dependencies in paint methods is to treat the CSSImageValue as a broken/invalid image. However, just the fact that two properties use the same paint worklet doesn't mean that there is a cycle.

It could just as easily represent a nested function call, equivalent to paint(effect, paint(effect, <image>)).

CSS custom properties (var() functions) are already defined to handle dependencies and cycle detections. The same rules could be used for dependencies between properties created by paint worklets (and layout worklets & so on). When there isn't a cycle, I would expect the worklets to be called in a sequence consistent with the dependencies.

That said, if current implementations can't handle dependencies in a clear and consistent manner, I agree that it might be worth spec'ing an overly-strict behavior with the intention of loosening it later.

flackr commented 5 years ago

I tried to create an example of this to test current behavior and found that it does not work. PaintRenderingContext2D.drawImage accepts CSSImageValue or HTMLImageElement or SVGImageElement or HTMLVideoElement or HTMLCanvasElement or ImageBitmap or OffscreenCanvas, but paint, gradient and crossfade are type CSSImageGeneratorValue. Since CSSImageGeneratorValue is not specified in CSSOM yet, we get a CSSStyleValue in the worklet which supports toString() but can't be drawn.

The drawImage call fails with:

painters.js:15 Uncaught TypeError: Failed to execute 'drawImage' on 'PaintRenderingContext2D': The provided value is not of type '(CSSImageValue or HTMLImageElement or SVGImageElement or HTMLVideoElement or HTMLCanvasElement or ImageBitmap or OffscreenCanvas)'

Since it seems it currently isn't possible I imagine we don't need a resolution until we get to exposing CSSImageGeneratorValue to PaintWorklet.

flackr commented 5 years ago

This also does change the issue to be whether we should allow calling an image generator's generate / paint function. If we do, because it would be an explicit request, it might make sense to synchronously call into the other painter's paint function (similar to CSS layout). The difference being layout only calls into children whereas this can be bidirectional.

css-meeting-bot commented 5 years ago

The Houdini Task Force just discussed Cycle possible using inputProperties(), and agreed to the following:

RESOLVED: We go with AmeliaBR's suggestion, for which input custom properties create edges in the custom property graph

The full IRC log of that discussion

<Rossen_> Topic: Cycle possible using inputProperties()
<Rossen_> github: https://github.com/w3c/css-houdini-drafts/issues/877
<emilio> flackr: we realize that with paint worklet you can specify an image as an input property, which can itself be backed by another worklet
<TabAtkins> q+
<AmeliaBR> q+
<emilio> flackr: so you can in theory, once we implement the TypedOM <image>, you could specify a cycle
<emilio> ... so there are multiple ways we could go about it
<emilio> ... we could try to spawn the paint worklet
<emilio> ... or try to prevent recursion somehow
<Rossen_> ack TabAtkins
<emilio> TabAtkins: the notion of CSS image generator values is not in any spec right now
<emilio> ... we could separate it
<emilio> ... we have cycle detection for custom properties
<emilio> myles: and then what?
<emilio> TabAtkins: you'd get a broken image
<emilio> myles: the whole chain?
<emilio> TabAtkins: yeah, probably
<emilio> heycam: how do you use the image values in the worklet?
<emilio> TabAtkins: you can pass them to drawImage
<Rossen_> ack AmeliaBR
<emilio> AmeliaBR: there are lots of very sensible use cases for reusing the paint worklet result from another paint worklet
<emilio> ... even the same worklet
<emilio> TabAtkins: that's true, so probably we don't want to use cycle detection
<emilio> ... but just depth detection
<emilio> AmeliaBR: cycle detection is fine, but not if you refer to the same worklet
<emilio> TabAtkins: I don't see the difference
<Rossen_> q?
<emilio> AmeliaBR: I talk about the case in which says paint(tiling) and another that uses paint(tiling) that depends on the first output
<emilio> TabAtkins: ok, so proper cyclic detection would work
<emilio> AmeliaBR: when you have a paint reference you treat them as bare references to the input custom properties
<AmeliaBR> s/bare/var()/
<emilio> whoops
<AmeliaBR> s/properties/properties, and then use the regular custom property cycle detection/
<AmeliaBR> s/whoops//
<emilio> myles: is this something that needs to be spec'd?
<emilio> ... HTML depth is not in any spec
<emilio> TabAtkins: CSS defines minimum depth and such though
<Rossen_> q?
<emilio> dbaron: it's also useful just to document so that people can pay attention to it
<TabAtkins> emilio: I agree that going for depth, the particular limit needs to be in a spec.
<TabAtkins> emilio: But the cusotm property cycle detection is in a spec.
<TabAtkins> emilio: So if we eagerly detect cycles, how to detec taht is needed.
<TabAtkins> emilio: And how to treat it.
<emilio> TabAtkins: if we do what AmeliaBR says then it's already in the spec
<emilio> RESOLVED: We go with AmeliaBR's suggestion, for which input custom properties create edges in the custom property graph

w3c / css-houdini-drafts

[css-paint-api] Cycle possible using inputProperties() #877