From 18839f2730fe2cc3497413ae72b81090935c30ba Mon Sep 17 00:00:00 2001 From: Elijah Semyonov Date: Wed, 6 Mar 2024 11:13:32 +0100 Subject: [PATCH 1/2] Fix incorrect a11y bounds calculation. --- .../ui/platform/Accessibility.uikit.kt | 13 +++--- .../platform/PlatformWindowContext.uikit.kt | 42 +++++++++++++++++++ .../ui/scene/ComposeSceneMediator.uikit.kt | 7 ++++ 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/Accessibility.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/Accessibility.uikit.kt index c0a434355f898..0adfb90c4ea82 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/Accessibility.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/Accessibility.uikit.kt @@ -575,7 +575,10 @@ private class AccessibilityElement( override fun accessibilityFrame(): CValue = getOrElse(CachedAccessibilityPropertyKeys.accessibilityFrame) { - mediator.convertRectToWindowSpaceCGRect(semanticsNode.boundsInWindow) + // AX services expect the frame to be in the coordinate space of the root UIWindow + // [semanticsNode.boundsInWindow] provide it in so called `window container` space, + // which can be different from the root UIWindow space. + mediator.convertContainerWindowRectToRootWindowCGRect(semanticsNode.boundsInWindow) } @@ -877,6 +880,7 @@ internal class AccessibilityMediator( private val owner: SemanticsOwner, coroutineContext: CoroutineContext, private val getAccessibilitySyncOptions: () -> AccessibilitySyncOptions, + val convertContainerWindowRectToRootWindowCGRect: (Rect) -> CValue, val performEscape: () -> Boolean ) { /** @@ -975,13 +979,6 @@ internal class AccessibilityMediator( } - fun convertRectToWindowSpaceCGRect(rect: Rect): CValue { - val window = view.window ?: return CGRectMake(0.0, 0.0, 0.0, 0.0) - val density = Density(window.screen.scale.toFloat()) - val localSpaceCGRect = rect.toDpRect(density).asCGRect() - return window.convertRect(localSpaceCGRect, fromView = view) - } - fun dispose() { check(isAlive) { "AccessibilityMediator is already disposed" } diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/PlatformWindowContext.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/PlatformWindowContext.uikit.kt index ba5d7fa6d2187..8327908127bce 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/PlatformWindowContext.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/PlatformWindowContext.uikit.kt @@ -17,14 +17,23 @@ package androidx.compose.ui.platform import androidx.compose.ui.geometry.Offset +import androidx.compose.ui.geometry.Rect import androidx.compose.ui.uikit.systemDensity +import androidx.compose.ui.unit.Density import androidx.compose.ui.unit.IntSize import androidx.compose.ui.unit.asCGPoint +import androidx.compose.ui.unit.asCGRect import androidx.compose.ui.unit.asDpOffset import androidx.compose.ui.unit.toDpOffset +import androidx.compose.ui.unit.toDpRect import androidx.compose.ui.unit.toOffset +import kotlinx.cinterop.CValue +import kotlinx.cinterop.readValue import kotlinx.cinterop.useContents +import platform.CoreGraphics.CGRect +import platform.CoreGraphics.CGRectZero import platform.UIKit.UIView +import platform.UIKit.UIWindow /** * Tracking a state of window. @@ -33,6 +42,9 @@ internal class PlatformWindowContext { private val _windowInfo = WindowInfoImpl() val windowInfo: WindowInfo get() = _windowInfo + /** + * A container used for additional layers and as reference for window coordinate space. + */ private var _windowContainer: UIView? = null fun setWindowFocused(focused: Boolean) { @@ -65,6 +77,36 @@ internal class PlatformWindowContext { ) } + /** + * Converts the given [Rect] from the coordinate space of the container window to the coordinate + * space of the root UIWindow in which hierarchy with the container window resides. + */ + fun convertContainerWindowRectToRootWindowCGRect(rect: Rect): CValue { + val windowContainer = _windowContainer ?: return CGRectZero.readValue() + + val windowContainerWindow = windowContainer.window + + fun UIWindow.density(): Density { + return Density(screen.scale.toFloat()) + } + + return if (windowContainerWindow == null) { + if (windowContainer is UIWindow) { + // windowContainer is the root window itself + rect.toDpRect(windowContainer.density()).asCGRect() + } else { + // windowContainer is not attached to any window + CGRectZero.readValue() + } + } else { + val cgRect = rect + .toDpRect(windowContainerWindow.density()) + .asCGRect() + + windowContainer.convertRect(cgRect, toView = windowContainerWindow) + } + } + private fun convertPoint(point: Offset, fromView: UIView, toView: UIView): Offset { return if (fromView != toView) { val density = fromView.systemDensity diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt index 3cd5187a5c915..89aaaeb7ec89d 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt @@ -23,6 +23,7 @@ import androidx.compose.runtime.InternalComposeApi import androidx.compose.runtime.MutableState import androidx.compose.runtime.mutableStateOf import androidx.compose.ui.geometry.Offset +import androidx.compose.ui.geometry.Rect import androidx.compose.ui.graphics.asComposeCanvas import androidx.compose.ui.input.InputMode import androidx.compose.ui.input.key.KeyEvent @@ -83,6 +84,7 @@ import org.jetbrains.skiko.SkikoKeyboardEventKind import platform.CoreGraphics.CGAffineTransformIdentity import platform.CoreGraphics.CGAffineTransformInvert import platform.CoreGraphics.CGPoint +import platform.CoreGraphics.CGRect import platform.CoreGraphics.CGRectMake import platform.CoreGraphics.CGRectZero import platform.CoreGraphics.CGSize @@ -124,6 +126,7 @@ private class SemanticsOwnerListenerImpl( private val container: UIView, private val coroutineContext: CoroutineContext, private val getAccessibilitySyncOptions: () -> AccessibilitySyncOptions, + private val convertContainerWindowRectToRootWindowCGRect: (Rect) -> CValue, private val performEscape: () -> Boolean ) : PlatformContext.SemanticsOwnerListener { var current: Pair? = null @@ -135,6 +138,7 @@ private class SemanticsOwnerListenerImpl( semanticsOwner, coroutineContext, getAccessibilitySyncOptions, + convertContainerWindowRectToRootWindowCGRect, performEscape ) } @@ -294,6 +298,9 @@ internal class ComposeSceneMediator( getAccessibilitySyncOptions = { configuration.accessibilitySyncOptions }, + convertContainerWindowRectToRootWindowCGRect = { + windowContext.convertContainerWindowRectToRootWindowCGRect(it) + }, performEscape = { val down = onKeyboardEvent( KeyEvent( From 2e055ecf6ff1aaa604623687124d920b75ec7d33 Mon Sep 17 00:00:00 2001 From: Elijah Semyonov Date: Wed, 6 Mar 2024 13:09:20 +0100 Subject: [PATCH 2/2] Refactor based on review --- .../compose/ui/unit/Density.jsNative.kt | 11 +++++ .../ui/platform/Accessibility.uikit.kt | 21 +++++++-- .../platform/PlatformWindowContext.uikit.kt | 47 ++++++++----------- .../ui/scene/ComposeSceneMediator.uikit.kt | 20 +++++--- .../compose/ui/uikit/Extensions.uikit.kt | 1 + 5 files changed, 61 insertions(+), 39 deletions(-) diff --git a/compose/ui/ui/src/jsNativeMain/kotlin/androidx/compose/ui/unit/Density.jsNative.kt b/compose/ui/ui/src/jsNativeMain/kotlin/androidx/compose/ui/unit/Density.jsNative.kt index e237e8b219c6e..c014b4f18cc4e 100644 --- a/compose/ui/ui/src/jsNativeMain/kotlin/androidx/compose/ui/unit/Density.jsNative.kt +++ b/compose/ui/ui/src/jsNativeMain/kotlin/androidx/compose/ui/unit/Density.jsNative.kt @@ -55,3 +55,14 @@ internal fun Rect.toDpRect(density: Density): DpRect = with(density) { size = size.toDpSize() ) } + +/** + * Convert a [DpRect] to a [Rect]. + */ +@Stable +internal fun DpRect.toRect(density: Density): Rect = with(density) { + Rect( + offset = DpOffset(left, top).toOffset(density), + size = size.toSize() + ) +} diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/Accessibility.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/Accessibility.uikit.kt index 0adfb90c4ea82..854f91836525b 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/Accessibility.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/Accessibility.uikit.kt @@ -27,20 +27,19 @@ import androidx.compose.ui.semantics.SemanticsProperties import androidx.compose.ui.semantics.getOrNull import androidx.compose.ui.state.ToggleableState import androidx.compose.ui.uikit.utils.* -import androidx.compose.ui.unit.Density -import androidx.compose.ui.unit.asCGRect -import androidx.compose.ui.unit.toDpRect import androidx.compose.ui.unit.toSize import kotlin.coroutines.CoroutineContext import kotlin.time.measureTime import kotlinx.cinterop.CValue import kotlinx.cinterop.ExportObjCClass +import kotlinx.cinterop.readValue import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.launch import platform.CoreGraphics.CGRect import platform.CoreGraphics.CGRectMake +import platform.CoreGraphics.CGRectZero import platform.Foundation.NSNotFound import platform.UIKit.NSStringFromCGRect import platform.UIKit.UIAccessibilityCustomAction @@ -64,6 +63,7 @@ import platform.UIKit.UIAccessibilityTraitSelected import platform.UIKit.UIAccessibilityTraitUpdatesFrequently import platform.UIKit.UIAccessibilityTraits import platform.UIKit.UIView +import platform.UIKit.UIWindow import platform.UIKit.accessibilityCustomActions import platform.UIKit.accessibilityElements import platform.UIKit.isAccessibilityElement @@ -578,7 +578,7 @@ private class AccessibilityElement( // AX services expect the frame to be in the coordinate space of the root UIWindow // [semanticsNode.boundsInWindow] provide it in so called `window container` space, // which can be different from the root UIWindow space. - mediator.convertContainerWindowRectToRootWindowCGRect(semanticsNode.boundsInWindow) + mediator.convertToAppWindowCGRect(semanticsNode.boundsInWindow) } @@ -880,7 +880,12 @@ internal class AccessibilityMediator( private val owner: SemanticsOwner, coroutineContext: CoroutineContext, private val getAccessibilitySyncOptions: () -> AccessibilitySyncOptions, - val convertContainerWindowRectToRootWindowCGRect: (Rect) -> CValue, + + /** + * A function that converts the given [Rect] from the semantics tree coordinate space (window container for layers) + * to the [CGRect] in coordinate space of the app window. + */ + val convertToAppWindowCGRect: (Rect, UIWindow) -> CValue, val performEscape: () -> Boolean ) { /** @@ -965,6 +970,12 @@ internal class AccessibilityMediator( } } + fun convertToAppWindowCGRect(rect: Rect): CValue { + val window = view.window ?: return CGRectZero.readValue() + + return convertToAppWindowCGRect(rect, window) + } + fun onSemanticsChange() { debugLogger?.log("onSemanticsChange") diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/PlatformWindowContext.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/PlatformWindowContext.uikit.kt index 8327908127bce..3c1390e754fdb 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/PlatformWindowContext.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/PlatformWindowContext.uikit.kt @@ -19,21 +19,17 @@ package androidx.compose.ui.platform import androidx.compose.ui.geometry.Offset import androidx.compose.ui.geometry.Rect import androidx.compose.ui.uikit.systemDensity -import androidx.compose.ui.unit.Density import androidx.compose.ui.unit.IntSize import androidx.compose.ui.unit.asCGPoint import androidx.compose.ui.unit.asCGRect import androidx.compose.ui.unit.asDpOffset +import androidx.compose.ui.unit.asDpRect import androidx.compose.ui.unit.toDpOffset import androidx.compose.ui.unit.toDpRect import androidx.compose.ui.unit.toOffset -import kotlinx.cinterop.CValue -import kotlinx.cinterop.readValue +import androidx.compose.ui.unit.toRect import kotlinx.cinterop.useContents -import platform.CoreGraphics.CGRect -import platform.CoreGraphics.CGRectZero import platform.UIKit.UIView -import platform.UIKit.UIWindow /** * Tracking a state of window. @@ -78,32 +74,29 @@ internal class PlatformWindowContext { } /** - * Converts the given [Rect] from the coordinate space of the container window to the coordinate - * space of the root UIWindow in which hierarchy with the container window resides. + * Converts the given [boundsInWindow] from the coordinate space of the container window to [toView] space. */ - fun convertContainerWindowRectToRootWindowCGRect(rect: Rect): CValue { - val windowContainer = _windowContainer ?: return CGRectZero.readValue() - - val windowContainerWindow = windowContainer.window + fun convertWindowRect(boundsInWindow: Rect, toView: UIView): Rect { + val windowContainer = _windowContainer ?: return boundsInWindow + return convertRect( + rect = boundsInWindow, + fromView = windowContainer, + toView = toView + ) + } - fun UIWindow.density(): Density { - return Density(screen.scale.toFloat()) - } + private fun convertRect(rect: Rect, fromView: UIView, toView: UIView): Rect { + return if (fromView != toView) { + val density = fromView.systemDensity - return if (windowContainerWindow == null) { - if (windowContainer is UIWindow) { - // windowContainer is the root window itself - rect.toDpRect(windowContainer.density()).asCGRect() - } else { - // windowContainer is not attached to any window - CGRectZero.readValue() + fromView.convertRect( + rect = rect.toDpRect(density).asCGRect(), + toView = toView, + ).useContents { + asDpRect().toRect(density) } } else { - val cgRect = rect - .toDpRect(windowContainerWindow.density()) - .asCGRect() - - windowContainer.convertRect(cgRect, toView = windowContainerWindow) + rect } } diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt index 89aaaeb7ec89d..f73296107b9a3 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt @@ -65,6 +65,9 @@ import androidx.compose.ui.window.FocusStack import androidx.compose.ui.window.InteractionUIView import androidx.compose.ui.interop.UIKitInteropContainer import androidx.compose.ui.node.TrackInteropContainer +import androidx.compose.ui.unit.Density +import androidx.compose.ui.unit.asCGRect +import androidx.compose.ui.unit.toDpRect import androidx.compose.ui.window.KeyboardEventHandler import androidx.compose.ui.window.KeyboardVisibilityListenerImpl import androidx.compose.ui.window.RenderingUIView @@ -101,6 +104,7 @@ import platform.UIKit.UITouch import platform.UIKit.UITouchPhase import platform.UIKit.UIView import platform.UIKit.UIViewControllerTransitionCoordinatorProtocol +import platform.UIKit.UIWindow import platform.darwin.NSObject /** @@ -116,17 +120,17 @@ internal sealed interface SceneLayout { /** * iOS specific-implementation of [PlatformContext.SemanticsOwnerListener] used to track changes in [SemanticsOwner]. * - * @property container The UI container associated with the semantics owner. + * @property rootView The UI container associated with the semantics owner. * @property coroutineContext The coroutine context to use for handling semantics changes. * @property getAccessibilitySyncOptions A lambda function to retrieve the latest accessibility synchronization options. * @property performEscape A lambda to delegate accessibility escape operation. Returns true if the escape was handled, false otherwise. */ @OptIn(ExperimentalComposeApi::class) private class SemanticsOwnerListenerImpl( - private val container: UIView, + private val rootView: UIView, private val coroutineContext: CoroutineContext, private val getAccessibilitySyncOptions: () -> AccessibilitySyncOptions, - private val convertContainerWindowRectToRootWindowCGRect: (Rect) -> CValue, + private val convertToAppWindowCGRect: (Rect, UIWindow) -> CValue, private val performEscape: () -> Boolean ) : PlatformContext.SemanticsOwnerListener { var current: Pair? = null @@ -134,11 +138,11 @@ private class SemanticsOwnerListenerImpl( override fun onSemanticsOwnerAppended(semanticsOwner: SemanticsOwner) { if (current == null) { current = semanticsOwner to AccessibilityMediator( - container, + rootView, semanticsOwner, coroutineContext, getAccessibilitySyncOptions, - convertContainerWindowRectToRootWindowCGRect, + convertToAppWindowCGRect, performEscape ) } @@ -298,8 +302,10 @@ internal class ComposeSceneMediator( getAccessibilitySyncOptions = { configuration.accessibilitySyncOptions }, - convertContainerWindowRectToRootWindowCGRect = { - windowContext.convertContainerWindowRectToRootWindowCGRect(it) + convertToAppWindowCGRect = { rect, window -> + windowContext.convertWindowRect(rect, window) + .toDpRect(Density(window.screen.scale.toFloat())) + .asCGRect() }, performEscape = { val down = onKeyboardEvent( diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/uikit/Extensions.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/uikit/Extensions.uikit.kt index f6bc784926592..e65c58954f64d 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/uikit/Extensions.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/uikit/Extensions.uikit.kt @@ -29,6 +29,7 @@ internal val UITraitEnvironmentProtocol.systemDensity: Density val contentSizeCategory = traitCollection.preferredContentSizeCategory ?: UIContentSizeCategoryUnspecified return Density( + // TODO: refactor to avoid mainScreen scale, window can be attached to different screens density = UIScreen.mainScreen.scale.toFloat(), fontScale = uiContentSizeCategoryToFontScaleMap[contentSizeCategory] ?: 1.0f )