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

experiment: mask CustomPaint widget #2346

Closed
wants to merge 1 commit into from
Closed

Conversation

vaind
Copy link
Collaborator

@vaind vaind commented Oct 10, 2024

📜 Description

Masking CustomPaint which is used to render custom graphics. The problem with the solution in this PR is that this widget is used for builtin widgets too, such us buttons, ...

As such, I don't think we can reliably filter out only user-defined CustomPaint widgets. Therefore, users should add custom masking using #2324

💡 Motivation and Context

Part of #1193 (comment)

💚 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

Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- mask CustomPaint widget ([#2346](https://github.com/getsentry/sentry-dart/pull/2346))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against de2b3df

Copy link

codecov bot commented Oct 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.31%. Comparing base (e239c83) to head (de2b3df).
Report is 4 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (e239c83) and HEAD (de2b3df). Click for more details.

HEAD has 7 uploads less than BASE
Flag BASE (e239c83) HEAD (de2b3df)
9 2
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2346       +/-   ##
===========================================
- Coverage   84.77%   63.31%   -21.47%     
===========================================
  Files         253        4      -249     
  Lines        9060      169     -8891     
===========================================
- Hits         7681      107     -7574     
+ Misses       1379       62     -1317     

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

Copy link
Contributor

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 457.90 ms 509.27 ms 51.37 ms
Size 6.49 MiB 7.56 MiB 1.07 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
0aaa46e 313.39 ms 373.23 ms 59.85 ms
4656f10 626.98 ms 693.36 ms 66.38 ms
2d74010 400.42 ms 466.50 ms 66.08 ms
f7f46dc 306.38 ms 363.32 ms 56.95 ms
3751dbc 486.20 ms 538.68 ms 52.48 ms
bffc2c5 348.00 ms 399.89 ms 51.89 ms
0bed04d 382.15 ms 458.33 ms 76.18 ms
cc80714 333.26 ms 388.80 ms 55.54 ms
870f5eb 329.45 ms 369.29 ms 39.84 ms
2f8f173 323.31 ms 373.29 ms 49.97 ms

App size

Revision Plain With Sentry Diff
0aaa46e 6.16 MiB 7.14 MiB 1007.32 KiB
4656f10 6.33 MiB 7.30 MiB 988.47 KiB
2d74010 6.33 MiB 7.26 MiB 943.41 KiB
f7f46dc 6.06 MiB 7.03 MiB 993.54 KiB
3751dbc 6.49 MiB 7.55 MiB 1.06 MiB
bffc2c5 6.34 MiB 7.28 MiB 966.31 KiB
0bed04d 6.33 MiB 7.30 MiB 987.71 KiB
cc80714 6.34 MiB 7.28 MiB 966.31 KiB
870f5eb 5.94 MiB 6.92 MiB 1005.77 KiB
2f8f173 5.94 MiB 6.95 MiB 1.01 MiB

Copy link
Contributor

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1243.35 ms 1263.98 ms 20.63 ms
Size 8.38 MiB 9.74 MiB 1.36 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
5f2f77b 1231.76 ms 1248.43 ms 16.67 ms
a510d1d 1277.04 ms 1291.57 ms 14.53 ms
3751dbc 1261.45 ms 1287.79 ms 26.34 ms
8a111a9 1251.49 ms 1270.77 ms 19.28 ms
f25f207 1233.44 ms 1260.73 ms 27.30 ms
04bd9e6 1230.78 ms 1250.71 ms 19.94 ms
633cf2e 1257.96 ms 1275.73 ms 17.77 ms
d783424 1265.71 ms 1277.00 ms 11.29 ms
e3ef570 1220.08 ms 1239.66 ms 19.58 ms
f9d18f3 1240.20 ms 1242.78 ms 2.57 ms

App size

Revision Plain With Sentry Diff
5f2f77b 8.33 MiB 9.64 MiB 1.31 MiB
a510d1d 8.16 MiB 9.17 MiB 1.01 MiB
3751dbc 8.38 MiB 9.73 MiB 1.35 MiB
8a111a9 8.38 MiB 9.74 MiB 1.36 MiB
f25f207 8.32 MiB 9.38 MiB 1.05 MiB
04bd9e6 8.33 MiB 9.61 MiB 1.27 MiB
633cf2e 8.15 MiB 9.12 MiB 986.26 KiB
d783424 8.29 MiB 9.38 MiB 1.09 MiB
e3ef570 8.32 MiB 9.38 MiB 1.06 MiB
f9d18f3 8.29 MiB 9.36 MiB 1.07 MiB

@vaind vaind closed this Oct 14, 2024
@vaind vaind reopened this Oct 14, 2024
@vaind vaind closed this Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant