Add Canvas Capture Strategy #4777
Conversation
…y-java into markushi/canvas-approach
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
- Add Canvas Capture Strategy ([#4777](https://github.com/getsentry/sentry-java/pull/4777))If none of the above apply, you can opt out of this check by adding |
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 604a261 | 380.65 ms | 451.27 ms | 70.62 ms |
| ee747ae | 382.73 ms | 435.41 ms | 52.68 ms |
| 3699cd5 | 423.60 ms | 495.52 ms | 71.92 ms |
| 17a0955 | 372.53 ms | 446.70 ms | 74.17 ms |
| ee747ae | 374.71 ms | 455.18 ms | 80.47 ms |
| d217708 | 355.34 ms | 381.39 ms | 26.05 ms |
| f634d01 | 375.06 ms | 420.04 ms | 44.98 ms |
| ee747ae | 357.79 ms | 421.84 ms | 64.05 ms |
| 1df7eb6 | 397.04 ms | 429.64 ms | 32.60 ms |
| ee747ae | 400.46 ms | 423.61 ms | 23.15 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 604a261 | 1.58 MiB | 2.10 MiB | 533.42 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| 3699cd5 | 1.58 MiB | 2.10 MiB | 533.45 KiB |
| 17a0955 | 1.58 MiB | 2.10 MiB | 533.20 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| d217708 | 1.58 MiB | 2.10 MiB | 532.97 KiB |
| f634d01 | 1.58 MiB | 2.10 MiB | 533.40 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| 1df7eb6 | 1.58 MiB | 2.10 MiB | 532.97 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
There was a problem hiding this comment.
are the changes in this class necessary? I don't see rootView or currentCaptureDelay being used anywhere
| private val processingHandler: Handler | ||
|
|
||
| init { | ||
| processingThread.start() |
There was a problem hiding this comment.
looking at this, we re-initialize ScreenshoRecorder every time there's a window/configuration change (which subsequently initializes this strategy), therefore we call thread.start and init the handler potentially multiple times during a single replay. Would it make sense to do that only once?
| return@Runnable | ||
| } | ||
| try { | ||
| holder.reader.setOnImageAvailableListener( |
There was a problem hiding this comment.
Is it possible to set this callback only once, or do we have to do it every time this Runnable runs?
| val hwImage = it?.acquireLatestImage() | ||
| if (hwImage != null) { | ||
| val hwScreenshot = toBitmap(hwImage) | ||
| screenshot = hwScreenshot |
There was a problem hiding this comment.
I guess we also have to synchronize access to screenshot (or use atomicRef) because it's being accessed from different threads potentially?
|
|
||
| val surface = holder.reader.surface | ||
| val canvas = surface.lockHardwareCanvas() | ||
| canvas.drawColor(Color.BLACK, PorterDuff.Mode.CLEAR) |
There was a problem hiding this comment.
could we use TRANSPARENT here actually? Wondering if there will be some weird artifacts due to us erasing with the black color
| } | ||
| if (holder == null) { | ||
| options.logger.log(SentryLevel.DEBUG, "No free Picture available, skipping capture") | ||
| executor.submitSafely(options, "screenshot_recorder.canvas", pictureRenderTask) |
There was a problem hiding this comment.
hm, is there a reason we submit here the task, even though we know there's no free picture? Or is this to submit any unprocessedPictures?
| } | ||
|
|
||
| private fun toBitmap(image: Image): Bitmap { | ||
| image.planes!!.let { |
There was a problem hiding this comment.
are we sure this is never null here 😅
| val color = paint.color | ||
| textPaint.color = Color.argb(100, Color.red(color), Color.green(color), Color.blue(color)) | ||
| tmpRect.offset(x.roundToInt(), y.roundToInt()) | ||
| drawRect(tmpRect, solidPaint) |
There was a problem hiding this comment.
oh btw, do you wanna use drawRoundRect so it aligns with the PixelCopy strategy? Same for images here
📜 Description
Adds an experimental option to enable an alternate way to capture screenshots, using a fake Canvas which draws rectangles instead of text, producing a safer way to mask all sensitive content.
💡 Motivation and Context
No masking issues.
💚 How did you test it?
Manually
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps