-
-
Notifications
You must be signed in to change notification settings - Fork 341
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
fix: TTFD waits for next drawn frame #3505
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3505 +/- ##
=============================================
+ Coverage 89.197% 89.204% +0.006%
=============================================
Files 528 528
Lines 57431 57466 +35
Branches 20572 20599 +27
=============================================
+ Hits 51227 51262 +35
- Misses 5291 5295 +4
+ Partials 913 909 -4
... and 15 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
45d3ca5 | 1248.27 ms | 1255.48 ms | 7.21 ms |
1437c68 | 1244.86 ms | 1254.18 ms | 9.32 ms |
db31083 | 1252.52 ms | 1266.65 ms | 14.13 ms |
32c4446 | 1225.00 ms | 1231.29 ms | 6.29 ms |
7bb0873 | 1360.94 ms | 1362.24 ms | 1.30 ms |
ad7cec6 | 1203.22 ms | 1224.74 ms | 21.52 ms |
ff09c7e | 1240.94 ms | 1262.66 ms | 21.72 ms |
a176fc4 | 1239.78 ms | 1258.98 ms | 19.20 ms |
3277f18 | 1229.29 ms | 1248.92 ms | 19.63 ms |
8f397a7 | 1251.82 ms | 1268.34 ms | 16.52 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
45d3ca5 | 20.76 KiB | 427.54 KiB | 406.78 KiB |
1437c68 | 22.85 KiB | 410.96 KiB | 388.11 KiB |
db31083 | 22.85 KiB | 407.62 KiB | 384.77 KiB |
32c4446 | 22.84 KiB | 403.24 KiB | 380.39 KiB |
7bb0873 | 22.85 KiB | 407.09 KiB | 384.24 KiB |
ad7cec6 | 20.76 KiB | 427.32 KiB | 406.55 KiB |
ff09c7e | 20.76 KiB | 427.76 KiB | 407.00 KiB |
a176fc4 | 22.84 KiB | 403.24 KiB | 380.39 KiB |
3277f18 | 22.84 KiB | 402.88 KiB | 380.03 KiB |
8f397a7 | 20.76 KiB | 420.55 KiB | 399.79 KiB |
This was always the intention, if its not working properly, I don't think is a breaking change fixing it. I prefer to not add this V2 thing. |
Okay, I didn't know it was the intention. We will add the V2 option, but it's better if we use it sparingly. I will update the PR. |
9fe74c3
to
91a603d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
📜 Description
The SDK waits for the next drawn frame after users call reportFullyDisplayed to reflect when UI changes are visible to the user correctly.
💡 Motivation and Context
Preparation for #3471.
💚 How did you test it?
Unit tests and simulator.
📝 Checklist
You have to check all boxes before merging:
sendDefaultPII
is enabled.🔮 Next steps