Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ensure repaint on replay screenshot capture #2527

Merged
merged 4 commits into from
Dec 20, 2024

Conversation

vaind
Copy link
Collaborator

@vaind vaind commented Dec 19, 2024

📜 Description

  • on iOS - ensure the screen is painted and run the capture after a frame (this was actually a bug because we could run it during widget build process)
  • on Android - Previously, we only captured a new screenshot when there was a paint after 1 second after the previous one. So if there were frames just after the previous one we captured (like during an animation, transition, etc) and the screen "settled" in in less than a second since our last capture, we're not going to capture the new state at all. I went for the simpler route of forcing a frame but if we want, the alternative would be to register a persistent frame listener and keep the information that there was a change between the last screenshot and "now", when the time for a new screenshot comes.

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

Copy link
Contributor

github-actions bot commented Dec 19, 2024

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against c141e10

Copy link

codecov bot commented Dec 19, 2024

Codecov Report

Attention: Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.81%. Comparing base (19a9adb) to head (c141e10).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...tter/lib/src/native/cocoa/sentry_native_cocoa.dart 87.50% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2527      +/-   ##
==========================================
+ Coverage   87.13%   91.81%   +4.68%     
==========================================
  Files         265       88     -177     
  Lines        9429     3068    -6361     
==========================================
- Hits         8216     2817    -5399     
+ Misses       1213      251     -962     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Dec 19, 2024

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1256.69 ms 1280.59 ms 23.90 ms
Size 8.42 MiB 9.86 MiB 1.44 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
683fd34 1239.83 ms 1259.08 ms 19.25 ms
bd37365 1236.22 ms 1243.27 ms 7.04 ms
202b83f 1206.16 ms 1225.33 ms 19.16 ms
d4d0807 1246.94 ms 1260.69 ms 13.75 ms
0bed04d 1235.57 ms 1251.02 ms 15.45 ms
5603ab2 1268.47 ms 1280.73 ms 12.26 ms
9d7e862 1236.88 ms 1258.62 ms 21.74 ms
4dc231b 1232.12 ms 1263.50 ms 31.38 ms
6aab859 1245.14 ms 1247.59 ms 2.45 ms
be8cafe 1239.94 ms 1266.20 ms 26.27 ms

App size

Revision Plain With Sentry Diff
683fd34 8.28 MiB 9.34 MiB 1.06 MiB
bd37365 8.28 MiB 9.34 MiB 1.06 MiB
202b83f 8.33 MiB 9.40 MiB 1.07 MiB
d4d0807 8.33 MiB 9.64 MiB 1.31 MiB
0bed04d 8.32 MiB 9.37 MiB 1.05 MiB
5603ab2 8.15 MiB 9.12 MiB 990.57 KiB
9d7e862 8.32 MiB 9.38 MiB 1.05 MiB
4dc231b 8.38 MiB 9.77 MiB 1.40 MiB
6aab859 8.29 MiB 9.36 MiB 1.07 MiB
be8cafe 8.33 MiB 9.40 MiB 1.07 MiB

Previous results on branch: feat/capture-replay-regardless-of-paints

Startup times

Revision Plain With Sentry Diff
01b6cd2 1247.55 ms 1262.73 ms 15.18 ms
b32691d 1246.86 ms 1270.83 ms 23.98 ms

App size

Revision Plain With Sentry Diff
01b6cd2 8.42 MiB 9.86 MiB 1.44 MiB
b32691d 8.42 MiB 9.86 MiB 1.44 MiB

Copy link
Contributor

github-actions bot commented Dec 19, 2024

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 455.70 ms 546.83 ms 91.13 ms
Size 6.46 MiB 7.48 MiB 1.02 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
1cdcacf 389.94 ms 463.53 ms 73.59 ms
be08ed1 361.37 ms 414.84 ms 53.47 ms
3ad66e4 406.90 ms 481.48 ms 74.58 ms
ad69abc 297.35 ms 385.89 ms 88.54 ms
ef6466d 373.96 ms 443.17 ms 69.21 ms
e893df5 310.60 ms 380.58 ms 69.98 ms
f25f207 341.40 ms 427.73 ms 86.34 ms
61e71ec 343.94 ms 410.59 ms 66.66 ms
1131914 317.90 ms 361.58 ms 43.69 ms
fe4aa56 356.06 ms 428.67 ms 72.61 ms

App size

Revision Plain With Sentry Diff
1cdcacf 6.33 MiB 7.26 MiB 949.77 KiB
be08ed1 6.33 MiB 7.26 MiB 946.42 KiB
3ad66e4 6.33 MiB 7.26 MiB 947.04 KiB
ad69abc 6.06 MiB 7.09 MiB 1.03 MiB
ef6466d 6.34 MiB 7.28 MiB 967.79 KiB
e893df5 6.06 MiB 7.09 MiB 1.03 MiB
f25f207 6.33 MiB 7.26 MiB 943.41 KiB
61e71ec 6.34 MiB 7.28 MiB 966.26 KiB
1131914 5.94 MiB 6.96 MiB 1.02 MiB
fe4aa56 6.06 MiB 7.10 MiB 1.04 MiB

Previous results on branch: feat/capture-replay-regardless-of-paints

Startup times

Revision Plain With Sentry Diff
b32691d 466.84 ms 578.08 ms 111.24 ms

App size

Revision Plain With Sentry Diff
b32691d 6.46 MiB 7.48 MiB 1.02 MiB

@vaind vaind marked this pull request as ready for review December 19, 2024 22:39
@vaind vaind merged commit c689845 into main Dec 20, 2024
50 of 51 checks passed
@vaind vaind deleted the feat/capture-replay-regardless-of-paints branch December 20, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants