Add fix for boundsInWindow incorrectly calculating position on screen (#1983)

Author: geoff-powell

paparazzi/src/main/java/app/cash/paparazzi/accessibility/AccessibilityRenderExtension.kt

@@ -94,9 +94,9 @@ public class AccessibilityRenderExtension : RenderExtension {
 
   private fun View.processAccessibleChildren(processElement: (AccessibilityElement) -> Unit) {
     val accessibilityText = this.accessibilityText()
-    if (isImportantForAccessibility && !accessibilityText.isNullOrBlank() && isVisible) {
-      val bounds = Rect().also(::getBoundsOnScreen)
+    val bounds = Rect().also(::getBoundsOnScreen)
 
+    if (isImportantForAccessibility && !accessibilityText.isNullOrBlank() && isVisible) {
       processElement(
         AccessibilityElement(
           id = "${this::class.simpleName}($accessibilityText)",
@@ -111,9 +111,18 @@ public class AccessibilityRenderExtension : RenderExtension {
       val viewRoot = getChildAt(0) as ViewRootForTest
       val unmergedNodes = viewRoot.semanticsOwner.getAllSemanticsNodes(false)
 
+      // SemanticsNode.boundsInScreen isn't reported correctly for nodes so locationOnScreen used to correctly calculate displayBounds.
+      val locationOnScreen = arrayOf(bounds.left, bounds.top).toIntArray()
+      locationOnScreen[0] += paddingLeft
+      locationOnScreen[1] += paddingTop
       val orderedSemanticsNodes = viewRoot.semanticsOwner.rootSemanticsNode.orderSemanticsNodeGroup()
       orderedSemanticsNodes.forEach {
-        it.processAccessibleChildren(processElement, unmergedNodes)
+        it.processAccessibleChildren(
+          processElement = processElement,
+          locationOnScreen = locationOnScreen,
+          viewBounds = bounds,
+          unmergedNodes = unmergedNodes
+        )
       }
     }
 
@@ -173,6 +182,8 @@ public class AccessibilityRenderExtension : RenderExtension {
 
   private fun SemanticsNode.processAccessibleChildren(
     processElement: (AccessibilityElement) -> Unit,
+    locationOnScreen: IntArray,
+    viewBounds: Rect,
     unmergedNodes: List<SemanticsNode>?
   ) {
     val accessibilityText = if (config.isMergingSemanticsOfDescendants) {
@@ -188,9 +199,14 @@ public class AccessibilityRenderExtension : RenderExtension {
     }
 
     if (accessibilityText != null) {
-      val displayBounds = with(boundsInWindow) {
-        Rect(left.toInt(), top.toInt(), right.toInt(), bottom.toInt())
+      // SemanticsNode.boundsInScreen isn't reported correctly for nodes so boundsInRoot + locationOnScreen used to correctly calculate displayBounds.
+      val displayBounds = with(boundsInRoot) {
+        Rect(left.toInt(), top.toInt(), right.toInt(), bottom.toInt()).run {
+          offset(locationOnScreen[0], locationOnScreen[1])
+          Rect(left, top, right.coerceIn(0, viewBounds.right), bottom.coerceIn(0, viewBounds.bottom))
+        }
       }
+
       processElement(
         AccessibilityElement(
           // SemanticsNode.id is backed by AtomicInteger and is not guaranteed consistent across runs.
@@ -200,6 +216,15 @@ public class AccessibilityRenderExtension : RenderExtension {
         )
       )
     }
+
+    children.forEach {
+      it.processAccessibleChildren(
+        processElement = processElement,
+        locationOnScreen = locationOnScreen,
+        viewBounds = viewBounds,
+        unmergedNodes = unmergedNodes
+      )
+    }
   }
 
   private fun SemanticsNode.findAllUnmergedNodes(): List<SemanticsNode> {

sample/src/test/java/app/cash/paparazzi/sample/ComposeA11yTest.kt

@@ -1,5 +1,8 @@
 package app.cash.paparazzi.sample
 
+import android.view.ViewGroup.LayoutParams.MATCH_PARENT
+import android.view.ViewGroup.LayoutParams.WRAP_CONTENT
+import android.widget.LinearLayout
 import androidx.compose.foundation.Image
 import androidx.compose.foundation.clickable
 import androidx.compose.foundation.layout.Box
@@ -19,6 +22,7 @@ import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.filled.Add
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.platform.ComposeView
 import androidx.compose.ui.semantics.Role
 import androidx.compose.ui.semantics.contentDescription
 import androidx.compose.ui.semantics.heading
@@ -40,6 +44,39 @@ class ComposeA11yTest {
     renderExtensions = setOf(AccessibilityRenderExtension())
   )
 
+  @Test
+  fun multiComposeViews() {
+    val view = LinearLayout(paparazzi.context).apply {
+      orientation = LinearLayout.VERTICAL
+      layoutParams = LinearLayout.LayoutParams(MATCH_PARENT, MATCH_PARENT)
+      setPaddingRelative(25, 25, 25, 25)
+
+      addView(
+        ComposeView(context = paparazzi.context).apply {
+          setPaddingRelative(32, 32, 32, 32)
+          setContent { Text("Number 1") }
+        },
+        LinearLayout.LayoutParams(WRAP_CONTENT, 0, 1f)
+      )
+      addView(
+        ComposeView(context = paparazzi.context).apply {
+          setPaddingRelative(32, 32, 32, 32)
+          setContent { Text("Number 2") }
+        },
+        LinearLayout.LayoutParams(WRAP_CONTENT, 0, 1f)
+      )
+      addView(
+        ComposeView(context = paparazzi.context).apply {
+          setPaddingRelative(32, 32, 32, 32)
+          setContent { Text("Number 3") }
+        },
+        LinearLayout.LayoutParams(WRAP_CONTENT, 0, 1f)
+      )
+    }
+
+    paparazzi.snapshot(view)
+  }
+
   @Test
   fun compositeItems() {
     paparazzi.snapshot {