[iOS] Migrate VSyncClient and DisplayLinkManager to Swift#186225
[iOS] Migrate VSyncClient and DisplayLinkManager to Swift#186225cbracken wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates the FlutterVSyncClient and FlutterDisplayLinkManager from Objective-C to Swift, introducing VSyncClient.swift and updating associated build files and tests. The review feedback focuses on adhering to the style guide by adding documentation to public members, refining access modifiers for internal properties and methods, and improving the visibility of constants for Objective-C compatibility.
03d3366 to
c4a0bd4
Compare
92e3e6b to
b440121
Compare
| // before that line can be deleted. | ||
| // | ||
| // If we intend to support configurable preferred FPS, then we should provide API for it. We | ||
| // should delete this code either way. |
There was a problem hiding this comment.
Note: this is ported line-for-line from the Obj-C implementation. We should still fix this but it's orthogonal to this change.
| import UIKit | ||
|
|
||
| @objc(FlutterDisplayLinkManager) | ||
| public class DisplayLinkManager: NSObject { |
There was a problem hiding this comment.
Nit: move this to a separate DisplayLinkManager.swift?
| @objc(FlutterDisplayLinkManager) | ||
| public class DisplayLinkManager: NSObject { | ||
|
|
||
| /// Info.plist key enabling the full range of ProMotion refresh rates for CADisplayLink callbacks |
There was a problem hiding this comment.
enabling -> disabling?
|
|
||
| /// Whether the max refresh rate on iPhone ProMotion devices are enabled. This reflects the value | ||
| /// of `CADisableMinimumFrameDurationOnPhone` in the info.plist file. On iPads that support | ||
| /// ProMotion, the max refresh rate is always enabled. |
There was a problem hiding this comment.
is there a way to disable it on ipad, via info.plist?
| ) { | ||
| self.callback = callback | ||
| self.isVariableRefreshRateEnabled = isVariableRefreshRateEnabled | ||
| self.refreshRate = maxRefreshRate |
There was a problem hiding this comment.
is self.refreshRate used?
Migrates FlutterVSyncClient and FlutterDisplayLinkManager from Objective-C to Swift. (void) casts were added to OCMock usages of DisplayLinkManager because by default Swift warns on unused results and overall, that behaviour is better in real production code. Issue: flutter#112232
b440121 to
8b5df75
Compare
|
@cbracken let me know if this is ready for another review |
|
I've rebased on top of my reland of the Obj-C-ified FlutterVSyncClient and cleaned up in #187001 |
Migrates FlutterVSyncClient and FlutterDisplayLinkManager from Objective-C to Swift.
(void) casts were added to OCMock usages of DisplayLinkManager because by default Swift warns on unused results and overall, that behaviour is better in real production code.
Issue: #112232
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.