Repository navigation
fix(annotate): loupe inner image clamped to loupe size, crosshair misaligned with placed point - #556
Merged
Merged
Conversation
…redSize, round offset, explicit Fit Root cause (measured): Modifier.size() inside the 120dp clipped loupe box is clamped to 120dp, so the 3x inner image the offset math assumes never existed. Also: roundToInt offset, explicit contentScale on main image, clear loupe after onTap. Adds LoupeInnerOffsetTest and a Robolectric gesture test. Criteria 0-7
Contributor
JVM Load Benchmark (Desktop)1 regressed · 2 improved · 2 unchanged · 2 new · 0 removed
|
| Benchmark | Base | Head | Change |
|---|---|---|---|
phase1TtiMs |
0 ms |
1 ms |
🔴 n/a (base is 0) |
phase3Ms |
2 ms |
1 ms |
🟢 -50.0% ▰▰▰▰▰ |
jvm/WriteLatency
| Benchmark | Base | Head | Change |
|---|---|---|---|
jankBaselineP95Ms |
37 ms |
32 ms |
🟢 -13.5% ▰▰▱▱▱ |
jankLoadP95Ms |
— | 0 ms |
🆕 new |
jankFactor |
— | 0 x |
🆕 new |
2 unchanged
jvm/GraphLoad
| Benchmark | Base | Head | Change |
|---|---|---|---|
phase2Ms |
0 ms |
0 ms |
⚪ n/a (base is 0) |
totalMs |
2 ms |
2 ms |
⚪ 0.0% ▱▱▱▱▱ |
Environment
| Base | Head | |
|---|---|---|
branch |
main |
HEAD |
gitSha |
9cc34447 |
532b7d9f |
graphConfig |
xlarge |
xlarge |
pageCount |
230 |
230 |
A change counts when it is ≥ 5% and, where the data allows, statistically significant. Judged: 5 by threshold only (point estimates).
Contributor
JVM Load Benchmark: profiling detailsFlamegraphs (this PR)**Allocation** — object allocation pressure (JDBC/SQLite churn)Alloc flamegraph not available CPU — method-level hotspots by on-CPU time CPU flamegraph not available Top allocation hotspots (this PR)`35.5%` byte[]_[k] `6.4%` java.lang.String_[k] `5.9%` int[]_[k] `5.4%` java.lang.Object[]_[k] `4.4%` java.util.LinkedHashMap$Entry_[k]Top CPU hotspots (this PR)`97.5%` /usr/lib/x86_64-linux-gnu/libc.so.6 `0.7%` /tmp/sqlite-3.51.3.0-2df47e86-1cf4-4237-8dac-8e30b6f4d491-libsqlitejdbc.so `0.3%` __libc_pwrite `0.3%` fsync `0.2%` pthread_cond_signal |
Contributor
CI memory (Gradle diff-coverage job, 16 GB runner)2 benchmarks (no base to compare with)
|
| Benchmark | Value | Range |
|---|---|---|
peak_used [max] |
7529 MB |
— |
min_available [min] |
8454 MB |
— |
Environment
| Head | |
|---|---|
sha |
532b7d9fcb336c0423136a6452cd33feb359d93f |
Head-only report: add a base to see changes.
Contributor
Android Load Benchmark (API 30 x86_64 emulator)5 regressed · 3 improved · 1 unchanged · 0 new · 0 removed
|
| Benchmark | Base | Head | Change |
|---|---|---|---|
phase1Ms |
21 ms |
46 ms |
🔴 +119.0% ▰▰▰▰▰ |
phase3Ms |
2546 ms |
4713 ms |
🔴 +85.1% ▰▰▰▰▰ |
android/WriteLatency
| Benchmark | Base | Head | Change |
|---|---|---|---|
baselineP95Ms |
7 ms |
12 ms |
🔴 +71.4% ▰▰▰▰▰ |
jankFactor |
2.57 x |
1.08 x |
🟢 -58.0% ▰▰▰▰▰ |
writes |
14 writes |
22 writes |
🟢 +57.1% ▰▰▰▰▰ |
phase3P95Ms |
18 ms |
13 ms |
🟢 -27.8% ▰▰▰▱▱ |
android/SafIo
| Benchmark | Base | Head | Change |
|---|---|---|---|
safPerFileMs |
0.13 ms |
0.2 ms |
🔴 +53.8% ▰▰▰▰▰ |
safOverhead |
4 x |
6 x |
🔴 +50.0% ▰▰▰▰▰ |
1 unchanged
android/SafIo
| Benchmark | Base | Head | Change |
|---|---|---|---|
safDirectPerFileMs |
0 ms |
0 ms |
⚪ n/a (base is 0) |
Environment
| Base | Head | |
|---|---|---|
branch |
main |
HEAD |
gitSha |
9cc34447 |
532b7d9f |
pageCount |
530 |
530 |
A change counts when it is ≥ 5% and, where the data allows, statistically significant. Judged: 9 by threshold only (point estimates).
diff-coverage gate failed (66% < 80%): the loupeInnerOffset call and requiredSize/offset modifiers in LoupeOverlay only run while the loupe is composed, and no test held a drag. Tag the overlay and assert it appears during a held drag and is removed on release.
Contributor
CI memory (Bazel Android build, 16 GB runner)2 benchmarks (no base to compare with)
|
| Benchmark | Value | Range |
|---|---|---|
peak_used [max] |
11075 MB |
— |
min_available [min] |
4908 MB |
— |
Environment
| Head | |
|---|---|
sha |
532b7d9fcb336c0423136a6452cd33feb359d93f |
Head-only report: add a base to see changes.
tstapler
marked this pull request as ready for review
November 4, 2026 19:50
tstapler
pushed a commit
that referenced
this pull request
Nov 6, 2026
🤖 I have created a release *beep* *boop* --- ## [0.106.1](v0.106.0...v0.106.1) (2026-11-05) ### Bug Fixes * **annotate:** loupe inner image clamped to loupe size, crosshair misaligned with placed point ([#556](#556)) ([0be6a65](0be6a65)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The annotation loupe's crosshair did not match where the point landed on release. Root cause (measured with a probe test): inside the 120dp clipped loupe box,
Modifier.size(canvas × 3)is clamped to 120dp by the parent constraints (size()= 120×120 vsrequiredSize()= 1000×2000), so the 3× inner image the offset math assumes never existed.What Changed
requiredSize.loupeInnerOffset(LoupeGeometry.kt) and usesroundToInt().AsyncImagedeclarescontentScale = ContentScale.Fit.onLoupeOffset(null)now runs afteronTap.LoupeInnerOffsetTest(5),InProgressAnnotationCanvasGestureTest(Robolectric), plus anupdateLabelText/dismiss ViewModel test.Test plan
./gradlew :kmp:testDebugUnitTestfor the loupe, gesture, editor screen, parts, transform and ViewModel test classes: 0 failures