-
Notifications
You must be signed in to change notification settings - Fork 143
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.
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
[css-paint-api] Cycle possible using inputProperties() #877
Comments
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. |
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? |
@stephenmcgruer I guess I was handling a more general situation than your issue. In what I was envisioning, you would use parameters, like I guess you are right that a pure loop (depending on the exact |
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. |
@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 Maybe something like that instead?
|
I agree that the reasonable reaction to circular dependencies in paint methods is to treat the It could just as easily represent a nested function call, equivalent to CSS custom properties ( 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. |
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:
Since it seems it currently isn't possible I imagine we don't need a resolution until we get to exposing CSSImageGeneratorValue to PaintWorklet. |
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. |
The Houdini Task Force just discussed
The full IRC log of that discussion<Rossen_> Topic: Cycle possible using inputProperties()<Rossen_> github: https://github.com//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 |
Consider the following:
index.html
recursive.js
This forms a cycle, where
RecursivePainter
depends onRecursivePainter
in order to draw, which is rather impossible. More generally, you could have a chain of PaintWorklets, such thatPW1 <-- 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 inputproperties
. This is a little tricky since some properties such asborder-image
can contain multiple images and it seems reasonable to allow the non-PaintWorklet ones to be used.The text was updated successfully, but these errors were encountered: