-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 #7329
Comments
I am very supportive of introducing a "layers" concept in the canvas API, and I'm glad to see this proposal. Currently the idea is to use the same model as the current Even though the proposal aims at allowing nested layers, the current // draw something on the default layer
ctx.globalCompositeOperation = "source-in";
ctx.beginLayer();
// dozens of lines of code here
// ...
ctx.beginLayer();
// more lines here
// ...
ctx.endLayer();
// wait, which layer is this already? It is thus also very easy to miss one call to And while less about the general model, as currently written, every time we call So I'd like to bring an alternative model into the discussion, actually based on an other proposal in the same repository: Recorded Pictures, which we could rename CanvasLayer. Here is a repository of a The idea would be to have a new So to be exhaustive and in IDL words: interface mixin CanvasState {
(+) undefined renderLayer(CanvasLayer layer);
}
interface CanvasLayer {
constructor();
CanvasLayer clone();
}
CanvasLayer includes CanvasState;
CanvasLayer includes CanvasTransform;
CanvasLayer includes CanvasCompositing;
CanvasLayer includes CanvasImageSmoothing;
CanvasLayer includes CanvasFillStrokeStyles;
CanvasLayer includes CanvasShadowStyles;
CanvasLayer includes CanvasFilters;
CanvasLayer includes CanvasRect;
CanvasLayer includes CanvasDrawPath;
CanvasLayer includes CanvasText;
CanvasLayer includes CanvasDrawImage;
CanvasLayer includes CanvasPathDrawingStyles;
CanvasLayer includes CanvasTextDrawingStyles;
CanvasLayer includes CanvasPath; Now we can have in a module something like const avatarLayer = new CanvasLayer();
avatarLayer.fill(somePath);
// dozens of line of code
export { avatarLayer }; and in an other module import avatarLayer from "./avatar.mjs";
// ... import more layers
const userLayer = new CanvasLayer();
userLayer.globalAlpha = 0.8;
userLayer.renderLayer(avatarLayer);
export { userLayer }; to finally do in the main script import userLayer from "./user.mjs";
// ...
ctx.globalCompositeOperation = "lighter";
ctx.renderLayer(userLayer); Now the code can be easily and clearly segmented, it's easy to understand which layer we're in and what rules will apply on the rendered layer, and it's easy to understand that calling It may be confusing to think about when the drawings actually occur, e.g calling The The main pain point of this model (more in #7329 (comment)), is that it may be too close to the Canvas2D API and one may end up writing code forgetting that drawing new commands to the same CanvasLayer without calling |
@Kaiido thanks for the great comment. Let me try to address the top comments first, and I'll write a second reply about the other proposal.
Do you think it would be easier if the
I think this was a confusion with the explainer. All state is saved/restored (identical to save/restore), but only the state that is applied to the layer is reset inside the layer. This seems to be the more intuitive approach as: ctx.globalAlpha = 0.5;
ctx.fillStyle = 'red';
ctx.beginLayer();
ctx.fillRect(0, 0, 10, 10);
ctx.fillStyle = 'blue';
ctx.fillRect(10, 10, 10, 10);
ctx.endLayer(); produces the same result as (and leave the context at the same state as): ctx.fillStyle = 'red';
ctx.globalAlpha = 0.5;
ctx.save();
ctx.fillRect(0, 0, 10, 10);
ctx.fillStyle = 'blue';
ctx.fillRect(10, 10, 10, 10);
ctx.restore(); This seems to be clear to me, but maybe I'm having some extra assumption that is not obvious. I see your point that one problem with beginLayer/endLayer is that the drawing commands after beginLayer are for the layer, even though they present themselves as for the canvas, and that could potentially lead to confusion. I'd argue that there's also an upside of this, as everything behaves the same as normal drawing (not having to worry about CTM or sizes). But I agree that "when this is going to be rendered" is a potential confusion point. |
Not really, As you note in your post-scriptum, my point is more that it's confusing to call the drawing methods on the context directly. ctx.fillStyle = "#00F";
ctx.fillRect(0, 0, 10, 10);
const px = ctx.getImageData(0, 0, 1, 1).data; one would assume that I think that by acting on two different objects it becomes clearer that we're not getting the layer's pixels. layer.fillStyle = "#00F";
layer.fillRect(0, 0, 10, 10);
const px = ctx.getImageData(0, 0, 1, 1).data;
You are entirely right, somehow I still had my first reading of the first alpha-version of the proposal in mind when writing that. The new wording is a lot clearer in this regard. At least one point that should be clarified then is how will
Also I assume that the current default path would be temporarily "sand-boxed", right? So doing ctx.rect(0, 0, 50, 50);
ctx.beginLayer();
ctx.rect(50, 50, 20, 20);
ctx.stroke();
ctx.endLayer(); would only draw the 20 x 20 rectangle in the layer and ignore the one in the parent layer. This current default path is not part of the CanvasState, so it would need special care. And I'd like to propose to also add
I am not sure to entirely follow here. What would be different between this model and the CanvasLayer one with regard to CTM? CTM is part of the attributes that will get reset, and any transformation that occurs in the layer will get multiplied with the CTM of the parent layer, right? I think CanvasLayers would do the same and I actually find it less confusing that doing layer.setTransform(1, 0, 0, 1, 50, 50);
layer.fillRect(0, 0, 50, 50);
ctx.setTransform(1, 0, 0, 1, -50, -50);
ctx.renderLayer(layer); will result in the rectangle to be drawn from the context's coords 0,0 to 50,50 rather than ctx.setTransform(1, 0, 0, 1, -50, -50);
ctx.beginLayer();
ctx.setTransform(1, 0, 0, 1, 50, 50);
ctx.fillRect(0, 0, 50, 50);
ctx.endLayer(); doing the same. Regarding the size I think I can see the point, you mean like if we prepare a CanvasLayer to draw at the bottom-right corner of the canvas, but then the canvas is shrank and our layer ends up off-screen because |
I do like the RecordedPicture proposal quite a lot (no surprise there, given that I proposed it in the first place ;) ), and I agree that it does address a similar issue to the layer proposal (whatever name we want to call it). I'm not against an alternative proposal, and my initial thinking was very similar to yours (that RecordedPicture was more generic, and Layer was too specific and a bit confusing). Over time I convinced myself that the layer is probably a better solution, and I'll try to go over some of the arguments that moved me there (and that were brought up when I presented RecordedPicture to other folks). But I welcome more conversations over this. (I'm using RecordedPicture nomenclature here instead of your proposed CanvasLayer just to keep it less confusing).
Those are the ones I remember by head, I'd have to dig into some past discussions to see other arguments. Again, I'm not against it. I'm just bringing up some of the potential downsides of this other approach, so we can compare with the slight confusion of layers. Also, it's interesting to notice that the semantics of other 2D APIs match the layer one (CoreGraphics's BeginTransparencyLayer, skia's SaveLayer, etc...). |
Interesting points, thanks for it. I should probably put a reminder that I am not an implementer, and I only had the ergonomics as an user and JS dev in mind when I came to the same idea as yours here. That's also why I took care of not mentioning the potential performance advantages of the RecordedPicture since I really have no idea if this will really have any. Naively, I thought of it only like something that would store a list of method names and arguments that would then get called on the target context. As a very schematic JS implementation: const actions = new WeakMap(); // would store arrays
// keeping RecordedPicture instead of CanvasLayer for clarity
RecordedPicture.prototype.fillRect = function(...args) { actions.get(this).push(["fillRect", args]); }
// repeat for every methods and do something more complicated for setters & getters
CanvasRenderingContext2D.prototype.drawPicture = function(picture) {
const { width, height } = magicallyGetTheSizeOfPicture(picture);
const detachedCanvas = new OffscreenCanvas(width, height);
const detachedCtx = detachedCanvas.getContext("2d");
actions.get(picture).forEach(([prop, args]) => detachedCtx[prop](...args));
this.drawImage(detachedCanvas, 0, 0);
}; So regarding the first point, with this kind of implementation in mind I am not sure what would need to be copied. For me everything would just be reapplied on the context at rendering, and stored as JS values (or seamlessly at least). But it seems you had a very different idea of how this would be implemented, and I am unfortunately unable to get a grasp on it. As such the third point is quite opaque to me, I am really not sure what kind of advantages in terms of performance either solutions would offer and I will take your words for it. For the state of the canvas, yes, it would probably be the same list as the "layer rendering attributes" list, but here there is no save()/reset()/restore() thingy implied. |
Thanks @Kaiido for the comments and suggestions! And thanks @fserb as well for your inputs! Going back to the original proposal, I made a quick change trying to better model the behavior when the layer stays open at the end of the frame. We tried to model the proposal of layers following closely to the current save/restore behavior. This way, it will probably be more consistent to what some web developers are already used, will know how to work with it, and it will also be an incremental change. Regarding RecordedPicture, I also like that idea a lot, and I was not around when it was initially proposed, but I agree with @fserb on the issues regarding that proposal. But I also see how this, even that it solves as well the same problem as beginLayer/endLayer would do, it solves also so many other problems, bringing also the challenges to define it properly in the spec. |
(I did comment on the "auto-endLayer" idea on your commit directly, to avoid making this thread already quite big get even bigger).
I fear that if there is already a layer feature available, there won't be as much traction for a RenderedPicture proposal, the layering abilities it offers seems to be the most noticeable improvement. |
By what fserb mentioned I still don't think that there is much traction for RendererPicture, as it was discarded in the past. I still think it'd be better to keep this issue discussion focused on this current proposal, and probably create a new one for RecordedPicture. It will be hard to discuss two different proposals in a single issue? |
Why not continue to use separate canvases? What are the benefits of this versus the status quo? |
The main two reasons from the browser developers side are:
For web-developers that only need that auxiliary canvas for that specific drawing, it will reduce the name pollution (avoiding having to use aux_canvas, canvas2, and aux_ctx, ctx2...), without having to make it too complex. This beginLayer/endLayer could follow the same logic of save/restore, in regards of the full current state of the canvas, making it esaier to reason about both for browser and for web developers. |
How do we address the portability concerns of browsers choosing sizes? Generally this causes calcification of heuristics as we eventually standardize on the de facto implementation, as alternative browsers respond to web-compat pressures. |
The way I phrased the point one is indeed misleading. Probably that point will make sense as:
The idea of this addition to the spec is precisely allow browsers to do that, as mentioned in point 2. The layer should nevertheless behave in the same way as creating the the smallest canvas possible that would allow to draw its contents to the intended canvas. |
I think we should really continue discussing the API design before going forward with this, and as per the new features guidelines, I believe that here is actually the right place to do so. The recent PR and the current implementation in Canary only cement my initial doubts regarding this design. And to be clear, I don't doubt at all of the use-case and the need for a solution to it. The main grievance I have against the For all such choice we will make here, we will need thorough and extensive docs for the user to be able to know what will happen, since they won't be able to logically deduce it. I'm pretty confident that we can come up with an alternative design where all these questions wouldn't even need answers because the design itself would force a logical behavior. I probably didn't see it all through with my proposed CanvasLayer interface either, but at least from here it seems that at least all these points would get clear answers and that it would get us closer to the stated goal in the PR's note:
Footnotes |
After some deliberation and re-prioritization I think that we will stop pursuing this change on the spec for the moment. It's clear that there are still many things open before arriving a consensus and probably this proposal is not totally well shaped and defined. The current prototype in Chrome has some issues that make the layers close sometimes, so it is actually making it more confusing to argue about it. We may bring back again this idea of Layers in the future, and we can probably get to a better proposal taking as well into account all of Kaiido suggestions, and we can work together on something better and more beneficial for the web :) Thanks for all the reviews and comments! |
In the hope it helps the discussion, I built an user-land prototype of my CanvasLayer interface proposal. Doing so, I must admit that I found a few issues with this proposal that I wasn't envisioning at first: ImplementationThis is a JS implementation, dealing only with what the current API offers and I thus expect it to be far more complex than what native implementations would look like. However, while building it I discovered a few pitfalls: Getters and setters are relatively awkward with this model.Indeed, getters ought to work synchronously, so that if we do layer.font = "1vmin sans-serif";
layer.strokeRect(x, y, layer.measureText(text).width, 2);
layer.fillText(text, x, y); the stroked rect would use the size of the window when the CanvasLayer was built, while the text would use the size of the window when the layer has been rendered on a canvas. Arguably, these relative units are already a mess, so I personally don't think this is a big draw-back. I'm not sure what implementers and spec editors will think of all that, but I believe that from an user perspective having these setters work is a must. Auto-sizing was hard (in user-land).Automagically finding the size of the layer based on the drawing input was really hard in this user-land implementation. I had to call all the saved commands in a first batch (without the painting ones) to be able to determine the bounding box of the current layer. Through comment I assume that at least in Skia there is something that would help here, but I'm not sure if this applicable to all the engines. UsabilityThis is a different model than a 2D context but maybe a bit too similar.While making the demo animation, I shot myself in the foot by not calling Apart from these few points, I still believe we absolutely need a layering API. I still think this CanvasLayer interface model requires less arbitrary decisions and is easier to think about than the |
Playing a bit more with my prototype I faced another case that I wasn't expecting and that neither the I edited my original comment to add this method in the IDL. |
A javascript interface for using layers within canvas.
Provide a beginLayer() and endLayer() functions that create and close layers for Canvas. These methods provide an alternative method for applying a filter, shadow or compositing to a set of drawn operations (as opposed to a single one), other than having to create a temporary canvas and then draw the temporary canvas into the intended canvas.
Working proposal: https://github.com/fserb/canvas2D/blob/master/spec/layers.md
(cc @whatwg/canvas)
The text was updated successfully, but these errors were encountered: