Skip to content
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

Canvas2D Filters #5621

Open
Tracked by #5613
fserb opened this issue Jun 8, 2020 · 28 comments
Open
Tracked by #5613

Canvas2D Filters #5621

fserb opened this issue Jun 8, 2020 · 28 comments
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: canvas

Comments

@fserb
Copy link
Contributor

fserb commented Jun 8, 2020

A javascript interface for using SVG filters within canvas.

The SVG filter exposes deep, flexible drawing modifiers for 2d graphics. Integrating these into canvas 2d should be technically feasible once an interface is defined.

Working proposal: https://github.com/fserb/canvas2D/blob/master/spec/filters.md

(cc @whatwg/canvas )

@fserb fserb mentioned this issue Jun 8, 2020
9 tasks
@annevk annevk added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: canvas labels Jun 9, 2020
@litherum
Copy link

litherum commented Jun 9, 2020

Conceptually, adding filters to Canvas2D seems reasonable.

Adding full SVG-filter support seems like a large amount of new API for limited benefit. 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. Also, we should start with the "linked-list" model of filters that the css filter() function accepts, rather than arbitrary graphs that are allowed by SVG.

@mysteryDate
Copy link
Contributor

I agree with @litherum. I felt a little odd writing up all the SVG filters into the doc. I have a hard time imagining the practical usage for the lighting filters, for example. Do we have any idea what a "small set of popular filters" would be? GaussianBlur, ConvolveMatrix, ColorMatrix and Blend seem like good initial candidates to me.

@Kaiido
Copy link
Member

Kaiido commented Jun 19, 2020

I'm not sure what would be the real benefit of this...
We can already apply all these filters through url(#svg-filter).

This should theoretically not even require access to the DOM (thinking of OffscreenCanvas), since we can generate such a filter from data:// URI or a blob:// URI (though Chrome has a bug where they don't allow blob URIs yet, IIRC per specs they should and FF does support it).

So the example in the explainer can already be achieved: https://jsfiddle.net/hogyqxw1/ and it seems everything in this proposal becomes neat API wrapping that could be done by a library.


What would be great though is that we finally have a load event for these filters, and that implementers support it in Worker thread too (they don't currently).

@mysteryDate
Copy link
Contributor

I think the idea is precisely to provide a more user-friendly API for a feature that's already implemented but relatively hidden and difficult to use.

@kdashg
Copy link

kdashg commented Jun 23, 2020

There is somewhat perverse value in hard-to-optimize paths being hard to use, so as to keep devs pointed at the APIs we want them to use, and can optimize well.

@mysteryDate
Copy link
Contributor

@jdashg Why are filters hard-to-optimize? Aren't they just shaders?
What APIs would you want devs to use to accomplish this?

@kdashg
Copy link

kdashg commented Jun 26, 2020

Use an API with shaders, like WebGL. Not all canvas2d implementations are implemented with shaders, since not all canvas2d implementations are hardware accelerated.
If you want it to be fast, use WebGL or libraries that do.

@fserb
Copy link
Contributor Author

fserb commented Feb 11, 2021

OffscreenCanvas can't support URL filters properly. Even if we could somehow address this, it's not clear at this point that any web engine can build a SVG on a WebWorker (and not sure if we ever want to do that), which would still leaves us with a weird situation of "we have this capability but no way for users to describe what they want".

I'd also argue that the URL filter to SVG is mostly a hack to allow filters to be described, as opposed to a properly well designed spec that describes Canvas filter capabilities. Having a clear and friendlier interface would be an overall win.

I'm not sure that "because some filters can be slow performing right now, let's make it hard for people to use it" is a good argument. If there was a fundamental performance problem with a feature, sure, make more expensive things harder. But if filters are useful and people start using it more, then eventually it will be worth it for us to optimize for it. Making it convoluted to use just to prevent people from using it at all sounds bad.

@kdashg
Copy link

kdashg commented Feb 11, 2021

We basically can't afford to optimize all fast-paths for all use-cases, so design-wise, we try to shepherd devs towards the always-fast APIs and have them write the fast-paths for their multitude of fast-paths that they care about.

If we want to keep shipping things that have the most impact for the most users, we don't want to design a system that encourages us to clean up after devs that string together friendly interfaces that are hard to optimize.

Put another way, I want to focus on providing the web with features that aren't possible today, rather than signing us up for perpetual maintenance for keeping these other APIs fast through sheer force of engineering.

This is not about making it convoluted, but cutting our losses and not making it easier to fall into the performance pits that we already have coming from svg.

@Kaiido
Copy link
Member

Kaiido commented Feb 12, 2021

it's not clear at this point that any web engine can build a SVG on a WebWorker

It seems indeed none can do it yet, but OffscreenCanvas support is limited to Blink based browsers currently.

(and not sure if we ever want to do that)

There are already at least two places where the specs currently expect this feature:

  • OffscreenCanvasRenderingContext2D and its filter property.
  • createImageBitmap() with a Blob representing an SVG image.

Also one advantage of the current CanvasFilter is its direct parallel with CSS. If you really want to go the Primitives interface way, I guess it might be good to consider how this could fit there too.

@shallawa
Copy link

shallawa commented Nov 1, 2021

I have few thoughts about the two proposals in https://github.com/fserb/canvas2D/blob/master/spec/filters.md:

For the main proposal:

  • It lacks a clear definition for the primitive inputs and result. It does not specify a name for the result. It has only 'SourceGraphic' and 'previous' as builtin names and I expect 'SourceAlpha' is also allowed. But there is no way to reference a result of a primitive multiple times.
  • This proposal is weakly typed since there is not explicit declaration for what inside the input of CanvasFilter(). It does not explicitly specify the properties of each filter primitives and what should happen if the value or the type of an attribute is invalid.

For the alternative proposal:

  • It is a little bit verbose. There are too many lines to define the filter.
  • It also allows two inputs at maximum for any filter primitive. But FEMerge can have more than two inputs.

Regarding both proposals

  • They both lack the definition of the filter and filter primitive geometry. There is no way to define where the filter is going to be applied to the SourceGraphic.
  • It is obvious that the filter result will not be clipped to any bounding box except the bounding box of the canvas itself.
  • It is not clear whether these proposals will allow relative lengths or all lengths have to be in absolute values.

Finally I have these two questions, I think the filter will be applied to every individual drawing. In this example:

ctx.filter = blurFilter;
ctx.drawRect();
ctx.drawCircle();
ctx.filter = null;

the rect and the circle will be blurred separately. What should a web developer do to blur the rect and the circle as a group?

Will it be a better solution to use a stack of nested filters:

ctx.startFilter(blurFilter);
ctx.drawRect();
ctx.drawCircle();
ctx.startFilter(dropShadowFilter);
ctx.drawPath();
ctx.endFilter();
ctx.endFilter();

In this example the rect and the circle will be blurred as a group. The path will be blurred and will have a drop shadow.

@smfr
Copy link

smfr commented Nov 1, 2021

It's notable that the canvas API doesn't allow for graphical grouping for opacity or compositing either, so any solution for filters should also be generalizable to allow for group opacity and group composing.

@Kaiido
Copy link
Member

Kaiido commented Nov 2, 2021

It lacks a clear definition for the primitive inputs and result. It does not specify a name for the result. It has only 'SourceGraphic' and 'previous' as builtin names and I expect 'SourceAlpha' is also allowed. But there is no way to reference a result of a primitive multiple times.

This point derives from the desire expressed in #5621 (comment) to start with only a very limited API, and particularly to "start with the "linked-list" model of filters that the css filter() function accepts, rather than arbitrary graphs that are allowed by SVG". This is addressed in the current PR which you may want to review: https://whatpr.org/html/6763/canvas.html. So currently the only inputs are indeed SourceGraphic and "previous". Note that SourceAlpha can be "hacked-around" by first extracting the alpha channel on a second canvas and then draw that canvas through the filter: https://jsfiddle.net/zcuwmy3L/. I personally agree that the graph model would be very useful, but I fear it's not gonna be for this first version yet.

This proposal is weakly typed since there is not explicit declaration for what inside the input of CanvasFilter(). It does not explicitly specify the properties of each filter primitives and what should happen if the value or the type of an attribute is invalid.

Once again, some work has been made in the PR to clear this up. Basically the current status is that all properties are accepted but only the ones that match with a valid attribute for said filter element would apply, minus core, presentation, filter primitives and "class", "style", "in", and "filter" attributes (which is a pain-point for me).

They both lack the definition of the filter and filter primitive geometry. There is no way to define where the filter is going to be applied to the SourceGraphic.

That's a point I agree on.

It is not clear whether these proposals will allow relative lengths or all lengths have to be in absolute values.

This is a good point, I think currently the conversion model only expects numbers, which would thus forbid the use of relative units, but given the small subset of supported filters and that currently filter primitives and presentation attributes are ignored, I doubt this is really a problem for now, is it?

Will it be a better solution to use a stack of nested filters:

As Simon said, this is not a problem that's unique to filters, the whole canvas API could benefit from such a thing, which we can currently workaround by using a second canvas.

annevk pushed a commit to mysteryDate/html that referenced this issue Dec 3, 2021
Tests: WPT html/canvas/element/manual/filters, html/canvas/element/filters, and html/canvas/offscreen/filters.

Closes whatwg#5621.
@smfr
Copy link

smfr commented Apr 29, 2022

I'm surprised this was merged without more discussion, particularly about how to achieve group effects. I think the API as it stands is a footgun, encouraging authors to write inefficient code (where every drawing effect gets filtered), and getting incorrect results (expectations of grouped effects where none occurs).

Not all implementations can simply map filters to GPU shaders, so filter operations can have significant cost. Filtering on every draw call has potentially high performance cost on those platforms.

@Kaiido
Copy link
Member

Kaiido commented Apr 29, 2022

Could you clarify what in this API makes the users expect "grouped effects" more than the simple CSS filters syntax we have for years?

It seems that you want a layer API, which was discussed in #7329 though it's currently postponed.
But note that 2D canvas users are probably very familiar with the idea that the context settings are applied on each drawing.

@annevk
Copy link
Member

annevk commented Apr 29, 2022

@smfr I'm sorry, it wasn't clear to me your feedback was blocking / not addressed. Could you please open a new issue to track this? Hopefully @mysteryDate can address it.

@othermaciej
Copy link

othermaciej commented Apr 29, 2022

Did the filters feature actually meet the “support of at least two implementers” requirement for adding features? I think maybe it didn’t. The PR lists WebKit and Chromium as the supporting implementers but it seems WebKit did not actually support; the three WebKit folks who spoke up in this issue all had objections that I don’t think were addressed in what was merged (despite an earlier email statement that this feature generally sounds ok).

@annevk
Copy link
Member

annevk commented Apr 29, 2022

That is a good point. Mozilla was not too enthusiastic about this, though we did not block.

I created #7874 to revert, though it seems that is not as straightforward as I would have hoped. Hopefully @mysteryDate or @domenic can help out.

@Kaiido
Copy link
Member

Kaiido commented Apr 29, 2022

Reading back the first comment by @litherum it seems the Webkit team is arguing against ctx.filter as a whole. This already shipped in 2016, Webkit indeed still doesn't support it but both Firefox and Chrome do support the CSS linked-list model since back then.

The proposal here was to extend this feature with a CanvasFilter interface so that SVG filters, which are already available through the CSS syntax ctx.filter = "url(#filter)", become also available in Worker threads, and are easier to use.

@domenic
Copy link
Member

domenic commented Apr 29, 2022

Yeah, if WebKit is against ctx.filter in general, then it's pretty clear that they were not the second interested implementer, so #6763 was merged in error.

I notice that PR cites https://lists.webkit.org/pipermail/webkit-dev/2021-May/031840.html which contains the exchange

  • Better support for SVG filters

Seems reasonable, although this would have performance implications in our current architecture.

Perhaps it wasn't clear to @smfr that "better support for SVG filters" was referring specifically to the proposal in #6763.

We will try to do better as editors about getting explicit support instead of relying on this sort of more-vague statement.

@litherum
Copy link

litherum commented Apr 30, 2022

It's important to try to tease apart "integration of filters into canvas2D" and "a specific formulation of the API that makes it way harder to create group effects than it is to apply individually to each drawing command independently."

We haven't argued against the integration of filters into canvas2D. We do believe, though, that a single individual drawing command is almost never sufficient to display anything interesting; drawing commands naturally need to be grouped together in collections to create images/graphics.

An API that applies filters independently to each individual drawing command is bad for everyone:

  • Authors almost certainly don't want this behavior, as authors want to apply filters to images/graphics, not each individual drawing command.
  • Implementations don't want this behavior either, because the performance cost of filters is significantly higher than the performance cost of a single drawing operation. So, a modal API that causes every drawing operation to get way slower and use way more memory, would be a mistake.

We understand that it is possible to use this API in conjunction with drawing canvases-to-other-canvases to create group effects. Our concerns are about how it's easier to use such an API poorly than it would be to use it correctly. We're referring to the "pit of success" idea here.

There are many potential ways of alleviating these concerns, each with pros and cons:

  • Have authors specify the filter chains as an argument to drawImage() instead
  • Add pushFilter() and popFilter() methods to the context object, where the filters are applied on the group formed between the two calls. Kind of like save() and restore().
  • Represent the filter chain as a stack of OM objects, with push and pop operations on the stack
  • Represent the whole filter chain as a single OM object, and add a commit() method on it
  • Add a one-shot applyFilterChain(filterChain) function to the whole context
  • Filters could piggyback off a potential "Picture" API (that I think is being discussed somewhere) with a drawPictureWithFilter() command
  • etc. etc. etc.

I think this is a ripe area for constructive design and collaboration; let's try to move forward together to make 2D canvas as great as it can be!!

@Kaiido
Copy link
Member

Kaiido commented Apr 30, 2022

So you want two different APIs to apply filters? ctx.filter is already a thing, it is in use on the web and I don't think this can be removed now.

Also, I once again recommend you to have a look at the Canvas Layers proposal since all your concerns are exactly the goal it tried to reach, with the added benefit that it would also work for globalAlpha, compositing, etc.

@fserb
Copy link
Contributor Author

fserb commented May 2, 2022

I agree with @Kaiido, it seems that the problems people are having with this API are orthogonal to the API itself, and were addressed on the Canvas Layer proposal.

Is the argument here that there should be a BeginLayer(filters) instead of ctx.filter?

@fserb fserb added the agenda+ To be discussed at a triage meeting label May 2, 2022
@Kaiido Kaiido reopened this May 2, 2022
@fserb
Copy link
Contributor Author

fserb commented May 3, 2022

I've added the agenda+, and it would be nice if people could come to the meeting so we can discuss this.

From my understanding so far, it seems people are opposed to the ctx.filter = semantic, which was not added by this change. This change just added a way to describe filters, not to set them.

I'm happy to discuss alternative ways of setting filters (with layers, for example). Or any other issue people may have.

@annevk
Copy link
Member

annevk commented Aug 31, 2022

The CanvasFilter feature has now been removed from the specification: c7ad099. (There was some delay due to vacation and change of employers.)

@tomayac

This comment was marked as off-topic.

@domenic

This comment was marked as off-topic.

@tomayac

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: canvas
Development

No branches or pull requests