iOS,macOS: migrate shell/gpu to ARC #56157
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
|
test-exempt: code refactor with no semantic change |
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. |
Migrates Objective-C code in shell/gpu to ARC. No changes to tests since this makes no semantic changes. Issue: flutter/flutter#137801
| // Provide accumulated damage to rasterizer (area in current framebuffer that lags behind | ||
| // front buffer) | ||
| uintptr_t texture = reinterpret_cast<uintptr_t>(drawable.texture); | ||
| void* texture = (__bridge void*)drawable.texture; |
There was a problem hiding this comment.
ARC implicit type conversion really doesn't want to allow (__bridge uintptr_t), maybe it's... a bridge too far. Yeah I could reinterpret_cast the void* to a uintptr_t now that the pointer is bridged, but this seems fine.
There was a problem hiding this comment.
Adjusting the types so that we no longer need the reinterpret_casts seems like pure win to me. The fact that it bypassed the compiler warning about needing a bridge is more evidence of how dangerous it is.
| // Provide accumulated damage to rasterizer (area in current framebuffer that lags behind | ||
| // front buffer) | ||
| uintptr_t texture = reinterpret_cast<uintptr_t>(drawable.texture); | ||
| void* texture = (__bridge void*)drawable.texture; |
There was a problem hiding this comment.
Adjusting the types so that we no longer need the reinterpret_casts seems like pure win to me. The fact that it bypassed the compiler warning about needing a bridge is more evidence of how dangerous it is.
cbracken
added
the
autosubmit
…157960) flutter/engine@f2154ef...bb77cf8 2024-10-31 [email protected] iOS: migrate Metal testing types to ARC (flutter/engine#56156) 2024-10-31 [email protected] iOS,macOS: migrate shell/gpu to ARC (flutter/engine#56157) 2024-10-31 [email protected] Roll Skia from 3c628426f85f to 9168ad248c69 (3 revisions) (flutter/engine#56272) 2024-10-31 [email protected] [Impeller] Reland: disable AHBs on devices that were upgraded to 29. (flutter/engine#56221) 2024-10-31 [email protected] Add `timeout` to `local_engine`. (flutter/engine#56271) 2024-10-31 [email protected] Refactor `ArgParser.usage` tests for `BuildPlan` to reduce duplication. (flutter/engine#56254) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Migrates Objective-C code in shell/gpu to ARC. Migrate `sk_cfp::reset(__bridge_retained ptr)` to `sk_cfp::retain(__bridge ptr)`. `reset` `CFRelease`s the previously held pointer and sets the pointer to the new pointer. `retain` `CFRetain`s the new pointer prior to setting it. No changes to tests since this makes no semantic changes. Issue: flutter#137801 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
3 participants
Migrates Objective-C code in shell/gpu to ARC.
Migrate
sk_cfp::reset(__bridge_retained ptr)tosk_cfp::retain(__bridge ptr).resetCFReleases the previously held pointer and sets the pointer to the new pointer.retainCFRetains the new pointer prior to setting it.No changes to tests since this makes no semantic changes.
Issue: flutter/flutter#137801
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.