pedrovgs / Shot

Screenshot testing library for Android
Apache License 2.0
1.18k stars 115 forks source link

Compose LazyColumn bug when using activity to capture the screenshot. #257

Closed Laimiux closed 2 years ago

Laimiux commented 2 years ago

I'll preface this issue that I think the issue is down the chain (Facebook screenshot library or Compose). The reason I'm filing this here is to see if others have run into it here and to provide this to others who run into it.

Expected behaviour

The screenshot is not capturing layout correctly when using compareScreenshot(activity) with a LazyColumn composable.

Actual behaviour

There should be: Row 1, Row N, ..., Row 5 laid out in the list, but it all overlaps.

Steps to reproduce

Test activity

class TestActivity : ComponentActivity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContent {
            AppContent()
        }
    }
}

@Composable
fun AppContent() {
    DesignTheme(
        lightMode = true
    ) {
        Box(modifier = Modifier
            .fillMaxSize()
            .background(backgroundColor())
        ) {
            Column {
                Text("Lazy Column Rows (1-5):")

                LazyColumn(modifier = Modifier.fillMaxWidth().fillMaxHeight()) {
                    (1..5).forEach { value ->
                        item {
                            Text("Row $value")
                        }
                    }
                }
            }
        }
    }
}

The test logic

class LazyColumnTest : ScreenshotTest {

    @get:Rule
    val composeRule = createAndroidComposeRule<TestActivity>()

    @Test
    fun lazyColumnTest() {
        compareScreenshot(composeRule.activity)
    }
}

Version of the library

Used the latest 5.11.2

Laimiux commented 2 years ago

The issue really lies with how the Facebook screenshot library generates bitmaps. It uses an old way instead of relying on semi-new PixelCopy utility. I've copied logic from androidx.compose.ui.test which uses PixelCopy and integrated with the ScreenshotTest in Karumi/Shot:

object MyScreenshots {

    fun takeScreenshot(activity: Activity, name: String? = null) {
        val testObject = object : ScreenshotTest {

        }
        val bitmap: Bitmap = captureActivity(activity)
        testObject.compareScreenshot(bitmap, name)
    }
}

private fun captureActivity(activity: Activity): Bitmap {
    val windowToCapture = activity.window
    val handler = Handler(Looper.getMainLooper())

    awaitDraw(activity)

    val locationInWindow = intArrayOf(0, 0)
    val view = activity.window.decorView
    view.getLocationInWindow(locationInWindow)
    val x = locationInWindow[0]
    val y = locationInWindow[1]
    val captureRectInWindow: Rect = Rect(0, 0 , view.measuredWidth, view.measuredHeight)
    captureRectInWindow.offset(x, y)

    // and then request the pixel copy of the drawn buffer
    val destBitmap = Bitmap.createBitmap(
        captureRectInWindow.width(),
        captureRectInWindow.height(),
        Bitmap.Config.ARGB_8888
    )

    val latch = CountDownLatch(1)
    var copyResult = 0
    val onCopyFinished = PixelCopy.OnPixelCopyFinishedListener { result ->
        copyResult = result
        latch.countDown()
    }
    PixelCopy.request(windowToCapture, captureRectInWindow, destBitmap, onCopyFinished, handler)

    if (!latch.await(1, TimeUnit.SECONDS)) {
        throw AssertionError("Failed waiting for PixelCopy!")
    }
    if (copyResult != PixelCopy.SUCCESS) {
        throw AssertionError("PixelCopy failed!")
    }
    return destBitmap
}

private fun awaitDraw(activity: Activity) {

    val windowToCapture = activity.window
    val handler = Handler(Looper.getMainLooper())

    // first we wait for the drawing to happen
    val drawLatch = CountDownLatch(1)
    val decorView = windowToCapture.decorView
    handler.post {
        if (Build.VERSION.SDK_INT >= 29 && decorView.isHardwareAccelerated()) {
            decorView.viewTreeObserver.registerFrameCommitCallback {
                drawLatch.countDown()
            }
        } else {
            decorView.viewTreeObserver.addOnDrawListener(object : ViewTreeObserver.OnDrawListener {
                var handled = false
                override fun onDraw() {
                    if (!handled) {
                        handled = true
                        handler.post {
                            drawLatch.countDown()
                            decorView.viewTreeObserver.removeOnDrawListener(this)
                        }
                    }
                }
            })
        }
        decorView.invalidate()
    }
    if (!drawLatch.await(1, TimeUnit.SECONDS)) {
        throw AssertionError("Failed waiting for DecorView redraw!")
    }
}

Since the Facebook screenshot library doesn't seem to be well maintained, I wonder if Karumi/Shot should replace it with an internal implementation.

pedrovgs commented 2 years ago

I'm afraid the library we use under the hood is not compatible with compose so you'll have to test your composable function using the Compose API I created.

About this:

Since the Facebook screenshot library doesn't seem to be well maintained, I wonder if Karumi/Shot should replace it with an internal implementation

Due to the fact that I'm working on a different project right now and there are no more active contributors implementing new features, I don't think this is something we can do. At least for now.

Laimiux commented 2 years ago

The issue happens with mixed-use apps which will usually be the case during migration to Compose. Would you be open if I contributed it? I have a working setup in our codebase and could add it here behind an optional flag?

pedrovgs commented 2 years ago

I think, if we want to be pragmatic, we should ask for the feature to the original facebook library even before implementing this. Implementing our own solution when taking the screenshot is a huge change in our codebase and would change how shot works under the hood. We should first check if Facebook is providing support for this feature and later make a decision.