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

CanvasRenderingContext2D: Add getImageDataAsync method #10855

Open
ccameron-chromium opened this issue Dec 12, 2024 · 6 comments
Open

CanvasRenderingContext2D: Add getImageDataAsync method #10855

ccameron-chromium opened this issue Dec 12, 2024 · 6 comments
Labels
addition/proposal New features or enhancements topic: canvas

Comments

@ccameron-chromium
Copy link
Contributor

What is the issue with the HTML Standard?

The existing getImageData method is synchronous.

In practice, canvases are draw asynchronously (often by a GPU, but contention for the Javascript main thread can be avoided by doing CPU drawing on a non-main thread). This means that getImageData must stall the main thread while it reads back data. This has been slightly mitigated by willReadFrequently, but that's not without its own issues.

A better solution would be to create an asynchronous getImageData function. Something to the effect of

Promise<?> getImageDataAsync(
    ImageData imagedata,
    [EnforceRange] long sx,
    [EnforceRange] long sy);

This will get a snapshot of the canvas at the moment that the getImageDataAsync call is made (it will not be affected by subsequent draw calls). The ImageData to populate is passed as a parameter, as opposed to being allocated by the call. This allows less memory thrashing, and it also removes ambiguity about "what should the default readback pixel format or color space be", because the ImageData already answers that.

@ccameron-chromium ccameron-chromium changed the title Add getImageDataAsync method CanvasRenderingContext2d: Add getImageDataAsync method Dec 12, 2024
@ccameron-chromium ccameron-chromium changed the title CanvasRenderingContext2d: Add getImageDataAsync method CanvasRenderingContext2D: Add getImageDataAsync method Dec 12, 2024
@annevk annevk added addition/proposal New features or enhancements topic: canvas labels Dec 12, 2024
@mdrejhon
Copy link

mdrejhon commented Dec 12, 2024

Brillant move. I like the async method, even if it presents a very slight increase in difficulty in workflows.

Performance AND battery efficiency is a simultaneous win-win with this async verison of API, especially with HDR pixel formats (48-bit / 16bpp via Float16)

@Kaiido
Copy link
Member

Kaiido commented Dec 14, 2024

Glad to see this coming back.

One aspect I'm unsure about though is the timing of the API.
It's unclear when an author can expect the image will be available. I guess that most of the time it would be after the next paint, but I'm not sure that'd always be the case. Would it be ensured somehow that it would be ready before the next frame's painting time?
Should two simultaneous calls to getImageDataAsync() always resolve in a deterministic order, whatever the options?
Should any of it even be specced at all? I suppose for most use cases it's ok to be at one frame late, but having variable timings might cause issues for a bunch and might prevent the adoption of this feature.

@mdrejhon
Copy link

mdrejhon commented Dec 14, 2024

I'd naturally expect that such an API always does that. Generally, this is kind of "Get this image once it's unblocked", and unblocking always happens at least once per paint cycle.

It's my understanding that getImageData blocks until the framebuffer is delivered -- so naturally I'd expect a nonblocking getImageDataAsync would report a callback right at the moment of getImageData unblock.

However:

That being said, it would be a good idea to explicitly mention it in the specifications, to assauge fears of a multi-frame latency added.

@annevk
Copy link
Member

annevk commented Dec 18, 2024

Minor nit: I kinda like the getImageDataInto() name I came up with in #5707. For parity with encodeInto() from the Encoding API.

@Kaiido
Copy link
Member

Kaiido commented Dec 20, 2024

Though encodeInto is sync, wouldn't this create confusion somehow?

@mdrejhon
Copy link

mdrejhon commented Dec 20, 2024

That would confusingly mean getImageDataAsyncInto() or getImageDataIntoAsync() but that becomes longwinded. The "get" prefix and "into" suffix is usually designed in APIs independently, whether you're getEncoding() or encodeInto(), rather than getEncodingInto()

Either way, I'm neutral in this case. I think @ccameron-chromium is running with the getImageDataAsync() ball.

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 topic: canvas
Development

No branches or pull requests

4 participants