onGloballyPositioned -- should it report the position of the modifier? [177926591]

Fixed

Bug

Status Update

No update yet.

Description

mo...@google.com

created issue #1

Jan 19, 2021 08:26PM

Based on comment: https://android-review.googlesource.com/c/platform/frameworks/support/+/1552681/comment/e10d2507_68d1ed41/

We've changed onSizeChanged to report the size at the position of the modifier, but onGloballyPositioned() reports the size/position of the content. Should this be changed to be the size/position of the place in the modifier chain? Andrey, as you wrote the original modifier, you know the use cases better and can comment on it.

Comments

po...@google.com <po...@google.com> #2Jan 19, 2021 08:44PM

Hi,

I also see my text cut off when I set maxLines = 2. Is it the same issue?

  Box(
    modifier =
      Modifier.size(
        width = 108dp,
        height = 34dp,
      ),
    contentAlignment = Alignment.Center,
  ) {
    BasicText(
      text = "text text text",
      maxLines = 2,
      autoSize = AutoSize.StepBased(minFontSize = 1.sp, maxFontSize = 13.sp, stepSize = 0.2.sp),
    )
  }

an...@google.com <an...@google.com> #3Jan 20, 2021 03:19PM

Project: platform/frameworks/support
Branch: androidx-main
Author: Jossi Wolf <jossiwolf@google.com>
Link: https://android-review.googlesource.com/3450389

Fix TextAutoSize bug with maxLines = 1

Expand for full commit details

Fix TextAutoSize bug with maxLines = 1 
 
We were overcaching the paragraphIntrinsics in MultiParagraphLayoutCache when mutating the style. For `AutoSizeStepBased` instances with biased windows (more values smaller/bigger than the optimal), this could result in performing layout with outdated intrinsics, and thus an outdated style and font size, without surfacing this in the TextLayoutResult. 
 
Test: New MultiParagraphLayoutCacheTests and manual testing 
Relnote: Fixed a bug in BasicText with TextAutoSize and maxLines set to 1. 
Fixes: 376834366 
 
Change-Id: Ic0450c763c5d764492995b44ee1fe570246a9689

Files:

M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/text/modifiers/MultiParagraphLayoutCacheTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/modifiers/MultiParagraphLayoutCache.kt

Hash: e1b712d78cc60384ed67a56c006148291ba146a6
Date: Tue Jan 07 18:52:26 2025

mo...@google.com <mo...@google.com> #4Jan 21, 2021 11:23PM

#2, yeah, that's the same issue.

ap...@google.com <ap...@google.com> #5Jan 22, 2021 11:24PM

Thanks @jo...@google.com for fixing this! Do you know when the fix would be available for g3 apps?

ae...@google.com <ae...@google.com> #6May 14, 2021 12:26AM

Reassigned to mo...@google.com.

Moving the internal discussion offline. The bug is fixed and the fix available in snapshot builds. We will comment on this issue when the bug fix is included in a release.

Message last modified on May 14, 2021 12:27AM

an...@google.com <an...@google.com> May 14, 2021 12:10PM

Reassigned to an...@google.com.

an...@google.com <an...@google.com> #7May 14, 2021 12:21PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.compose.foundation:foundation:1.8.0-beta01
androidx.compose.foundation:foundation-android:1.8.0-beta01
androidx.compose.foundation:foundation-jvmstubs:1.8.0-beta01
androidx.compose.foundation:foundation-linuxx64stubs:1.8.0-beta01

ae...@google.com <ae...@google.com> #8May 14, 2021 08:33PM

Great, thanks Andrey!

ap...@google.com <ap...@google.com> #9May 17, 2021 05:05PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit eef6d918107789be7c43b293df82d2b9ae02f058
Author: Andrey Kulikov <andreykulikov@google.com>
Date: Fri May 14 13:12:45 2021

Change Modifier.onGloballyPositioned to report the modifier coordinates

Relnote: Modifier.onGloballyPositioned() was changed to report the coordinates of this modifier in the modifier chain, not the layout coordinates after applying all the modifiers. This means that now the ordering of modifiers is affecting what coordinates would be reported.
Fixes: 177926591
Test: OnGloballyPositionedTest
Change-Id: Ieb67da0c327c9dc323a4b0a8bf33dbb66f0611e3

M compose/animation/animation/src/androidAndroidTest/kotlin/androidx/compose/animation/AnimatedVisibilityTest.kt
M compose/foundation/foundation-layout/src/androidAndroidTest/kotlin/androidx/compose/foundation/layout/AlignmentLineTest.kt
M compose/foundation/foundation-layout/src/androidAndroidTest/kotlin/androidx/compose/foundation/layout/IntrinsicTest.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/IndicationSamples.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/textfield/TextFieldDefaultWidthTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/CoreTextField.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/BottomSheetScaffoldTest.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/ScaffoldTest.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/Drawer.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/ModalBottomSheet.kt
M compose/ui/ui-inspection/src/androidTest/java/androidx/compose/ui/inspection/rules/ComposeInspectionRule.kt
M compose/ui/ui-tooling-data/src/androidTest/java/androidx/compose/ui/tooling/data/ToolingTest.kt
M compose/ui/ui-tooling/src/androidTest/java/androidx/compose/ui/tooling/ToolingTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/draw/GraphicsLayerTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/OnGloballyPositionedTest.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNode.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/OnGloballyPositionedModifierWrapper.kt

https://android-review.googlesource.com/1707955