Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve screenshot performance #83

Merged
merged 13 commits into from
Jan 9, 2025
Merged

Improve screenshot performance #83

merged 13 commits into from
Jan 9, 2025

Conversation

mbarta
Copy link
Contributor

@mbarta mbarta commented Jan 9, 2025

This PR replaces the original View.drawToBitmap way of capturing screenshots for HotwireView with a modern and more performant PixelCopy API.

Along with this, the screenshot properties have been extracted to a HotwireScreenshotHolder to improve code organisation.


Using the old canvas -> bitmap drawing approach produced wildly inconsistent times to create a bitmap. Sometimes taking over 300-450ms:

viewScreenshotCreated ............. [size: 1280x2172, duration: 436ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 48ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 300ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 39ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 155ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 43ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 32ms]

Using the PixelCopy API is much faster and more consistent, typically under 20ms to produce a bitmap:

viewScreenshotCreated ............. [size: 1280x2172, duration: 21ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 12ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 13ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 11ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 12ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 12ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 12ms]
viewScreenshotCreated ............. [size: 1280x2172, duration: 9ms]

jayohms and others added 7 commits January 7, 2025 16:43
* main:
  Rename the request result class and move the redirect properties to its own data class
  Detect cross-origin visit redirects natively via OkHttp, instead of relying on javascript Fetch API, which has CORS limitations.
  Allow cross-origin redirects from link clicks to propose a new visit
…eenshot-performance

* origin/screenshot-performance:
  Rename the request result class and move the redirect properties to its own data class
  Detect cross-origin visit redirects natively via OkHttp, instead of relying on javascript Fetch API, which has CORS limitations.
  Allow cross-origin redirects from link clicks to propose a new visit
@mbarta mbarta requested a review from jayohms January 9, 2025 10:40
@mbarta mbarta self-assigned this Jan 9, 2025
Copy link
Contributor

@jayohms jayohms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, thank you!

@mbarta mbarta merged commit f44a3f4 into main Jan 9, 2025
1 check passed
@mbarta mbarta deleted the screenshot-performance branch January 9, 2025 13:57
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants