Improve screenshot pixel flipping with contiguous row copies - #5852
Open
diarmidmackenzie wants to merge 1 commit into
Open
Improve screenshot pixel flipping with contiguous row copies#5852diarmidmackenzie wants to merge 1 commit into
diarmidmackenzie wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(PR assisted by Codex/Astra)
Description:
Improve screenshot performance by copying contiguous pixel rows instead of traversing the image column by column. This avoids the cache-unfriendly memory access pattern that makes large screenshots particularly slow, while preserving the synchronous
getCanvas()API.Changes proposed:
subarray()andset()to copy rows in reverse order.CPU performance
Local benchmarks comparing the implementation on
masterwith this change:Measured on an Intel i5-1035G1 using Node.js v22.22.2. Results are medians of nine batches after warmup, alternating implementation order, with multiple calls per batch at smaller resolutions. Timings include output allocation but exclude rendering, readback, canvas writing, and PNG encoding.
The absolute saving is small at low resolutions, but reaches about 322 ms at the default 4096×2048 screenshot size.
Browser-native alternative explored
Following this suggestion on #5789, we also explored
createImageBitmap(imageData, {imageOrientation: 'flipY'})followed by transfer to abitmaprenderercanvas.The comparison below includes buffer conversion and canvas writing for both paths, so these timings should not be compared directly with the flip-only results above.
Measured in ChromeHeadless 152 on the same machine: medians of five measurements after two warmups, alternating order. The flip-orientation probe passed.
A separate run extended the comparison to larger images, releasing each output canvas between measurements to limit memory use:
These are medians of three measurements after one warmup. Rendering, readback, and PNG encoding are excluded. Hardware acceleration was not verified, and main-thread responsiveness was not measured.
The browser-native approach provides a modest additional improvement at very large sizes, but would introduce asynchronous capture behavior, capability detection and fallback handling, canvas context management, and additional testing. Changing
getCanvas()to return a Promise would also affect existing callers.This PR therefore keeps the simpler CPU optimization, which delivers most of the measured improvement without changing the public API. An asynchronous capture API could be investigated separately if responsiveness during very large captures remains a problem.
Validation
git diff --checkpass.