Status Update
Comments
se...@google.com <se...@google.com>
il...@gmail.com <il...@gmail.com> #2
Branch: androidx-main
commit a330c0d3bcdd41326f37968a60e6084ad4a2e32c
Author: Chet Haase <chet@google.com>
Date: Wed Jul 05 07:26:46 2023
Convert APIs using PointF to use Float instead
PointF is a convenient mechanism for passing around x.y values
representing 2D points. But there are downsides, including:
- Converting to PointF: You may not have the data in PointF form
to begin with, so using an API which takes PointF requires converting
the data to that form (including allocating a PointF object every time)
- Mutability: Point structures can be mutated internally, causing
unpredictability in what that mutation means. Should the library
react to those changes? Ignore them? Do defensive copies (requiring
even more allocations)? Using primitive types like Float make the
behavior more obvious (by making the data inherently immutable).
- Allocations: Whenever we use object types, there are necessarily
allocations on the Java heap for them. This puts pressure on the GC
at both allocation and collection time. Given the amount of points
being passed around (especially at morph creation time, when curves
are being split and created), this causes a lot of PointF objects to
be allocated (even temporarily). Using Float avoids that problem.
Also fixed bug with unclosed paths causing discontinuity at the
start/end point.
Bug: 276466399
Bug: 290254314
Test: integration and unit tests pass
Relnote: PointF parameters changed to Float pairs
Change-Id: Id4705d27c7be31b26ade8186b99fffe2e2f8450e
M graphics/graphics-shapes/api/current.txt
M graphics/graphics-shapes/api/restricted_current.txt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/CubicShapeTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/CubicTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/PolygonMeasureTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/PolygonTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/RoundedPolygonTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/ShapesTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/TestUtils.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Cubic.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/CubicShape.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/FeatureMapping.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/FloatMapping.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Morph.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/PolygonMeasure.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/RoundedPolygon.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Shapes.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Utils.kt
M graphics/integration-tests/testapp-compose/src/main/java/androidx/graphics/shapes/testcompose/DebugDraw.kt
M graphics/integration-tests/testapp-compose/src/main/java/androidx/graphics/shapes/testcompose/ShapeEditor.kt
M graphics/integration-tests/testapp/src/main/java/androidx/graphics/shapes/test/MaterialShapes.kt
mp...@google.com <mp...@google.com>
ap...@google.com <ap...@google.com> #3
sg...@google.com <sg...@google.com> #4
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.graphics:graphics-shapes:1.0.0-alpha04
[Deleted User] <[Deleted User]> #5
Is there an example somewhere of the new expanded and collapsed height parameters being used with dynamic font sizes? I've tried measuring the text, using onGloballyPositioned
, etc, and nothing seems to quite work in both the collapsed and expanded states for all text sizes, especially if the increased font size makes the text wrap to a second line
sg...@google.com <sg...@google.com> #6
The most simplistic approach I can think of is using the LocalConfiguration.current.fontScale
.
@Composable
fun AppBarDynamicHeightSamples() {
val scrollBehavior = TopAppBarDefaults.exitUntilCollapsedScrollBehavior()
val expandedHeightRatio = LocalConfiguration.current.fontScale
Scaffold(
modifier = Modifier.nestedScroll(scrollBehavior.nestedScrollConnection),
topBar = {
TopAppBar(
title = {
Text("Title")
},
expandedHeight = TopAppBarDefaults.TopAppBarExpandedHeight * expandedHeightRatio,
colors = TopAppBarDefaults.topAppBarColors(containerColor = Color.Green),
scrollBehavior = scrollBehavior
)
},
content = { ... }
}
)
}
But note that the docs indicate that: "Please do not use this to hardcode font size equations. The equation for font scaling is now non-linear;... Please use android. util. TypedValue. applyDimension(int, float, DisplayMetrics) or android. util. TypedValue. deriveDimension(int, float, DisplayMetrics) to convert between scaled font size dimensions and pixels.", which can also help here to get a better formula.
Hope that helps
[Deleted User] <[Deleted User]> #7
Thank you for your response - unfortunately that approach only works in the most basic of circumstances (single line text). In the event that the text gets big enough to run onto two lines, or there's a subtitle, simply multiplying by the font scale is not sufficient.
Take the following as an example:
val heightRatio = LocalConfiguration.current.fontScale
MediumTopAppBar(
modifier = modifier,
title = {
Column {
Text(text = title)
Text(text = subtitle)
}
},
expandedHeight = TopAppBarDefaults.MediumAppBarExpandedHeight * heightRatio,
collapsedHeight = TopAppBarDefaults.MediumAppBarCollapsedHeight * heightRatio,
navigationIcon = /* back arrow */,
scrollBehavior = TopAppBarDefaults.exitUntilCollapsedScrollBehavior()
)
The expanded height takes into account space for the collapsed height as well, so multiplying by the font scale doesn't produce an accurate result.
It seems to me that this should be something the TopAppBar handles on its own - I would expect the app bar to wrap its content in both the expanded and collapsed states. Right now the default behavior is to cut off the content if it doesn't fit, which makes it difficult to use an customize, but more importantly is a serious accessibility issue for when the font size is increased, even for single-line titles.
Is there a way to have the app bar support this natively such that it has a minimum height (likely just the current default values), and expands to wrap its content in the event that the content is too big? I imagine this could be done while still leaving the collapsedHeight
and expandedHeight
fields as is, so that the developer could hard-code that if they'd like to.
I would expect this component to support font accessibility by default, so I'd love to see that change if it's something that's possible.
Description
Jetpack Compose component used: TopAppBar, Scaffold
Android Studio Build: #AI-223.8836.35.2231.10406996, built on June 29, 2023
Kotlin version: 1.9.10
Steps to Reproduce or Code Sample to Reproduce:
1. Set system font size to large
2. Open any screen using Scaffold with TopAppBar
Actual result: the title gets truncated