Status Update
No update yet.
Comments
co...@google.com
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
lp...@google.com
lp...@google.com
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
al...@gmail.com
The issue exists in the current material catalog app (which is the code I linked too) so I figured they could just use that to reproduce it (since all you have to do is go to the top app bar screen in the catalog demo app, scroll the screen container, and then rotate the screen). But I could also create a standalone separate Android Studio project with the same code as what is in the material catalog app if that is the preferred format.
an...@google.com
LazyListState knows nothing about how other participants of nested scrolling mechanism used the unused deltas. So you will not be able to get this info from the restored LazyListState.
So
remember { TopAppBarDefaults.pinnedScrollBehavior() }
is the only source of truth for such info and if it is desired to be able to restore its state it should have rememberSaveable.
Also note that it is in fact not only about restoration after screen rotation. Does AppBar display the state you want it to when you initialise LazyColumn with not 0 offset straight away? I mean something like
LazyColumn(state = rememberLazyListState(initialFirstVisibleItemIndex = 2, initialFirstVisibleItemScrollOffset = 10))
al...@gmail.com
In response to
I think there would still be a problem using rememberSaveable though. Because for example consider:
- You start out in landscape in a list screen
- You scroll down a tiny bit in the list screen causing the top app bar to animate its color
- You rotate the device to portrait (and let's assume the list is small enough such that the entire list is shown, making the list no longer scrollable).
The desired behavior here would be that the top app bar would switch back to its initial state since the scroll position in portrait has now become 0, right?
So seems like there would need to be some way to have the top app bar and scroll position linked somehow...
sg...@google.com
sg...@google.com
ap...@google.com
Branch: androidx-main
commit e260141f2a679c33bfbe576d3a73fd4338d54d1a
Author: Shalom Gibly <sgibly@google.com>
Date: Fri May 06 12:50:36 2022
Adds support for saving a top app bar state
- Adds a TopAppBarScrollState and use it inside the
TopAppBarScrollBehavior implementations.
- The state is provides a Saver and used by a rememberSaveable to
maintain the app bar position on configuration change.
Test: AppBarTest
Bug: 216160958
Relnote: "Supports maintaining the top app bar position on configuration
change."
Change-Id: I104599fa724196bbf1fec1bfa424a2a70abaf2fe
M compose/material3/material3/api/restricted_current.txt
M compose/material3/material3/samples/src/main/java/androidx/compose/material3/samples/AppBarSamples.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/AppBar.kt
M compose/material3/material3/api/current.txt
M compose/material3/material3/integration-tests/material3-catalog/src/main/java/androidx/compose/material3/catalog/library/ui/common/CatalogScaffold.kt
M compose/material3/material3/api/public_plus_experimental_current.txt
M compose/material3/material3/src/androidAndroidTest/kotlin/androidx/compose/material3/AppBarTest.kt
M compose/integration-tests/demos/src/main/java/androidx/compose/integration/demos/DemoApp.kt
sg...@google.com
The main part of this issue was fixed, and we made changes to the API to support it.
Example usage:
val scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior(rememberTopAppBarScrollState())
SmallTopAppBar(
....
scrollBehavior = scrollBehavior
) ...
We may still have issues with cases mentioned at
al...@gmail.com
Does it make sense to create a separate issue to track the remaining bug?
Description
When you scroll a screen using a material3 top app bar, the top app bar's scroll container color changes. However, if you rotate the screen afterwards, the
LazyColumn's scroll position will persist but the top app bar's color will reset back as if the screen was not scrolled at all.I attached a video illustrating the problem. The video is taken from the material demo app in AOSP, specifically this demo