Status Update
Comments
au...@google.com <au...@google.com> #2
The internal systemBarsForVisualComponents
right now only uses systemBars
, which doesn't include the display cutout:
Updating to systemBars.union(displayCutout)
should fix this, if that's a change we can make.
sp...@google.com <sp...@google.com> #3
Thank you for looking into this! It would be crucial to get this right soon as the edge2edge project depends on handling this properly :)
je...@google.com <je...@google.com> #4
Not taking the display cutout into account will impact cars heavily, since some have very large display cutouts:
sp...@google.com <sp...@google.com> #5
Project: platform/frameworks/support
Branch: androidx-main
Author: Alex Vanyo <
Link:
Add displayCutout to default Material insets
Expand for full commit details
Add displayCutout to default Material insets
Relnote: "Adds displayCutout to the group of insets that Material
components take into account by default, to avoid content overlapping
with the display cutout.
This is a behavior change that will impact how inset-aware
components behave around a display cutout. This includes the default
value of the WindowInsets parameter for inset-aware Material 3
components, and the WindowInsets values provided in the component
Defaults objects for both Material 2 and Material 3.
If this change causes undesirable behavior,
manually specify the WindowInsets parameter on a per-component basis."
Fixes: 362508045
Change-Id: I43ee9ad12db0450ebb9c65ce10d5c39d12628b6c
Files:
- M
compose/material/material/src/androidMain/kotlin/androidx/compose/material/SystemBarsDefaultInsets.android.kt
- M
compose/material3/material3/src/androidInstrumentedTest/kotlin/androidx/compose/material3/MaterialComponentsInsetSupportTest.kt
- M
compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/internal/SystemBarsDefaultInsets.android.kt
Hash: 692b440ac97b1831d5b35aafff241a98aeeacb93
Date: Wed Dec 18 19:49:15 2024
sp...@google.com <sp...@google.com> #6
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.compose.material:material:1.8.0-alpha08
androidx.compose.material:material-android:1.8.0-alpha08
androidx.compose.material:material-jvmstubs:1.8.0-alpha08
androidx.compose.material:material-linuxx64stubs:1.8.0-alpha08
androidx.compose.material3:material3:1.4.0-alpha06
androidx.compose.material3:material3-android:1.4.0-alpha06
androidx.compose.material3:material3-jvmstubs:1.4.0-alpha06
androidx.compose.material3:material3-linuxx64stubs:1.4.0-alpha06
da...@google.com <da...@google.com> #7
I don't think there's any more reason to include symbols in the test APK than in the main APK. The debugger will use the symbols in the out directory rather than in the APK anyway. The value in having the symbols on the device (in the APK) is that crashes in logcat will be readable (function names and potentially line numbers for each stack frame instead of just addresses).
Strip behavior isn't binary, btw. strip
has a handful of modes. --strip-unneeded
is the nuclear option for minimal size, and is good for release builds of apps, but --strip-debug
will preserve symbol names (but not line numbers). If you can't afford the APK size that full debug info would require, --strip-debug
is a really good compromise for debug builds (it's cheap and helpful enough that some OS libraries even ship this way). ndk-build's LOCAL_STRIP_MODE
option takes a string, not a bool (externalNativeBuild
pulls its artifacts from the intermediate directory, so it's irrelevant for AGP, and the packaging step will apply --strip-unneeded
or nothing depending on the user option). Taking a string or enum in AGP might be a good idea.
sp...@google.com <sp...@google.com> #8
Thanks for the insights, Dan!
Do I understand correctly that using --strip-debug
would improve readability in logcat for crashes involving native code (vs. --strip-unneeded
)? If so, yes, maybe it would make sense to default to --strip-debug
for debug and test APKs and default to --strip-unneeded
for release APKs.
AGP's StripDebugSymbolsTask
currently always uses --strip-unneeded
.
da...@google.com <da...@google.com> #9
Do I understand correctly that using --strip-debug would improve readability in logcat for crashes involving native code (vs. --strip-unneeded)?
Yes, but at the cost of increased APK size. It's significantly less than full debug info, but I'm not sure how much beyond that.
If so, yes, maybe it would make sense to default to --strip-debug for debug and test APKs and default to --strip-unneeded for release APKs.
I think that makes sense, yeah. Even if that only covers the tests and not the implementation, it'd still let you know which of your tests is crashing (though I'd hope that's clear from the test output).
em...@google.com <em...@google.com> #10
Jomo, do you know if AndroidTest APKs should be left unstripped by default for native debugging? I assumed they were left unstripped to support native debugging, but Xav pointed out that the APK being tested contains stripped native libraries by default, which makes me think maybe there's no strong reason to leave the AndroidTest APK native libraries unstripped.
Android Studio native debugger traverses the module dependency graph [1] to collect the directories that contain symbol files. It starts the search from the app.main
module (not the app.androidTest
module)[2], and therefore, only knows about the implementation
dependencies (not androidTestImplementation
dependencies). If the concern is about the symbol files for an .so
file that gets included in the test APK through an androidTestImplementation
dependency, those symbol files will not be discovered. Today, those .so
files are deployed to the Android device unstripped, and LLDB automatically pulls all .so
files from device to host, which means the user can debug them. If they will be stripped, then we need to fix this module dependency checking to preserve debugging behavior.
[1]
[2]
sp...@google.com <sp...@google.com> #11
Thanks, Emre!
Based on #10, it seems AGP's current behavior makes sense.
Regardless of whether we keep or change the default behavior, it makes sense to me to add an API to either (1) enable/disable stripping per variant / component or (2) set the "local strip mode" per variant / component.
sp...@google.com <sp...@google.com> #13
This was discussed at the AGP API meeting on Feb 1. There was no consensus about the API, but there was consensus that AS should support debugging, whether the android test APK is stripped or not.
With that in mind, I wrote
As it turns out, all of the following cases of debugging android tests are already broken, regardless of whether the test APK native libraries are stripped:
- Error when trying to debug android test of library from ASIssue 268213434 - connected test fails if androidTestImplementation dependency on android native libraryIssue 268083076 - Unable to debug androidTest-specific native codeIssue 268223043
The good news is that we can merge
au...@google.com <au...@google.com> #14
Any updates here? Could we make progress on this? We still have massively large APKs due to not doing any stripping on camke made binaries
sp...@google.com <sp...@google.com> #15
A fix on the AGP side is here:
Emre, now that
em...@google.com <em...@google.com> #16
After the change I did in app.androidTest
module or the app.main
module (as opposed to the app
holder module), and propagate that throughout the native debugger in Android Studio.
However, when we get the module dependencies to collect symbols in app.main
module.
There can be other places where we call facet.getMainModule()
which we might need to update.
Before looking into this, it would be nice to create a simple test app that demonstrates the problem.
au...@google.com <au...@google.com> #17
androidx-main has several projects that hit this, you can just look for cmake usage in build.gradle files.
sp...@google.com <sp...@google.com> #18
Before looking into this, it would be nice to create a simple test app that demonstrates the problem.
Would it make sense to go ahead and merge
em...@google.com <em...@google.com> #19
The test app from
A more complex test app would have:
- An 'app' module with native code.
- A 'lib' module with native code.
- An app-to-lib
androidTestImplementation
dependency.
And, test cases:
- Run instrumented test of 'app' module, verify:
a. hit native breakpoints in 'app' module
b. hit native breakpoints in 'lib' module - Run instrumented test of 'lib' module, verify:
a. hit native breakpoints in 'lib' module.
Current Status (studio-main, around Hedgehog canary 3):
- The following tests pass: 1(a), 2(a)
- The following tests fail: 1(b)
I prepared ag/23066587, which also makes 1(b) pass. I need to add some integration tests to that change.
spollom@ Feel free to submit your ag/21235820.
sp...@google.com <sp...@google.com> #20
Thanks, Emre!
sp...@google.com <sp...@google.com> #21
spollom@ Feel free to submit your ag/21235820.
Just merged it.
em...@google.com <em...@google.com> #22
I just merged ag/23066587, which should fix the debugger for 1(b) in comment19. Adding integration tests is still WIP, it's done but is flaky due to some unexpected resource leaks.
au...@google.com <au...@google.com> #23
Is it too risky to cp this to 8.1.0?
sp...@google.com <sp...@google.com> #24
hs...@gmail.com <hs...@gmail.com> #26
Could someone share in which version of AGP this issue is (planned to be) fixed?
em...@google.com <em...@google.com> #27
The fix is in 8.2.0 and newer. It's currently in beta, but a stable version will be available soon.
Description
Currently,
AndroidTest.packaging.jniLibs.keepDebugSymbols
has no effect on native library stripping in androidTest APKs... native libraries are packaged unstripped regardless.Proposal: For androidTest APKs, AGP should continue to package unstripped native libraries by default, but ifAndroidTest.packaging.jniLibs.keepDebugSymbols
contains any patterns, only the matching native libraries should be packaged unstripped.See Issue 228627720 for context.
Edit: See #3 for new proposal.