Skip to content

Commit 153c6ea

Browse files
romtsnclaude
andcommitted
test(replay): Cover SurfaceView capture paths
Add unit tests for the new SurfaceView capture support and extract a compositeSurfaceViewInto helper so the drawing contract can be verified with hand-built bitmaps (Robolectric's ShadowPixelCopy cannot produce meaningful SurfaceView pixels because there is no real GL producer). The tests cover: - ViewHierarchyNode.fromView returns SurfaceViewHierarchyNode vs. generic - View.traverse collects SurfaceView nodes when a list is supplied, not when it is null, and skips invisible SurfaceViews - PixelCopyStrategy leaves hasSurfaceViews false when the option is off - PixelCopyStrategy flags hasSurfaceViews true when the option is on - PixelCopyStrategy completes gracefully when a SurfaceView has no valid surface (the common Robolectric case) - compositeSurfaceViewInto fills transparent holes behind existing window content via DST_OVER, and respects both window offset and scale factors Also fixes a latent NPE in captureSurfaceViews when SurfaceHolder.surface is null (not just invalid) — happens before the surface is created. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent df58de1 commit 153c6ea

4 files changed

Lines changed: 352 additions & 12 deletions

File tree

sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,9 @@ internal class PixelCopyStrategy(
170170

171171
for ((index, node) in surfaceViewNodes.withIndex()) {
172172
val surfaceView = node.surfaceViewRef.get()
173-
if (surfaceView == null || !surfaceView.holder.surface.isValid) {
173+
// holder.surface can be null before the surface is created — guard against NPE.
174+
val surface = surfaceView?.holder?.surface
175+
if (surfaceView == null || surface == null || !surface.isValid) {
174176
onCaptureComplete()
175177
continue
176178
}
@@ -220,18 +222,19 @@ internal class PixelCopyStrategy(
220222
if (capture == null) continue
221223
if (capture.bitmap.isRecycled) continue
222224

223-
val left = (capture.x - windowLocation[0]) * config.scaleFactorX
224-
val top = (capture.y - windowLocation[1]) * config.scaleFactorY
225-
tmpSrcRect.set(0, 0, capture.bitmap.width, capture.bitmap.height)
226-
tmpDstRect.set(
227-
left,
228-
top,
229-
left + capture.bitmap.width * config.scaleFactorX,
230-
top + capture.bitmap.height * config.scaleFactorY,
225+
compositeSurfaceViewInto(
226+
screenshotCanvas,
227+
dstOverPaint,
228+
tmpSrcRect,
229+
tmpDstRect,
230+
capture.bitmap,
231+
capture.x,
232+
capture.y,
233+
windowLocation[0],
234+
windowLocation[1],
235+
config.scaleFactorX,
236+
config.scaleFactorY,
231237
)
232-
233-
// DST_OVER draws the SurfaceView content behind the existing Window content
234-
screenshotCanvas.drawBitmap(capture.bitmap, tmpSrcRect, tmpDstRect, dstOverPaint)
235238
capture.bitmap.recycle()
236239
}
237240

@@ -277,3 +280,38 @@ internal class PixelCopyStrategy(
277280
)
278281
}
279282
}
283+
284+
/**
285+
* Composites [sourceBitmap] (a SurfaceView capture) onto [destCanvas] (wrapping the recording
286+
* screenshot) using [destPaint] (expected to have DST_OVER xfermode), so the SurfaceView content
287+
* draws _behind_ existing Window content — filling the transparent holes the Window PixelCopy
288+
* leaves where SurfaceViews are.
289+
*
290+
* Extracted for testability — the compositing is pure drawing logic that can be driven with
291+
* hand-built bitmaps, while the surrounding [PixelCopyStrategy.captureSurfaceViews] flow depends
292+
* on a real SurfaceView producer that Robolectric cannot provide.
293+
*/
294+
internal fun compositeSurfaceViewInto(
295+
destCanvas: Canvas,
296+
destPaint: Paint,
297+
tmpSrc: Rect,
298+
tmpDst: RectF,
299+
sourceBitmap: Bitmap,
300+
sourceX: Int,
301+
sourceY: Int,
302+
windowX: Int,
303+
windowY: Int,
304+
scaleFactorX: Float,
305+
scaleFactorY: Float,
306+
) {
307+
val left = (sourceX - windowX) * scaleFactorX
308+
val top = (sourceY - windowY) * scaleFactorY
309+
tmpSrc.set(0, 0, sourceBitmap.width, sourceBitmap.height)
310+
tmpDst.set(
311+
left,
312+
top,
313+
left + sourceBitmap.width * scaleFactorX,
314+
top + sourceBitmap.height * scaleFactorY,
315+
)
316+
destCanvas.drawBitmap(sourceBitmap, tmpSrc, tmpDst, destPaint)
317+
}

sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,19 @@
11
package io.sentry.android.replay.screenshot
22

33
import android.app.Activity
4+
import android.graphics.Bitmap
5+
import android.graphics.Canvas
6+
import android.graphics.Color
7+
import android.graphics.Paint
8+
import android.graphics.PorterDuff
9+
import android.graphics.PorterDuffXfermode
10+
import android.graphics.Rect
11+
import android.graphics.RectF
412
import android.os.Bundle
513
import android.os.Handler
614
import android.os.Looper
15+
import android.view.SurfaceView
16+
import android.widget.FrameLayout
717
import android.widget.LinearLayout
818
import android.widget.LinearLayout.LayoutParams
919
import android.widget.TextView
@@ -18,18 +28,23 @@ import java.util.concurrent.ScheduledExecutorService
1828
import java.util.concurrent.atomic.AtomicReference
1929
import kotlin.test.BeforeTest
2030
import kotlin.test.Test
31+
import kotlin.test.assertEquals
2132
import kotlin.test.assertFalse
33+
import kotlin.test.assertTrue
2234
import org.junit.runner.RunWith
2335
import org.mockito.kotlin.any
2436
import org.mockito.kotlin.doAnswer
2537
import org.mockito.kotlin.mock
38+
import org.mockito.kotlin.verify
2639
import org.mockito.kotlin.whenever
2740
import org.robolectric.Robolectric.buildActivity
2841
import org.robolectric.Shadows.shadowOf
2942
import org.robolectric.annotation.Config
43+
import org.robolectric.annotation.GraphicsMode
3044
import org.robolectric.shadows.ShadowPixelCopy
3145

3246
@Config(shadows = [ShadowPixelCopy::class], sdk = [30])
47+
@GraphicsMode(GraphicsMode.Mode.NATIVE)
3348
@RunWith(AndroidJUnit4::class)
3449
class PixelCopyStrategyTest {
3550

@@ -54,6 +69,18 @@ class PixelCopyStrategyTest {
5469
debugOverlayDrawable,
5570
)
5671
}
72+
73+
/** Executor mock that runs submitted tasks synchronously on the calling thread. */
74+
fun inlineExecutor(): ScheduledExecutorService {
75+
return mock {
76+
doAnswer {
77+
(it.arguments[0] as Runnable).run()
78+
null // submit(Runnable) returns Future<?>; returning Unit breaks the cast
79+
}
80+
.whenever(mock)
81+
.submit(any<Runnable>())
82+
}
83+
}
5784
}
5885

5986
private val fixture = Fixture()
@@ -101,6 +128,125 @@ class PixelCopyStrategyTest {
101128

102129
if (failure.get() != null) throw failure.get()
103130
}
131+
132+
@Test
133+
fun `capture does not flag hasSurfaceViews when option is disabled`() {
134+
val activity = buildActivity(ActivityWithSurfaceView::class.java).setup()
135+
shadowOf(Looper.getMainLooper()).idle()
136+
137+
// Default: isCaptureSurfaceViews = false
138+
val strategy = fixture.getSut(executor = fixture.inlineExecutor())
139+
strategy.capture(activity.get().findViewById(android.R.id.content))
140+
shadowOf(Looper.getMainLooper()).idle()
141+
142+
assertFalse(strategy.hasSurfaceViews())
143+
assertTrue(strategy.lastCaptureSuccessful())
144+
verify(fixture.callback).onScreenshotRecorded(any<Bitmap>())
145+
}
146+
147+
@Test
148+
fun `capture flags hasSurfaceViews when option is enabled and SurfaceView is present`() {
149+
val activity = buildActivity(ActivityWithSurfaceView::class.java).setup()
150+
shadowOf(Looper.getMainLooper()).idle()
151+
152+
fixture.options.sessionReplay.isCaptureSurfaceViews = true
153+
154+
val strategy = fixture.getSut(executor = fixture.inlineExecutor())
155+
strategy.capture(activity.get().findViewById(android.R.id.content))
156+
shadowOf(Looper.getMainLooper()).idle()
157+
158+
assertTrue(strategy.hasSurfaceViews())
159+
}
160+
161+
@Test
162+
fun `capture completes when SurfaceView surface is not valid`() {
163+
// In Robolectric the SurfaceView holder surface is not valid — this exercises the
164+
// `surfaceView.holder.surface.isValid == false` branch: each SurfaceView skips its
165+
// PixelCopy and onCaptureComplete still fires, eventually running the compositor and
166+
// callback.
167+
val activity = buildActivity(ActivityWithSurfaceView::class.java).setup()
168+
shadowOf(Looper.getMainLooper()).idle()
169+
fixture.options.sessionReplay.isCaptureSurfaceViews = true
170+
171+
val strategy = fixture.getSut(executor = fixture.inlineExecutor())
172+
strategy.capture(activity.get().findViewById(android.R.id.content))
173+
shadowOf(Looper.getMainLooper()).idle()
174+
175+
assertTrue(strategy.lastCaptureSuccessful())
176+
verify(fixture.callback).onScreenshotRecorded(any<Bitmap>())
177+
}
178+
179+
@Test
180+
fun `compositeSurfaceViewInto draws source behind existing destination with DST_OVER`() {
181+
// Destination ("Window capture"): 100x100, opaque red in the top half,
182+
// fully transparent in the bottom half (the "hole" where the SurfaceView sits).
183+
val dest = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888)
184+
val destCanvas = Canvas(dest)
185+
destCanvas.drawColor(Color.RED)
186+
val clearPaint = Paint().apply { xfermode = PorterDuffXfermode(PorterDuff.Mode.CLEAR) }
187+
destCanvas.drawRect(0f, 50f, 100f, 100f, clearPaint)
188+
189+
// Source ("SurfaceView capture"): 100x50, solid blue — matches the hole.
190+
val source = Bitmap.createBitmap(100, 50, Bitmap.Config.ARGB_8888)
191+
source.eraseColor(Color.BLUE)
192+
193+
val dstOverPaint = Paint().apply { xfermode = PorterDuffXfermode(PorterDuff.Mode.DST_OVER) }
194+
compositeSurfaceViewInto(
195+
destCanvas = destCanvas,
196+
destPaint = dstOverPaint,
197+
tmpSrc = Rect(),
198+
tmpDst = RectF(),
199+
sourceBitmap = source,
200+
sourceX = 0,
201+
sourceY = 50,
202+
windowX = 0,
203+
windowY = 0,
204+
scaleFactorX = 1f,
205+
scaleFactorY = 1f,
206+
)
207+
208+
// Top region: still red (DST_OVER must not overwrite existing opaque pixels).
209+
assertEquals(Color.RED, dest.getPixel(50, 10))
210+
assertEquals(Color.RED, dest.getPixel(50, 49))
211+
// Bottom region: now blue (source filled the transparent hole).
212+
assertEquals(Color.BLUE, dest.getPixel(50, 50))
213+
assertEquals(Color.BLUE, dest.getPixel(99, 99))
214+
}
215+
216+
@Test
217+
fun `compositeSurfaceViewInto respects scale factors and window offset`() {
218+
// Destination is 50x50 (scaled recording), fully transparent.
219+
val dest = Bitmap.createBitmap(50, 50, Bitmap.Config.ARGB_8888)
220+
val destCanvas = Canvas(dest)
221+
222+
// Source is 40x40, solid green; its on-screen location is (20, 20).
223+
val source = Bitmap.createBitmap(40, 40, Bitmap.Config.ARGB_8888)
224+
source.eraseColor(Color.GREEN)
225+
226+
val dstOverPaint = Paint().apply { xfermode = PorterDuffXfermode(PorterDuff.Mode.DST_OVER) }
227+
compositeSurfaceViewInto(
228+
destCanvas = destCanvas,
229+
destPaint = dstOverPaint,
230+
tmpSrc = Rect(),
231+
tmpDst = RectF(),
232+
sourceBitmap = source,
233+
sourceX = 20,
234+
sourceY = 20,
235+
windowX = 10, // window is at (10, 10)
236+
windowY = 10,
237+
scaleFactorX = 0.5f, // 0.5x scale → destination coords halve
238+
scaleFactorY = 0.5f,
239+
)
240+
241+
// Expected destination rect: ((20-10)*0.5, (20-10)*0.5) = (5, 5), size 40*0.5 = 20x20
242+
// → occupies pixels [5..25) × [5..25). Check inside, on the edge, and just outside.
243+
assertEquals(Color.GREEN, dest.getPixel(5, 5))
244+
assertEquals(Color.GREEN, dest.getPixel(15, 15))
245+
assertEquals(Color.GREEN, dest.getPixel(24, 24))
246+
// Just outside the rect — still transparent.
247+
assertEquals(0, dest.getPixel(4, 4))
248+
assertEquals(0, dest.getPixel(25, 25))
249+
}
104250
}
105251

106252
private class SimpleActivity : Activity() {
@@ -123,3 +269,28 @@ private class SimpleActivity : Activity() {
123269
setContentView(linearLayout)
124270
}
125271
}
272+
273+
private class ActivityWithSurfaceView : Activity() {
274+
override fun onCreate(savedInstanceState: Bundle?) {
275+
super.onCreate(savedInstanceState)
276+
val root =
277+
FrameLayout(this).apply {
278+
setBackgroundColor(android.R.color.white)
279+
layoutParams =
280+
FrameLayout.LayoutParams(
281+
FrameLayout.LayoutParams.MATCH_PARENT,
282+
FrameLayout.LayoutParams.MATCH_PARENT,
283+
)
284+
}
285+
root.addView(
286+
TextView(this).apply {
287+
text = "Overlay"
288+
layoutParams = FrameLayout.LayoutParams(200, 50)
289+
}
290+
)
291+
root.addView(
292+
SurfaceView(this).apply { layoutParams = FrameLayout.LayoutParams(200, 200) }
293+
)
294+
setContentView(root)
295+
}
296+
}

0 commit comments

Comments
 (0)