Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a11y wrong bounds calculation #1165

Merged
merged 2 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -575,7 +575,10 @@ private class AccessibilityElement(

override fun accessibilityFrame(): CValue<CGRect> =
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.convertToAppWindowCGRect(semanticsNode.boundsInWindow)
}


Expand Down Expand Up @@ -877,6 +880,12 @@ internal class AccessibilityMediator(
private val owner: SemanticsOwner,
coroutineContext: CoroutineContext,
private val getAccessibilitySyncOptions: () -> AccessibilitySyncOptions,

/**
* 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<CGRect>,
val performEscape: () -> Boolean
) {
/**
Expand Down Expand Up @@ -961,6 +970,12 @@ internal class AccessibilityMediator(
}
}

fun convertToAppWindowCGRect(rect: Rect): CValue<CGRect> {
val window = view.window ?: return CGRectZero.readValue()

return convertToAppWindowCGRect(rect, window)
}

fun onSemanticsChange() {
debugLogger?.log("onSemanticsChange")

Expand All @@ -975,13 +990,6 @@ internal class AccessibilityMediator(

}

fun convertRectToWindowSpaceCGRect(rect: Rect): CValue<CGRect> {
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" }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,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.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 androidx.compose.ui.unit.toRect
import kotlinx.cinterop.useContents
import platform.UIKit.UIView

Expand All @@ -33,6 +38,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) {
Expand Down Expand Up @@ -65,6 +73,33 @@ internal class PlatformWindowContext {
)
}

/**
* Converts the given [boundsInWindow] from the coordinate space of the container window to [toView] space.
*/
fun convertWindowRect(boundsInWindow: Rect, toView: UIView): Rect {
val windowContainer = _windowContainer ?: return boundsInWindow
return convertRect(
rect = boundsInWindow,
fromView = windowContainer,
toView = toView
)
}

private fun convertRect(rect: Rect, fromView: UIView, toView: UIView): Rect {
return if (fromView != toView) {
val density = fromView.systemDensity

fromView.convertRect(
rect = rect.toDpRect(density).asCGRect(),
toView = toView,
).useContents {
asDpRect().toRect(density)
}
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check if windowContainerWindow != windowContainer

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

windowContainer.window will be null in case it's true, it's a redundant check.

rect
}
}

private fun convertPoint(point: Offset, fromView: UIView, toView: UIView): Offset {
return if (fromView != toView) {
val density = fromView.systemDensity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -64,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
Expand All @@ -83,6 +87,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
Expand All @@ -99,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

/**
Expand All @@ -114,27 +120,29 @@ 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 convertToAppWindowCGRect: (Rect, UIWindow) -> CValue<CGRect>,
private val performEscape: () -> Boolean
) : PlatformContext.SemanticsOwnerListener {
var current: Pair<SemanticsOwner, AccessibilityMediator>? = null

override fun onSemanticsOwnerAppended(semanticsOwner: SemanticsOwner) {
if (current == null) {
current = semanticsOwner to AccessibilityMediator(
container,
rootView,
semanticsOwner,
coroutineContext,
getAccessibilitySyncOptions,
convertToAppWindowCGRect,
performEscape
)
}
Expand Down Expand Up @@ -294,6 +302,11 @@ internal class ComposeSceneMediator(
getAccessibilitySyncOptions = {
configuration.accessibilitySyncOptions
},
convertToAppWindowCGRect = { rect, window ->
windowContext.convertWindowRect(rect, window)
.toDpRect(Density(window.screen.scale.toFloat()))
.asCGRect()
},
performEscape = {
val down = onKeyboardEvent(
KeyEvent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
Loading