LibWeb: Add OffscreenCanvas support

[Totto16]

This adds basic support for OffscreenCanvas. It is just a skeleton with many complex methods stubbed out. The more complicated things include:

• how to deal with the skia_context and everything related to that in WebWorkers
• adding (an optional) placeholder `HTMLCanvasElement (outside of the DOM), synchronize the state of that like it is specified in the spec
• support every operation with Window as global object and with Worker, this requires some existing functionality to be adapted to OffscreenCanvas as argument

There is at least one spec bug (https://github.com/whatwg/html/issues/11101) that the spec for OffscreenCanvas has.

Some of these things were already tackled, but not all of them. It works as a basic skeleton, everything else can be done in additional PRs

[github-actions[bot]]

Your pull request has conflicts that need to be resolved before it can be reviewed and merged. Make sure to rebase your branch on top of the latest master.

[Totto16]

I'd like either @Lubrsi or @kalenikaliaksandr to take a look at this as well, but overall this seems like a good start. The commits need a bit of reshuffling, however.

I have my own mental model on how atomic the commits should be, so what is the way you intend it to be. Everything in one is too big imo. The creation of IDL files, test in separate commits, the creation of a separate file for SerializeBitmap in one. But the rest?

[ADKaster]

"how atomic" has a clear definition: the repository with commit as HEAD must build without warnings or errors. Every commit should pass all tests in the test suite. If a test is added later that's okay, but for small commits (which this is clearly not :) ) we generally prefer the test in the same commit as the change.

However you chunk up the changes to achieve that is highly change-dependent.

[Totto16]

"how atomic" has a clear definition: the repository with commit as HEAD must build without warnings or errors. Every commit should pass all tests in the test suite. If a test is added later that's okay, but for small commits (which this is clearly not :) ) we generally prefer the test in the same commit as the change.

However you chunk up the changes to achieve that is highly change-dependent.

So would one commit for all modification in the cpp files be too big in this case? I have no problems with chunking it in commits, that all compile.

[ADKaster]

So would one commit for all modification in the cpp files be too big in this case?

If you did that, the maintainers would start pointing out places where changes are independent, like your move of the buffer serialization code. Giant mono-commits are also harder to review, and might result in changes going unreviewed for a while until you poke #code-review enough.

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

[Totto16]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

👀

[github-actions[bot]]

Your pull request has conflicts that need to be resolved before it can be reviewed and merged. Make sure to rebase your branch on top of the latest master.

[gmta]

Hi! Just popping in here to say that this PR seems to add ~200 new WPT passes in html/canvas/offscreen, but I also see an increase in crashes from 1 to 44. Definitely something to check out.

[Totto16]

Hi! Just popping in here to say that this PR seems to add ~200 new WPT passes in html/canvas/offscreen, but I also see an increase in crashes from 1 to 44. Definitely something to check out.

Yeah, the stubbed out things certainly cause some issues, maybe also crashes, but this PR is already large.

I don't know, if you operate with feature branches, but maybe we could tackle that in that way. I am willing to work more on this. But doing so, without approval for this basic structure PR, is just a waste of time, since resolving later merge conflicts with future work is not that easy.

This PR is already quite large in its size, so I don't know what the best option is 🤷🏼‍♂️ I just know, that we have to start somewhere with the OffscreenCanvas, and if there are some crashes, that don't occur after a long time, I personally think it is okay, to merge it into main 🤷🏼‍♂️

But you (as in the maintainers) have to decide where to go with this, I am happy to help 😄

[Totto16]

CI failures are due to changes in #4506 and needs #4726 so that it can be fixed.

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

[Totto16]

Hi @gmta, I made #4726 to make the changes from #4506 (your PR) available to other files too, and not implemented inside CanvasRenderingContext2D. But I didn't fully test these changes and if they would be enough, but now I did that and found out, that CanvasRenderingContext2D::context_attributes_from_options (see here ) also needs to be shared, I added a commit here, to move it into the LibWeb/HTML/Canvas/CanvasSettings.* files so that it can be used also from OffscreenCanvasRenderingContext2D.

My question would be, should I make this commit (atm the moment of writing ea21480b5bab98e9406951f0b745a15a93cc4348, but might change hash, due to rebases) into a separate PR like #4726 or just keep it in this PR?

And also, I copied your copyright into the new file, as I only moved the content and changed the name, but the content is the same, is that fine for you?