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 layers with filter support #8476

Open
graveljp opened this issue Nov 4, 2022 · 28 comments · May be fixed by #9537
Open

Canvas2D layers with filter support #8476

graveljp opened this issue Nov 4, 2022 · 28 comments · May be fixed by #9537
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: canvas

Comments

@graveljp
Copy link

graveljp commented Nov 4, 2022

This is a proposal for adding layers with filter support in the Canvas2D.

Layer support has previously been proposed in #7329. A new CanvasFilter API has also been presented in #5621. Both proposals failed due to concerns around the suggest API and it's implication on performance.

This new proposal aims at addressing these concerns with an API unifying both layers and filters.
https://github.com/fserb/canvas2D/blob/master/spec/layers.md

The proposal discusses corner cases and interactions with other APIs. A detailed analysis of the different options considered is provided. Depending on the outcome of this discussion, we might have to revisit details of the CanvaFilter proposal, which will have to launch simultaneously with the layer API.

@fserb
Copy link
Contributor

fserb commented Nov 4, 2022

Tagging folks: @whatwg/canvas.

This is the follow up from our conversation earlier in the year about bringing Layers and Filters. Given that Filters was unshipped we still have the opportunity to discuss it further, within this new context of layers.

@smfr
Copy link

smfr commented Nov 8, 2022

WebKit certainly has interesting in a beginLayer()/endLayer() API. We believe the "filter each drawing call independently" behavior of the current canvas filter API was a mistake.

@Kaiido
Copy link
Member

Kaiido commented Nov 8, 2022

Thank you very much for the effort put in this.
A lot of the questions I asked in the last proposal now have answers and I like that they're all backed with alternative designs analyses.
However I must say that most of the choices that have been made turn around "technical" reasons and I'm still not sure these will be obvious for authors. For instance this "layer resampling optimization" that is being used to explain the exclusion of some properties from the "layer rendering attributes" doesn't seem very obvious for authors. Similarly, I still find the draw-in-the-middle-at-readback behavior particularly surprising. Authors probably will have to learn the hard way that calling getImageData() (or doing even less obvious stuff) in the middle of a layer will split their layer in two. I do get the points made there and I'm not arguing against these choices in particular, but I still find that this proposal makes some choices that might surprise a few authors, and I'm not sure how to feel about that.

One thing I'd like to be discussed though is this beginLayer(CanvasFilter) thing. I don't understand why the filter would be declared in this beginLayer() call: Such a design could have make sense if the layer was only affected by this filter, but here it isn't, all the "layer rendering attributes" do affect the layer. Also that doesn't seem like a design that will be easily extendable in the future and that doesn't look like anything we've got in the API currently. I'd like to propose here too a layerFilter attribute, that would only affect the rendering of layers in endLayer() calls (and override the filter one if present). I think this would fulfill WebKit's requirement to not have filters affecting all drawing calls independently, while keeping the overall API more consistent.

@shallawa
Copy link

shallawa commented Nov 8, 2022

I think the layer should be a general addition to the Canvas and not necessarily tied to the filters. So I have these questions:

  1. Layer reuse: should not the layer be reused as the memory canvas?
    In this example canvas2 can be drawn multiple times. But the layer can be used only once.
const canvas = document.createElement('canvas');
const ctx = canvas.getContext('2d');

const canvas2 = document.createElement('canvas');
const ctx2 = canvas.getContext('2d');
// ... draw into ctx2

ctx.drawImage(canvas2, 0, 0);
ctx.drawImage(canvas2, 100, 100);
  1. filter is just style: Why do we treat filter differently from other styles?
    Should not filter be treated as globalAlpha? I think in this example the filter and globalAlpha should be applied only when the layer is composited to the destination context:
const canvas = document.createElement('canvas');
const ctx = canvas.getContext('2d');

ctx.globalAlpha = 0.5; 
ctx.filter = 'blur(4px)';

ctx.beginLayer();
ctx.fillStyle = 'rgba(225, 0, 0, 1)';
ctx.fillRect(50, 50, 75, 50);
ctx.fillStyle = 'rgba(0, 255, 0, 1)';
ctx.fillRect(70, 70, 75, 50);
ctx.endLayer();
  1. Layer dimension: The new proposal suggests the browser

automatically decides what's the best dimension of the temporary image buffer

Does this mean, the layer dimension will be the current clipped rect of the canvas? Or is it the minimum bounding box of the drawings which are called between beginLayer() and endLayer()?

@graveljp
Copy link
Author

graveljp commented Nov 8, 2022

Hi all, thanks for the feedback!

I still find the draw-in-the-middle-at-readback behavior particularly surprising.

I understand your position. This is a hard problem and there are no obvious best solutions. What behavior would you expect as a web developer? I'd also be interested in hearing other implementer's opinions on this.

The next best option I could think of is to hold on unclosed layer content, across frames, until all layers are closed. But this means that nothing would get rendered at all, which can also be surprising. This option is more complicated to implement. Chromium batches draw calls using a paint op buffer. We would need to implement partial flushing, which doesn't exist right now (to draw previous calls up the first beginLayer). It might also remove some optimization opportunities. For instance, direct-mode rendering (skipping the layer's temporary texture) wouldn't be possible anymore since calls like putImageData would have to be able to update the main canvas without rendering any of the layer’s draw ops. Unless we made these cases raise an exception?

I'd like to propose here too a layerFilter attribute, that would only affect the rendering of layers in endLayer() calls (and override the filter one if present)

ctx.layerFilter = ... would indeed fit in the canvas's current API style. We are however designing a new API and this represents an opportunity for revisiting these ideas and perhaps modernizing the canvas API. If we started from a blank slate, I would find:

ctx.beginLayer({filter: my_filter, alpha: 0.5, compositeOp: "source-over");
ctx.endLayer();

nicer than:

ctx.save();
ctx.layerFilter = my_filter;
ctx.alpha = 0.5;
ctx.compositeOp: "source-over"
ctx.beginLayer();
ctx.endLayer();
ctx.restore();

Adding filters to the global context would also add overhead for all the canvas, even though they are only ever used by filters. Take for instance:

ctx.layerFilter = my_filter;
ctx.beginLayer();
ctx.endLayer();

ctx.save();  // All save/restore now has to push and pop layerFilter.
ctx.restore();

Layer reuse: should not the layer be reused as the memory canvas?

This has been discussed in depth in the previous layer proposal (#7329). The key takeaway is that replaying draw commands multiple times is an orthogonal concern to grouping draw calls for filtering purposes. We really should have both: we should be able to create a display list, which contains layers, which apply filters on a group of draw calls. Display list comes with a whole set of different challenges and problems to be resolved. They will also incur runtime overhead not required and desirable for the use cases addressed by layers. Non-display-list layers can unlock high performance code paths not currently accessible via temporary canvas.

The display list idea is being tracked separately and has been suggested in the Recorded Picture proposal.

Should not filter be treated as globalAlpha?

This is exactly how the filter feature was originally suggested, but that option never got full approval. The problem with doing ctx.filter = 'blur(4px)'; is that the filter would apply to every single draw call. Most filters require a temporary surface to work in. Applying filters on individual draw calls would be very expensive, so we do not want to provide an API that makes that very easy to do.

Perhaps a better option that fits along your line of thought would be Kaiido's suggestion in #8476 (comment) to use ctx.layerFilter = ..., to make it clear that this filter only applies to layers, not to every single draw call. I'm happy to discuss this option further, see my thoughts on this above in this reply.

Does this mean, the layer dimension will be the current clipped rect of the canvas? Or is it the minimum bounding box of the drawings which are called between beginLayer() and endLayer()?

That's an implementation detail, left for the browser implementers to figure out. The key point here is that "The browser will produce the drawing equivalent to having a temporary canvas with the minimum size required, given the current transform/clip." In other words, browsers can choose to optimize the temporary texture size as long as users can't tell the difference.

@shallawa
Copy link

shallawa commented Nov 8, 2022

The new proposal https://github.com/fserb/canvas2D/blob/master/spec/layers.md says that

The layer rendering attributes are applied on the layer's resulting texture, these attributes must be resetted to their default values at the beginning of the layer. endLayer() will restore them to the value they had when beginLayer() was called.

So if I have

ctx.globalAlpha = 0.5; 
ctx.filter = 'blur(4px)';

ctx.beginLayer();
// ... draw into ctx.
ctx.endLayer();

I would expect ctx.globalAlpha to be set to 1.0 and ctx.filter to be set to "none" when beginLayer() is called. They will be restored to their original values once endLayer() is called. So these styles will be applied to the result image and they will not be applied to every single drawing API between beginLayer() and endLayer(). I think this is what the user wants.

@graveljp
Copy link
Author

graveljp commented Nov 8, 2022

Yes, that's exactly the plan. Although, the part about ctx.filter only applies to browsers that shipped that feature already. Just to clarify, Chromium and Firefox did implement the ctx.filter global state, but that proposal was rejected by Safari and was never implemented in WebKit (unless I'm mistaken?) The goal of the current proposal is for all browsers to agree on how filters should be specified given that ctx.filter = ... isn't an option.

@graveljp
Copy link
Author

Hi,

I would like to gather feedback on some API design decisions. This proposal is currently suggesting a beginLayer API accepting a CanvasFilter object:

crx.beginLayer(new CanvasFilter(...));
ctx.endLayer();

The CanvasFilter proposal has been approved (see pull request), but it got reverted because of concerns around the ctx.filter = ... syntax. This proposal addresses these concerns with the beginLayer(...) syntax. The CanvasFilter proposal however has been written and approved with the ctx.layer = ... syntax in mind, so we should make sure it still works when used with beginLayer.

Here's an example of the of what the current syntax look like:

ctx.beginLayer(new CanvasFilter([
  {
    filter: "colorMatrix",
    type: "matrix",
    values: [
      0, 1, 0, 0, 0,
      1, 0, 0, 0, 0,
      0, 0, 1, 0, 0,
      0, 0, 0, 1, 0
    ],
  },
  {
    filter: "gaussianBlur",
    stdDeviation: 5,
  }
]));

beginLayer could also accept a CanvasFilter dictionary directly:

ctx.beginLayer({ filter: "gaussianBlur", stdDeviation: 5 });

Any good API should pass the test of time, it should therefore be flexible and expansible. What if we ever wanted to add new parameters [1] to the beginLayer API? We could do so by overloading the function to accept an options object:

ctx.beginLayer(/*options=*/{
  filter: {filter: "gaussianBlur", stdDeviation: 5},
  alpha: 0.5,
  compositeOperation: "xor"});

But now, we have two nested objects with filter keys, making the API ambiguous (i.e. is the argument passed to beginLayer a filter or an options object?) This suggests that we might want to revisit the CanvasLayer proposal. An easy solution would be to rename the keys for something like {filter: {name: ...}}, {filter: {filter_name: ...}} or {filters: {filter: ...}}. Or we could investigate other syntax altogether. This Filter syntax alternatives page shows some possible options.

I'm curious to hear your thoughts. What would you like the layer+filter API to look like? Do we even want to plan for adding more parameters to beginLayer?

[1] I used alpha and compositeOperation as examples of extra parameters here, but we are not even convinced we would ever want these to be layer attributes. It would come as a surprise if the global states didn't apply to layers, and it would be misleading and needlessly complex to have two ways to do the same thing.

@annevk annevk added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: canvas labels Jan 20, 2023
@Kaiido
Copy link
Member

Kaiido commented Jan 21, 2023

Personally I really think it should be one or the other:

  • either we use the "layer rendering attributes" globals to render the layer (and thus add a new layerFilter property to it and don't accept an input to beginLayer()),
  • either we use only the properties set on the options object passed to beginLayer(options) and reset all the attributes of the context (though that's still unclear to me right now what would happen to the CTM and clipping regions in that case).

Anything in between is just too complex and unclear.

But I'd really like to hear implementers's thoughts on this.

graveljp added a commit to graveljp/html that referenced this issue Jul 19, 2023
This adds new beginLayer and endLayer functions to open and close layers
in the canvas. While layers are active, draw calls operate on a separate
texture that gets composited to the parent output bitmap when the layer
is closed. An optional filter can be specified in beginLayer, allowing
effects to be applied to the layer's texture when it's composited its
parent.

Tests:
 https://github.com/web-platform-tests/wpt/tree/master/html/canvas/element/layers
 https://github.com/web-platform-tests/wpt/tree/master/html/canvas/offscreen/layers

Fixes whatwg#8476
@graveljp graveljp linked a pull request Jul 19, 2023 that will close this issue
4 tasks
@graveljp
Copy link
Author

graveljp commented Jul 19, 2023

I have made good progress on this proposal. I wrote the specification for this feature, available in this pull request.

The spec differs from the original proposal in a few points. The proposal has been updated to reflect these. The changes are:

  • Filters specified via ctx.filter = ... are ignored by layers (for browsers that implemented that feature). We want to encourage developers to specify filters via beginLayer(...).

  • In accordance to the recommended JavaScript API design guidelines, the spec proposes to use an options dictionary as argument to beginLayer, such that the API would now look like:

    context.beginLayer({filter: {
      name: 'gaussianBlur',
      stdDeviation: 5}});
    

    To remove ambiguity between the filter definition object and the option dictionary, the filter name is now specified with name: instead of filter: (else, we'd have: {filter: {filter: 'gaussianBlur', ... }}).

    An instance where this options dictionary might be useful in the future to be able to control whether the layer is antialiased or not. It's not currently possible to control this at the context level.

  • The spec proposes that functions reading the canvas (e.g. getImageData) throw an exception if there are unclosed layers.

Everything from the original proposal is implemented in Chromium (behind the --enable-experimental-web-platform-features flag), with WPT tests available at:

I will start updating Chromium to reflect the changes above, unless there are objections.

@annevk
Copy link
Member

annevk commented Jul 21, 2023

@graveljp did you consider specifying a filter as a CSS <filter-function> instead of a dictionary?

@graveljp
Copy link
Author

We could certainly add support for CSS filters, by having the IDL accept either a string or a dictionary. This doesn't have to be part of the MVP however, we could add that support as a follow up. For now, we wanted to see whether we could use this proposal as an opportunity to break free from legacy APIs and do things in a more modern way.

@Kaiido
Copy link
Member

Kaiido commented Jul 21, 2023

So to be clear, the "more modern way" here is the setting of the context properties through a dictionary instead of setting each property one by one, right? In that case I don't think it does much improvement if there is a single property that can be set this way and if you still must set all the others in the legacy way. Once again, I strongly believe all the canvas layer state attributes should be defined inside beginLayer() and none should remain from the parent layer. Having a mix of stuff like that, that we have to remember is just not sustainable.

But if by "more modern" you were talking about the opaque canvas state model already in use by save() restore() then I don't see how this is "modern" nor what improvements it brings. The save() restore() model is one of the most confusing part of the current API for web-devs (with beginPath() to be honest). In my mind the more "modern" additions to the API were Path2D objects and CanvasFilter ones, i.e. objects that could be shared around and reused multiple times.

As for Anne's point about CSS filters, at the time of the CanvasFilter proposal we could wait for a later iteration since we still could use these in ctx.filter. But you can't disregard it here because you're effectively disabling ctx.filter. Having to go through over complicated colorMatrix to do simple invert(n), grayscale(n) or contrast() is a no-go.
I see two ways around that, either, quick and dirty, set the CanvasFilterInput interface accept a DOMString as ctx.filter was doing, or, probably more useful but implying more work, to include it directly in the CanvasFilter model, so that we can chain these with other filters, just like we can do saturate(n) url(#svg-filter) luminosity(n). But this would mean we'd need to revisit the XML Filter List model a bit to incorporate these.

Now, regarding the other points, I still have to make a thorough review of the PR (impressive work btw), but it's interesting that you talk here about the eventuality of disabling antialiasing while imageSmoothingEnabled got disregarded arbitrarily. Should we solve this issue right now? Because this is certainly going to come bite us later.
Throwing everywhere the context is read while a layer is open kind of makes sense, except that it will unfortunately not catch all cases. For instance, there is the obvious presentation to screen where we can't throw, and less obvious consumers like the (ill-defined) canvas.captureStream(). And once again we end up with an hybrid situation where sometimes it will do the sensible thing and other times it will do a complicated save the current state — draw an half baked frame — restore the intermediate state — hope we'll get closed someday and we'll just have to remember which action triggers what.
Also, the point around reset() isn't clear. The explainer said this should throw when a layer is open, but the PR doesn't say so, actually reset doesn't seem to work with layers in the PR, it never resets the original bitmap. But throwing here would not work, setting the canvas width or height attributes will also reset the rendering context to its default state and setAttribute() doesn't really throw.

@graveljp
Copy link
Author

Thanks for the feedback! There's a lot to unpack in here. Let's break it down.

Add alpha and compositeOperation arguments in beginLayer()?

We could certainly have beginLayer accept and alpha and compositeOperation parameter, but it's not clear to me that it would be less confusing.

What would we do with globalAlpha and globalCompositeOperation inside the layer? Should they apply to draw calls in the layer, or should they be reset to their default value? Keeping global states inside layers, we would have:

ctx.globalAlpha = 0.3;
ctx.fillRect(...);  // Uses 0.3
ctx.beginLayer({alpha: 0.5});  // Uses 0.5
ctx.fillRect(...);  // Uses 0.3, then 0.5 from the layer's output compositing
ctx.globalAlpha = 0.7;
ctx.fillRect(...);  // Uses 0.7, then 0.5 from the layer's output compositing
ctx.endLayer();  // Restores the original 0.3 value

ctx.beginLayer();  // Uses 1.0
ctx.fillRect(...);  // Uses 0.3, then 1.0 from the layer's output compositing
ctx.endLayer();

This would come as a surprise in it's own right, if developers try to modularize their code like so:

function drawFancyButton() {
  ctx.fillRect(...);  // Will use 0.5
  ctx.beginLayer({filter: {name: 'dropShadow'}});  // Will use 1.0
  ctx.fillRect(...);  // Will use 0.5 * 1.0
  ctx.globalAlpha = 0.8;
  ctx.fillRect(...);  // Will use 0.8 * 1.0 instead of 0.8 * 0.5
  ctx.endLayer();
}

// Attempting to blend the button as a whole:
ctx.globalAlpha = 0.5;
drawFancyButton();

Should beginLayer accept CSS filter strings?

Maybe "modern" wasn't the right word. Dictionaries have a number of advantages over plain strings. They are easier to assemble if the filter attributes comes from variable (CSS filters would require string concatenations). Dictionary fields are also named, as opposed to CSS strings whose arguments are ordered. This is nicer for developers because it allows you to specify only the field you care, leaving others to their default values. It also helps maintain forward and backward compatibility if we need to change a filter's argument list. If we add a bunch of the attributes in one spec change, implementers can then implement them in the order they want.

That being said, we could allow both dictionaries and CSS strings, if that's preferred. Updating CanvasFilterInput to accept a DOMString would be easy to implement, at least in Chromium. Mixing strings and dictionaries would add complexities, but might be possible. I'll have to check how that could be implemented.

In my mind the more "modern" additions to the API were Path2D objects and CanvasFilter ones, i.e. objects that could be shared around and reused multiple times.

We could still add support for CanvasFilter later. As you pointed out, CanvasFilter can be shared between contexts, which would add an extra level of complexity to this already complex proposal. Filters like "dropShadow" should technically support CurrentColor, color-scheme-dependent system colors and forced color modes, which means that they cannot be fully resolved until they are assigned to a context. Then, you'd want to cache the resolved filter in case they are re-used, but if the CurrentColor changes, the filters must be invalidated, for the affected context.

imageSmoothingEnabled vs. antialiasing

imageSmoothingEnabled and antialiasing are different things. The former applies exclusively to transforms whereas the latter applies to shapes and paths being drawn. As detailed in the explainer, we do not want transforms to resample each individual nested layer outputs as this would dramatically reduce image quality (and lower performance). We instead want to draw the shapes directly to their final position in the HTML canvas element buffer. imageSmoothingEnabled therefore can't be applied to each individual layers. See the code snippet and images here for an example of how detrimental applying imageSmoothingEnabled to layer output would be.

Unclosed layer handling

The story around unclosed layer is complex and there are no perfect solutions. I would love to hear the opinion of other browser implementers on the idea discussed in the explainer and the analysis of alternative solutions.

For the point around reset(), the example in the explainer is ctx.beginLayer(); ctx.reset(); ctx.endLayer();. The throw happens in ctx.endLayer() because it doesn't have a corresponding beginLayer(). The reset() doesn't throw. I'll update the explainer to clarify this.

@Kaiido
Copy link
Member

Kaiido commented Jul 25, 2023

Add alpha and compositeOperation arguments in beginLayer()?

What I had in mind about passing all the layer rendering states in the BeginLayerOption would indeed be to ignore the context's global settings. Basically you would reset all the context's settings inside the layer, use only the ones passed in the BeginLayerOption to render the layer, and finally restore() all the global settings as they were when beginLayer() got called.
The current proposal already does this, but only for the current filter, plus different rules for the properties that fall in the layer rendering states, and yet another rule for the remaining properties (like fillStyle etc.).
That may indeed sound a bit surprising at first that the global settings are ignored, but I believe it's because we have the drawImage(layer) model in mind. After all we already have putImageData() that does also ignore all the global settings of the context, so it's not like an entirely new thing. And moreover, it's something you have to learn once and it's all cleared up, instead of always having to remember Is filter ignored? Yes. Is gCO ignored? No. Is CTM ignored? Yes. Is shadow ignored? No. Is clip() ignored? That's complicated.
And resetting all the context's settings to their default may even simplify things for module scripting as you don't have to reset these manually at the beginning of your layer. Inside a layer, you would start from a fresh context, that would get put like an ImageData ignoring all the global context settings but the ones you explicitly passed to beginLayer(options).

So for your example if you wanted all of the fancy button to have its alpha set to 0.5 you'd do

ctx.beginLayer({alpha: 0.5}):
drawFancyButton();
ctx.endLayer();

and all the button component would be rendered with the desired alpha. It would be sure that its first fillRect() is "black" and not whatever the outside script left the context with, if it draws some text it won't have to manually set all the context fontXXX properties, etc.
Or you could even have drawFancyButton() accept a BeginLayerOption param and wrap all its code in a layer itself.
By the way, note that in your example, each drawing inside the button component would have its alpha set separately, which leads to different results than if the alpha is set on the whole. For instance if you had a drawImage() call after the endLayer() it would blend with the rest of the button, which is probably unexpected.


Should beginLayer accept CSS filter strings?

I wasn't arguing against the use of dictionaries and I don't think nobody was. The thing is that currently, we can't use the simple, and well known CSS <filter-function> in the layers. This looks like a regression. To get the same results we need to rewrite the CSS filters through SVG filters and that's overly complicated.
So to be clear, the idea would be to make CanvasFilterPrimitive potentially a CSS <filter-function>, so we can write something like

ctx.beginLayer({
  filter: [
    "brightness(1.5)",
    { name: "colorMatrix", type: "luminanceToAlpha" },
    "blur(5px)",
    "invert(1)",
  ],
  composite: "destination-in"
});

instead of having super complex colorMatrix tables.


My point about CanvasFilter and Path2D was more about the beginLayer() endLayer() model, I'd have preferred a CanvasLayer interface. The CanvasFilterInput interface doesn't change much from CanvasFilter from an user's perspective. (And regarding the currentColor case, we already handle the same issue with CanvasGradient#addColorStop()).


imageSmoothingEnabled vs. antialiasing

I see your point, thanks. I think I was hoping for the canvas layer API to be able to replace your use of the offscreen raster entirely in that example, as this is something that comes up very often.


Unclosed layer handling

A thanks for the update, indeed I misread the explainer. I believe there is still an issue with this method in the current PR, but we can handle it there.

@annevk
Copy link
Member

annevk commented Jul 26, 2023

We could certainly add support for CSS filters, by having the IDL accept either a string or a dictionary. This doesn't have to be part of the MVP however, we could add that support as a follow up.

At WebKit our thinking has been the other way around. We'd like the MVP to center around CSS (as most of canvas already does) and potentially add dictionary syntax for filters later.

@graveljp
Copy link
Author

graveljp commented Jul 26, 2023

@annevk, are you suggesting to remove the dictionary syntax from this proposal and create a separate proposal for it? Or can I update the current proposal to accept either a CSS string or a dictionary? The CanvasFilter proposal originally had traction and even got submitted, but it got reverted only because using the global ctx.filter was not acceptable. This new proposal is a follow-up to the CanvasFilter proposal, fixing it by adding the layer API. It would be nice if we could move forward with that proposal.

@Kaiido, looking back at the list of rendering states, we could interpret them all as applying to layers' output:

  • The filter passed as argument to beginLayer unambiguously tells how the layer itself look like, not it's content.
  • The filter specified in the context really should be ignored from this discussion: it was never implemented by Safari, it might as well never have existed.
  • globalCompositeOperation and globalAlpha are not properties of any draw calls or layers. They are properties telling how two images are composited together. They are not a property of either images, but a property that arises from the need to combine them. From that point of view, it would not be technically correct to say that a layer "has" a compositeOperation and it would be perfectly reasonable to say that as a layer gets composited with the canvas, it should do so according to how the context combines any pair of images, that is, using globalCompositeOperation and globalAlpha.
  • The imageSmoothingEnabled is in fact specific to drawImage. It's called imageSmoothingEnabled, not smoothingEnabled, or layerSmoothingEnabled. From that point of view, it should apply to drawImage calls inside layers, not to the layers themselves.
  • For CTM, the problem is perfectly symmetrical. You can equivalently say that individual draw calls are transformed inside the layer, or that they are not and that the layer itself is transformed. The result will be exactly the same. We choose to rotate individual draw calls for technical reasons only, but from the user's perspective, they cannot see the difference.
  • The clipping has to be applied to the layer's output (I'll have to fix the PR to reflect this). If the clip was applied only inside, a filter like "blur" would create pixels outside of the clip region. If the clipping was applied both inside and outside, the clip boundary would get double-anti-aliased. The only good solution is to apply it outside.
  • For global shadows, we get to choose where they apply. The current proposal choses to have them apply to layer's output, to be in line with the other rendering states.

Therefore, there is no confusion: all states apply to layers (if you exclude those specific to text, lines, images, etc.)

As to whether we should reset the whole context to it's default state inside layers, that might be surprising and inefficient. Whether we like it or not, the canvas rendering context is stateful. beginLayer() has nothing to do with text or fill style, so why should these state change? beginLayer() is defined as saving and restoring states, just like save() and restore() do. save() however doesn't reset all states. Resetting all states, and restoring them all would come at a cost too, as changing rendering states isn't free.

@Kaiido
Copy link
Member

Kaiido commented Jul 27, 2023

The filter passed as argument to beginLayer unambiguously tells how the layer itself look like, not it's content.

Yes and that's great, hence why I propose we do the same with all the layer rendering states, not just this one. Having some picked from the context's state and one (for now) as a clear argument is confusing.

The filter specified in the context really should be ignored from this discussion: it was never implemented by Safari, it might as well never have existed.

But it does exist, it is being used by authors even though WebKit never supported it and can't be removed from the web now. We have to deal with it at least for backward compatibility. I'm ok with ignoring it in layers though. But authors will have to remember it.

globalCompositeOperation and globalAlpha are not properties of any draw calls or layers

Sorry but here I don't follow. globalAlpha acts just the same as filters, and theoretically you should be able to replace globalCompositeOperation with SVG filters (if in="BackgroundImage" were supported by browsers), so I don't see why you want to oppose them like that. I never argued that they shouldn't be part of the layer rendering states, I'm only arguing that these states should have the same clear and "unambiguous" place as filter does, i.e. to pass them as an argument to beginLayer(), which makes perfect sense since they will define how the layer will be composited with the parent bitmap.

[...] The current proposal choses [...]
Therefore, there is no confusion: all states apply to layers (if you exclude those specific to text, lines, images, etc.)

:-). You spent maybe a year of hard thinking about these rules to come up with explanations of why they should or should not be included. Every choice you made had multiple possible alternatives that you did document. I'm pretty confident that your choices are the right ones, but you can't say that "there is no confusion". A web-author will not have spent as much time as you did thinking about these issues. Asking them to do that same reflection process in order to use that feature doesn't seem like great UX.

Having three levels of rules about which properties are reset or not in a layer is confusing.

As to whether we should reset the whole context to it's default state inside layers, that might be surprising and inefficient.

If one uses beginLayer() to replace their current detached canvas as was the original use case that this proposal tried to fulfill, then not having a clean state will be the most confusing. Just look at your own drawFancyButton() example, the most expected use of layers is indeed for modular components and for these having a clear canvas state is certainly the best. Just like in JS modules the scoped variables aren't polluting the inner modules scopes. Unfortunately there is no way to "clean" the canvas state. reset() will close the layer (and clear the bitmap).

As for inefficiency I can't tell, but if resetting the context state to their default values is that bad, then it's probably a bad thing that the current PR already asks to do so (step 10).

beginLayer() is defined as saving and restoring states

No. beginLayer() is not yet defined. This is what we are doing right now.

Maybe a compromise could be found through a field added to BeginLayerOption that would control whether the context's state should be reset or not inside the layer, but I feel that it would make everything much more easier if it did reset everything.

@graveljp
Copy link
Author

@annevk, the agreement from our March meeting was indeed to start with CSS filter support, sorry I didn't connect the dots before. But wasn't WebKit also interested in supporting color matrix filters? Can we make this part of this first layer spec version?

@Kaiido,

As for inefficiency I can't tell, but if resetting the context state to their default values is that bad, then it's probably a bad thing that the current PR already asks to do so (step 10).

Maybe this step 10 could be phrased differently. The my intent was to precisely describe which states should be observed in step 11, which invokes the "steps outlined in the drawing model." In practice, implementations don't really have to reset all states. Writing specs is hard! ;)

@tuankiet65
Copy link

Besides the concern that @annevk has voiced (that we should support specifying filters as CSS <filter-function>), I also have a concern about the way unclosed layers should be handled. I believe it's better to leave them open instead of force closing and reopening them, because this is the most intuitive behavior to the web developers. If layers were to be implicitly closed, this might encourage developers to write ill-formed code that does not explicitly close layers, because they'd assume the layer would be closed anyway. I assume there is a technical reason why this is undesirable? FYI, I'm implementing this API in WebKit as an experiment, and we're able to display the content of a canvas with unclosed layers.

@graveljp
Copy link
Author

graveljp commented Aug 4, 2023

Happy to hear that you started developing this features for WebKit!

That's a good point. I agree that auto-closing the layer could trick web developers to leave layers open and they would never notice. This could lead to a proliferation of bugs similar to crbug.com/1299201.

Just to confirm, when you say "leave them open", do you mean that frames will be presented without the layer content, until the layer is closed, after which the whole layer is finally presented as a whole? This is option 4 (hold on to incomplete layers, render them once completed), described here, is that correct?

I'll have to see if we can implement this in an efficient way in Chromium. We would need to do a partial flush of the canvas (flushing up to the first beginLayer) and copy the remaining paint ops to the next frame's buffer. This would add a cost for sure, but if that cost only happens for unclosed layers and we can print a console message, it would be easily fixed by web developers.

Another alternatives we considered was to just drop unclosed layers entirely and not draw any of its content. This could solve the issue in a similar way, without the need to carry over draw calls to future frames. I'm curious to hear your thoughts on this one.

@tuankiet65
Copy link

This is option 4 (hold on to incomplete layers, render them once completed), described here, is that correct?

Yeah that is correct. At least on our side (which uses Core Graphics drawing contexts), we don't have to do any sort of special handling (like partial flush as you said), we just issue draw commands to CG like normally.

Another alternatives we considered was to just drop unclosed layers entirely and not draw any of its content

I'm not aware of how to implement this in Core Graphics. CG provides two layer-related APIs which we use: one to start a transparency layer, one to end a transparency layer and composite it back to the previous layer. I don't think we have such an API for "throw away the current topmost layer and do not composite it back".

graveljp added a commit to graveljp/html that referenced this issue Nov 7, 2023
This adds new beginLayer and endLayer functions to open and close layers
in the canvas. While layers are active, draw calls operate on a separate
texture that gets composited to the parent output bitmap when the layer
is closed. An optional filter can be specified in beginLayer, allowing
effects to be applied to the layer's texture when it's composited its
parent.

Tests:
 https://github.com/web-platform-tests/wpt/tree/master/html/canvas/element/layers
 https://github.com/web-platform-tests/wpt/tree/master/html/canvas/offscreen/layers

Fixes whatwg#8476
@graveljp
Copy link
Author

graveljp commented Nov 9, 2023

Hi all,

I have finalized the Canvas 2D layer specification (#9537). I have added support for CSS filter strings and updated the way unclosed layers are handled according what was agreed in #8476 (comment). @domenic reviewed the spec and I addressed all editorial feedback.

We would like to move forward with the approval process. This API could be included in Interop 2024 if it's approved in time.

I think that the only remaining open question is around a concerned raised by @Kaiido about whether global rendering states (globalAlpha, globalCompositeOperation, shadowColor, etc.) should be observed by layers. He is arguing that we should be consistent with filters. If layers only observe filters specified via beginLayer(), it should be the same for all rendering states (@Kaiido, correct me if I'm wrong).

What is the opinion of other browser implementers? Our opinion is that if layers didn't observe global rendering states, it would likely cause more confusion than the alternative (e.g. developer will complain that their layer isn't blended, assuming that this certainly must be a bug). In addition, globalAlpha/CompositeOperation are are not conceptually states of the layer itself, they are states that describe how the layer is composited with it's parent. This seems better suited to being a state of the parent than a state of the layer.

@annevk & @tuankiet65, could you review/approve the spec on behalf of WebKit?

@kdashg, does this spec looks good from Gecko's perspective?

@tuankiet65
Copy link

I'll have to delegate to someone else since I'm no longer involved with WebKit.

@Kaiido
Copy link
Member

Kaiido commented Nov 26, 2023

I think that the only remaining open question is around a concerned raised by @Kaiido [...]

Well, to summarize the points I developed in the previous comments I'd say there are 2 very different ways to see this new feature.

  • One is to see this like a grouping of drawing calls that will end up being drawn like drawImage() does, i.e. it will be affected by the context's global drawing settings when rendered on the parent layer. But indeed we have this filter thing. It has 2 implementations and is already overly used on the web, so we probably can't remove that from the standards now. But, it will obviously be in conflict with the one we're defining here, so it has to excluded by the layer API. But the only real reason it has to be excluded is because that's how the API got built over time. We're basically adding one more "hysterical raisin" in the ecosystem and I personally believe this is not good. And then even some settings that would affect drawImage's rendering are excluded of the list for other (better) reasons. And for an user it's almost impossible to guess what this list is made of, and thus it will be harder to remember it.
    Then there is the issue related to which properties are reset and which aren't. The rendering properties are but not the rest, but they're going to be reset to their current state in endLayer(), leading to IMO very confusing situations.
ctx.fillStyle = ctx.shadowColor = "red";
ctx.fillRect(30, 30, 40, 40);
ctx.beginLayer();
ctx.fillRect(10, 10, 50, 50); // This one is red, because fillStyle isn't in the list of stuff that get reset in the layer
ctx.shadowBlur = 3;
ctx.strokeStyle = "green";
ctx.strokeRect(0, 0, 20, 20); // green stroke, but no shadow, because shadowColor is in the list and has been reset to transparent
ctx.endLayer();
ctx.strokeRect(70, 70, 20, 20); // black, because even though it's not reset in the layer, it is reset to what it was when we called beginLayer()
  • The other way of seeing this feature is as a way to have a new separate drawing context drawn on top of the current one. Like we currently do when using multiple canvases, but in a more performant way. From that point of view, the context's global settings don't matter when rendering the layer. Only the ones passed to beginLayer(settings) would be used to render the layer. All the context settings would be reset to their default inside the layer. This allows to write clean and modular code without having to care about the state the caller left the canvas in. Because this doesn't map to drawImage at all, but rather to putImageData in that it isn't affected by any global settings either, there is no confusion as to why a context whose globalAlpha is set to 0.5 would have a layer rendered opaque anyway if they didn't pass globalAlpha in the beginLayer() call. And as a bonus, if handles the filter issue nicely since now only the settings passed to beginLayer(settings) matter.

As for the argument that compositing states are more states of the parent I do agree, but it seems that it's an argument that would hold for filters too, no? If this is that much of an issue, I'd be ok with having these settings passed in the closing call (current endLayer()), which could then be renamed to something like renderLayer(settings) or similar.

@graveljp
Copy link
Author

graveljp commented Nov 28, 2023

Hi Kaiido,

Thanks for expanding on your ideas.

Just a little note regarding your example for the first point. On the line that says:

ctx.strokeRect(0, 0, 20, 20); // green stroke, but no shadow, because shadowColor is in the list and has been reset to transparent

the rect will be shadowed when the layer is closed. [EDIT: in the particular case, the shadow won't be applied. See Kaiido's reply.] I'm sure you understood that, but I just wanted to make it very clear to everyone that the shadow isn't just lost for draw calls inside the layer. It's just that if the shadow gets applied to the layer output, it cannot also be applied to each individual draw calls inside the layer.

Regarding the second point, I think there are two independent questions to consider:

  • Should the global rendering states apply to the layer output.
  • Should the global rendering states be reset to their default values when entering a layer.

For the analogy you mentioned of using a secondary canvas and drawing it to the main one, the global states would be defaulted inside the secondary canvas, but the main canvas' global states would be used when drawing the secondary canvas to the main one. Your proposal appears to suggest otherwise by saying that global states shouldn't be applied to the layer's output. The analogy seems to support the idea that it would come as a surprise if the global states didn't apply to the layer's output.

As for whether the global states should be reset inside the layer, I was saying this in #8476 (comment):

As to whether we should reset the whole context to it's default state inside layers, that might be surprising and inefficient. Whether we like it or not, the canvas rendering context is stateful. beginLayer() has nothing to do with text or fill style, so why should these state change? beginLayer() is defined as saving and restoring states, just like save() and restore() do. save() however doesn't reset all states. Resetting all states, and restoring them all would come at a cost too, as changing rendering states isn't free.

@Kaiido
Copy link
Member

Kaiido commented Nov 28, 2023

the rect will be shadowed when the layer is closed.

This example was to highlight how confusing it is when we do not reset the state inside the layer, and I think it demonstrates this pretty well because, no,there won't be any shadow at all. To have a shadow drawn from a default context state we need to set both shadowColor to something else than transparent, and to have either shadowBlur, shadowOffsetX, or shadowOffsetY set to something else than 0. So in this case the top level layer has only the shadowColor set, no shadow will be drawn. Then in the layer we set the shadowBlur, here, if the shadowColor wasn't reset, we would have a shadow, but because shadowColor is reset to transparent black, nothing actually is. Obviously, the confusing part is that fillStyle wasn't reset.
If everything is reset there would be no such confusion.

As for my point about the second canvas, I too would have preferred a drawImage like behavior where the context's global state would apply on the rendering of the layer, but we have this filter case... And because of it, I believe we're better stepping off that behavior entirely and follow the putImageData behavior instead. Since this is a new API, I personally believe this is a possible approach.

But I'd really like someone else to step in the discussion with us and hear others thoughts on all this, both from implementers and from potential users.

@graveljp
Copy link
Author

There is no fundamental reasons why filter shouldn't apply to layers' output. If a global filter is specified and the layer has a filter specified via beginLayer, we could just chain the two filters. If the fact that layers don't observe filter is to derail the whole design, we could certainly revisit this decision. I remain to be convinced that there is a real issue here.

The reason why this initial spec proposal excludes filter is that we have to be very very careful with any feature we release. We could always add more features later, but once we make something available on the open web, we can never take it back. Now, filter remains unsupported by Safari and so, web developers would be more than happy to switch to beginLayer when it's launched. I suppose we could promote that migration by printing a console message if filter is used. In that sense, filter would become a deprecated feature that received no future feature improvements, including layer support. All non-deprecated rendering states would affect layers.

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

Successfully merging a pull request may close this issue.

7 participants