Fixed
Status Update
Comments
cc...@google.com <cc...@google.com> #2
Sergio shared a WIP CL to calculate more correct bounds. You can see in the screenshot attached here that it does a better job of tightening the bounds around the object (the yellow box), but that it misses some of the curves which expand beyond these tighter bounds. In particular, note the two triangles and the two blob shapes (rotated round rects).
I've attached another 2 screenshots from the ShapeEditor view of one of the blobs, showing the shape and controls/anchors. You can see that the tighter bounds (yellow rectangle) are constrained to the anchors, but the curves at the end go beyond those points and exceed the bounds.
I've attached another 2 screenshots from the ShapeEditor view of one of the blobs, showing the shape and controls/anchors. You can see that the tighter bounds (yellow rectangle) are constrained to the anchors, but the curves at the end go beyond those points and exceed the bounds.
cc...@google.com <cc...@google.com> #3
Part of the problem is that the min/max values are assigned the anchors to begin with. That would cause the problem we're seeing in #2, because these new bounds are constrained to the rectangle created by the anchor points, even if the control points cause curves to go outside of those bounds.
But that's not all of the problem here. When I changed those min/max values to use the control points instead, the bounds expanded to be essentially the same as the estimated bounds from before. So there's more to it.
But that's not all of the problem here. When I changed those min/max values to use the control points instead, the bounds expanded to be essentially the same as the estimated bounds from before. So there's more to it.
ra...@google.com <ra...@google.com>
ra...@google.com <ra...@google.com> #4
Some of #3 is incorrect - the bounds can be expanded from the initial min/max values, so it is fine to have them start at the anchor points.
Meanwhile, I thought part of the problem might be calculating derivatives/etc for zero-length curves (of which there are many, given the way our shape rounding works). But adding in a check for this (setting the bounds of a curve to the same point when the anchors are equal within the distance epsilon) did not fix the problem; we still end up with bounds that are inside some of the curves (see attached screenshot)
Meanwhile, I thought part of the problem might be calculating derivatives/etc for zero-length curves (of which there are many, given the way our shape rounding works). But adding in a check for this (setting the bounds of a curve to the same point when the anchors are equal within the distance epsilon) did not fix the problem; we still end up with bounds that are inside some of the curves (see attached screenshot)
cc...@google.com <cc...@google.com>
cc...@google.com <cc...@google.com> #5
Project: platform/frameworks/support
Branch: androidx-main
commit 958577b15712233aa487003deb0ab37de3af7ab7
Author: Chet Haase <chet@google.com>
Date: Wed Dec 20 13:21:53 2023
Added more bounds calculation utilities
Previously, calculateBounds() returned the estimated bounds
for a RoundedPolygon. This is reasonable in most cases, but sometimes
you want the actual/minimal bounds instead. Also, sometimes you want
the max bounds for an object that might be rotated within its container,
so you need to size the container appropriately.
This CL introduces options for bounds calculations to handle these
new cases, along with some minor cleanups along the way.
Bug: 317286450
Test: Existing tests pass, added new bounds tests
Relnote: Now more options for retrieving exact and max bounds
Change-Id: I6d49f468a28c1f360000e8370f02a50841f744e4
M graphics/graphics-shapes/api/current.txt
M graphics/graphics-shapes/api/restricted_current.txt
M graphics/graphics-shapes/src/androidInstrumentedTest/kotlin/androidx/graphics/shapes/PolygonTest.kt
M graphics/graphics-shapes/src/commonMain/kotlin/androidx/graphics/shapes/Cubic.kt
M graphics/graphics-shapes/src/commonMain/kotlin/androidx/graphics/shapes/RoundedPolygon.kt
M graphics/graphics-shapes/src/commonMain/kotlin/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/MainActivity.kt
https://android-review.googlesource.com/2889768
Branch: androidx-main
commit 958577b15712233aa487003deb0ab37de3af7ab7
Author: Chet Haase <chet@google.com>
Date: Wed Dec 20 13:21:53 2023
Added more bounds calculation utilities
Previously, calculateBounds() returned the estimated bounds
for a RoundedPolygon. This is reasonable in most cases, but sometimes
you want the actual/minimal bounds instead. Also, sometimes you want
the max bounds for an object that might be rotated within its container,
so you need to size the container appropriately.
This CL introduces options for bounds calculations to handle these
new cases, along with some minor cleanups along the way.
Bug: 317286450
Test: Existing tests pass, added new bounds tests
Relnote: Now more options for retrieving exact and max bounds
Change-Id: I6d49f468a28c1f360000e8370f02a50841f744e4
M graphics/graphics-shapes/api/current.txt
M graphics/graphics-shapes/api/restricted_current.txt
M graphics/graphics-shapes/src/androidInstrumentedTest/kotlin/androidx/graphics/shapes/PolygonTest.kt
M graphics/graphics-shapes/src/commonMain/kotlin/androidx/graphics/shapes/Cubic.kt
M graphics/graphics-shapes/src/commonMain/kotlin/androidx/graphics/shapes/RoundedPolygon.kt
M graphics/graphics-shapes/src/commonMain/kotlin/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/MainActivity.kt
ap...@google.com <ap...@google.com> #6
Problem fixed - we now offer the option to request actual bounds (or faster/approximated, based on a parameter to calculateBounds()).
We also offer a new API, calculateMaxBounds(), which returns the square bounds in which the object can be rotated without ever exceeding the bounds.
We also offer a new API, calculateMaxBounds(), which returns the square bounds in which the object can be rotated without ever exceeding the bounds.
ap...@google.com <ap...@google.com> #7
Project: platform/frameworks/support
Branch: androidx-main
commit 0ba5c3dad9652a85319894f9aa7ad52664be72d2
Author: Chris Craik <ccraik@google.com>
Date: Tue Oct 18 20:56:23 2022
Fix deleteShaderCache by using Profileinstaller or fallback correctly with root
Test: SmallListStartupBenchmark
Test: MacrobenchmarkScopeTest
Fixes: 231455742
Relnote: """
BENCHMARK RELEASE NOTES
Fixed MacrobenchmarkScope.dropShaderCache(). This removes roughly 20ms
of noise from StartupMode.COLD benchmarks, as shaders are now
consistently cleared each iteration. Previously, `Partial` compilation
using warmup iterations would report incorrectly fast numbers, as
shader caching was more likely to happen during warmup. This fix
requires either a rooted device, or using
`profileinstaller:1.3.0-alpha02` in the target app.
PROFILEINSTALLER RELEASE NOTES
Added a hook for benchmarks to drop the shader cache, to ensure
consistent performance for cold startups, especially when compiling
with profiles from warmup iterations. This update is required to
measure cold startups using `benchmark-macro-junit4:1.2.0-alpha05` or
later.
"""
Change-Id: Ia5171b0f40dd8ce6f64f5ccf0a33281a4d8b121e
M benchmark/benchmark-macro/src/androidTest/java/androidx/benchmark/macro/MacrobenchmarkScopeTest.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/Macrobenchmark.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/MacrobenchmarkScope.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/ProfileInstallBroadcast.kt
M profileinstaller/profileinstaller/api/current.txt
M profileinstaller/profileinstaller/api/public_plus_experimental_current.txt
M profileinstaller/profileinstaller/api/restricted_current.txt
A profileinstaller/profileinstaller/src/main/java/androidx/profileinstaller/BenchmarkOperation.java
M profileinstaller/profileinstaller/src/main/java/androidx/profileinstaller/ProfileInstallReceiver.java
M profileinstaller/profileinstaller/src/main/java/androidx/profileinstaller/ProfileInstaller.java
https://android-review.googlesource.com/2262662
Branch: androidx-main
commit 0ba5c3dad9652a85319894f9aa7ad52664be72d2
Author: Chris Craik <ccraik@google.com>
Date: Tue Oct 18 20:56:23 2022
Fix deleteShaderCache by using Profileinstaller or fallback correctly with root
Test: SmallListStartupBenchmark
Test: MacrobenchmarkScopeTest
Fixes: 231455742
Relnote: """
BENCHMARK RELEASE NOTES
Fixed MacrobenchmarkScope.dropShaderCache(). This removes roughly 20ms
of noise from StartupMode.COLD benchmarks, as shaders are now
consistently cleared each iteration. Previously, `Partial` compilation
using warmup iterations would report incorrectly fast numbers, as
shader caching was more likely to happen during warmup. This fix
requires either a rooted device, or using
`profileinstaller:1.3.0-alpha02` in the target app.
PROFILEINSTALLER RELEASE NOTES
Added a hook for benchmarks to drop the shader cache, to ensure
consistent performance for cold startups, especially when compiling
with profiles from warmup iterations. This update is required to
measure cold startups using `benchmark-macro-junit4:1.2.0-alpha05` or
later.
"""
Change-Id: Ia5171b0f40dd8ce6f64f5ccf0a33281a4d8b121e
M benchmark/benchmark-macro/src/androidTest/java/androidx/benchmark/macro/MacrobenchmarkScopeTest.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/Macrobenchmark.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/MacrobenchmarkScope.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/ProfileInstallBroadcast.kt
M profileinstaller/profileinstaller/api/current.txt
M profileinstaller/profileinstaller/api/public_plus_experimental_current.txt
M profileinstaller/profileinstaller/api/restricted_current.txt
A profileinstaller/profileinstaller/src/main/java/androidx/profileinstaller/BenchmarkOperation.java
M profileinstaller/profileinstaller/src/main/java/androidx/profileinstaller/ProfileInstallReceiver.java
M profileinstaller/profileinstaller/src/main/java/androidx/profileinstaller/ProfileInstaller.java
na...@google.com <na...@google.com> #8
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.benchmark:benchmark-macro:1.2.0-alpha07
androidx.profileinstaller:profileinstaller:1.3.0-alpha02
Description
Alert on dashboard:https://androidx-perf.skia.org/t/?begin=1651608277&end=1651608278&subset=all
CLs in build:https://android-build.googleplex.com/builds/jump-to-build/8534762
Caused byhttps://android-review.googlesource.com/c/platform/frameworks/support/+/2086023/ - Improve macrobench iter speed by optimizing shell commands
Regression only affects baseline profile and full compilation variants of cold startup benchmarks.
Need to see if this makes the numbers more or less correct, and why. Triaging to 1.1 for this reason.