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

Async readPixels and process extracted pixels in a worker #9353

Draft
wants to merge 16 commits into
base: v7.x
Choose a base branch
from

Conversation

dev7355608
Copy link
Collaborator

Description of change

  • Async gl.readPixels (WebGL 2 only)
  • Flip and unpremultiply pixels in a worker.
  • Convert to base64 in the worker (if OffscreenCanvas is supported).

This allows extraction with minimal blocking of the main thread. For example, base64 extraction before changes could block for over 200ms (4096x4096); after the changes it may block for just ~6ms. Firefox might not perform as well though: it doesn't seem to do async readPixels right (getBufferSubData blocks for about the same duration as if non-async readPixels was used) if webgl.out-of-process is true (which is the default).

Pre-Merge Checklist
  • Tests and/or benchmarks are included
  • Documentation is changed or added
  • Lint process passed (npm run lint)
  • Tests passed (npm run test)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 6, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c49558f:

Sandbox Source
pixi.js-sandbox Configuration

@dev7355608
Copy link
Collaborator Author

There's isn't really any documentation on what kind of ICanvas extract.canvas() returns. Is that right? I assume it's supposed to be HTMLCanvasElement in the main thread and OffscreenCanvas in a worker. There's also mention in the documentation what kind of context the canvas element has: we could use the bitmaprenderer context instead of 2d to place the pixels into the canvas. Maybe canvas should be deprecated. The user can easily create the canvas element they want from the pixels or the ImageBitmap (if we add extract.bitmap()).

@dev7355608
Copy link
Collaborator Author

This is how it currently works:

extract.pixels(target); // sync
extract.pixels(target, frame, false); // sync
await extract.pixels(target, frame, true); // async

There are other options:

  1. Add pixelsAsync instead and leave pixels be.
  2. Add pixelsSync and make pixels async.
  3. Make pixels async and don't have a sync pixels (maybe not a good idea to remove sync pixels).

@SuperSodaSea
Copy link
Member

Maybe it is time to use Rollup plugins to build the worker code instead of hard-coding them as strings, as mentioned in #8741 (comment). I'll look into it soon.

@dev7355608
Copy link
Collaborator Author

#9363, #9356, and #9315 should be merged before I mark this ready-for-review.

@bigtimebuddy bigtimebuddy added the 🥶 Low Priority Generally issues or PRs that don’t need to make it into the next release. label May 24, 2023
@Zyie Zyie deleted the branch v7.x March 5, 2024 17:16
@Zyie Zyie closed this Mar 5, 2024
@Zyie Zyie reopened this Mar 5, 2024
@Zyie Zyie changed the base branch from dev to v7.x March 5, 2024 18:07
@Zyie Zyie added the v7 label Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🥶 Low Priority Generally issues or PRs that don’t need to make it into the next release. v7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants