Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Migrate vsync_waiter_ios to ARC #52104

Merged
merged 2 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions shell/platform/darwin/ios/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ source_set("flutter_framework_source_arc") {
"framework/Source/KeyCodeMap_Internal.h",
"framework/Source/UIViewController+FlutterScreenAndSceneIfLoaded.h",
"framework/Source/UIViewController+FlutterScreenAndSceneIfLoaded.mm",
"framework/Source/vsync_waiter_ios.h",
"framework/Source/vsync_waiter_ios.mm",
]

frameworks = [
Expand All @@ -90,8 +92,10 @@ source_set("flutter_framework_source_arc") {
]

deps += [
"//flutter/common:common",
"//flutter/lib/ui",
"//flutter/runtime",
"//flutter/shell/common",
"//flutter/shell/platform/embedder:embedder_as_internal_library",
]
}
Expand Down Expand Up @@ -149,8 +153,6 @@ source_set("flutter_framework_source") {
"framework/Source/platform_message_response_darwin.mm",
"framework/Source/profiler_metrics_ios.h",
"framework/Source/profiler_metrics_ios.mm",
"framework/Source/vsync_waiter_ios.h",
"framework/Source/vsync_waiter_ios.mm",
"ios_context.h",
"ios_context.mm",
"ios_context_metal_impeller.h",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ - (instancetype)init {
_availableTextures = [[NSMutableSet alloc] init];

_displayLink = [CADisplayLink displayLinkWithTarget:self selector:@selector(onDisplayLink:)];
[self setMaxRefreshRate:[DisplayLinkManager displayRefreshRate] forceMax:NO];
[self setMaxRefreshRate:DisplayLinkManager.displayRefreshRate forceMax:NO];
[_displayLink addToRunLoop:[NSRunLoop currentRunLoop] forMode:NSRunLoopCommonModes];
[[NSNotificationCenter defaultCenter] addObserver:self
selector:@selector(didEnterBackground:)
Expand Down Expand Up @@ -214,7 +214,7 @@ - (void)onDisplayLink:(CADisplayLink*)link {
if (_displayLinkPauseCountdown == 3) {
_displayLink.paused = YES;
if (_displayLinkForcedMaxRate) {
[self setMaxRefreshRate:[DisplayLinkManager displayRefreshRate] forceMax:NO];
[self setMaxRefreshRate:DisplayLinkManager.displayRefreshRate forceMax:NO];
_displayLinkForcedMaxRate = NO;
}
} else {
Expand Down Expand Up @@ -395,7 +395,7 @@ - (void)presentOnMainThread:(FlutterTexture*)texture {
_didSetContentsDuringThisDisplayLinkPeriod = YES;
} else if (!_displayLinkForcedMaxRate) {
_displayLinkForcedMaxRate = YES;
[self setMaxRefreshRate:[DisplayLinkManager displayRefreshRate] forceMax:YES];
[self setMaxRefreshRate:DisplayLinkManager.displayRefreshRate forceMax:YES];
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1321,7 +1321,7 @@ - (void)createTouchRateCorrectionVSyncClientIfNeeded {
return;
}

double displayRefreshRate = [DisplayLinkManager displayRefreshRate];
double displayRefreshRate = DisplayLinkManager.displayRefreshRate;
const double epsilon = 0.1;
if (displayRefreshRate < 60.0 + epsilon) { // displayRefreshRate <= 60.0

Expand Down
16 changes: 16 additions & 0 deletions shell/platform/darwin/ios/framework/Source/VsyncWaiterIosTest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,20 @@ - (void)testAwaitAndPauseWillWorkCorrectly {
XCTAssertTrue(link.isPaused);
}

- (void)testReleasesLinkOnInvalidation {
__weak CADisplayLink* weakLink;
@autoreleasepool {
auto thread_task_runner = CreateNewThread("VsyncWaiterIosTest");
VSyncClient* vsyncClient = [[VSyncClient alloc]
initWithTaskRunner:thread_task_runner
callback:[](std::unique_ptr<flutter::FrameTimingsRecorder> recorder) {}];

weakLink = [vsyncClient getDisplayLink];
XCTAssertNotNil(weakLink);
[vsyncClient invalidate];
}
// VSyncClient has released the CADisplayLink.
XCTAssertNil(weakLink);
Comment on lines +142 to +143
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't currently pass on main with the current code.

}

@end
14 changes: 5 additions & 9 deletions shell/platform/darwin/ios/framework/Source/vsync_waiter_ios.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
#include <QuartzCore/CADisplayLink.h>

#include "flutter/fml/macros.h"
#include "flutter/fml/memory/weak_ptr.h"
#include "flutter/fml/platform/darwin/scoped_nsobject.h"
#include "flutter/shell/common/variable_refresh_rate_reporter.h"
#include "flutter/shell/common/vsync_waiter.h"

Expand All @@ -31,7 +29,7 @@
///
/// @return The refresh rate in frames per second.
///
+ (double)displayRefreshRate;
@property(class, nonatomic, readonly) double displayRefreshRate;

@end

Expand All @@ -54,10 +52,11 @@

- (void)pause;

//------------------------------------------------------------------------------
/// @brief Call invalidate before releasing this object to remove from runloops.
///
- (void)invalidate;

- (double)getRefreshRate;

- (void)setMaxRefreshRate:(double)refreshRate;

@end
Expand All @@ -73,15 +72,12 @@ class VsyncWaiterIOS final : public VsyncWaiter, public VariableRefreshRateRepor
// |VariableRefreshRateReporter|
double GetRefreshRate() const override;

// Made public for testing.
fml::scoped_nsobject<VSyncClient> GetVsyncClient() const;

// |VsyncWaiter|
// Made public for testing.
void AwaitVSync() override;

private:
fml::scoped_nsobject<VSyncClient> client_;
VSyncClient* client_;
double max_refresh_rate_;

FML_DISALLOW_COPY_AND_ASSIGN(VsyncWaiterIOS);
Expand Down
97 changes: 41 additions & 56 deletions shell/platform/darwin/ios/framework/Source/vsync_waiter_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
#include "flutter/fml/memory/task_runner_checker.h"
#include "flutter/fml/trace_event.h"

@interface VSyncClient ()
@property(nonatomic, assign, readonly) double refreshRate;
@end

// When calculating refresh rate diffrence, anything within 0.1 fps is ignored.
const static double kRefreshRateDiffToIgnore = 0.1;

Expand All @@ -27,63 +31,55 @@
const fml::TimePoint target_time = recorder->GetVsyncTargetTime();
FireCallback(start_time, target_time, true);
};
client_ =
fml::scoped_nsobject{[[VSyncClient alloc] initWithTaskRunner:task_runners_.GetUITaskRunner()
callback:callback]};
max_refresh_rate_ = [DisplayLinkManager displayRefreshRate];
client_ = [[VSyncClient alloc] initWithTaskRunner:task_runners_.GetUITaskRunner()
callback:callback];
max_refresh_rate_ = DisplayLinkManager.displayRefreshRate;
}

VsyncWaiterIOS::~VsyncWaiterIOS() {
// This way, we will get no more callbacks from the display link that holds a weak (non-nilling)
// reference to this C++ object.
[client_.get() invalidate];
[client_ invalidate];
}

void VsyncWaiterIOS::AwaitVSync() {
double new_max_refresh_rate = [DisplayLinkManager displayRefreshRate];
double new_max_refresh_rate = DisplayLinkManager.displayRefreshRate;
if (fabs(new_max_refresh_rate - max_refresh_rate_) > kRefreshRateDiffToIgnore) {
max_refresh_rate_ = new_max_refresh_rate;
[client_.get() setMaxRefreshRate:max_refresh_rate_];
[client_ setMaxRefreshRate:max_refresh_rate_];
}
[client_.get() await];
[client_ await];
}

// |VariableRefreshRateReporter|
double VsyncWaiterIOS::GetRefreshRate() const {
return [client_.get() getRefreshRate];
}

fml::scoped_nsobject<VSyncClient> VsyncWaiterIOS::GetVsyncClient() const {
return client_;
return client_.refreshRate;
}

} // namespace flutter

@implementation VSyncClient {
flutter::VsyncWaiter::Callback callback_;
fml::scoped_nsobject<CADisplayLink> display_link_;
double current_refresh_rate_;
flutter::VsyncWaiter::Callback _callback;
CADisplayLink* _displayLink;
}

- (instancetype)initWithTaskRunner:(fml::RefPtr<fml::TaskRunner>)task_runner
callback:(flutter::VsyncWaiter::Callback)callback {
self = [super init];

if (self) {
current_refresh_rate_ = [DisplayLinkManager displayRefreshRate];
_refreshRate = DisplayLinkManager.displayRefreshRate;
_allowPauseAfterVsync = YES;
callback_ = std::move(callback);
display_link_ = fml::scoped_nsobject<CADisplayLink> {
[[CADisplayLink displayLinkWithTarget:self selector:@selector(onDisplayLink:)] retain]
};
display_link_.get().paused = YES;

[self setMaxRefreshRate:[DisplayLinkManager displayRefreshRate]];

task_runner->PostTask([client = [self retain]]() {
[client->display_link_.get() addToRunLoop:[NSRunLoop currentRunLoop]
forMode:NSRunLoopCommonModes];
[client release];
_callback = std::move(callback);
_displayLink = [CADisplayLink displayLinkWithTarget:self selector:@selector(onDisplayLink:)];
_displayLink.paused = YES;

[self setMaxRefreshRate:DisplayLinkManager.displayRefreshRate];

// Strongly retain the the captured link until it is added to the runloop.
CADisplayLink* localDisplayLink = _displayLink;
task_runner->PostTask([localDisplayLink]() {
[localDisplayLink addToRunLoop:NSRunLoop.currentRunLoop forMode:NSRunLoopCommonModes];
});
}

Expand All @@ -97,19 +93,19 @@ - (void)setMaxRefreshRate:(double)refreshRate {
double maxFrameRate = fmax(refreshRate, 60);
double minFrameRate = fmax(maxFrameRate / 2, 60);
if (@available(iOS 15.0, *)) {
display_link_.get().preferredFrameRateRange =
_displayLink.preferredFrameRateRange =
CAFrameRateRangeMake(minFrameRate, maxFrameRate, maxFrameRate);
} else {
display_link_.get().preferredFramesPerSecond = maxFrameRate;
_displayLink.preferredFramesPerSecond = maxFrameRate;
}
}

- (void)await {
display_link_.get().paused = NO;
_displayLink.paused = NO;
}

- (void)pause {
display_link_.get().paused = YES;
_displayLink.paused = YES;
}

- (void)onDisplayLink:(CADisplayLink*)link {
Expand All @@ -126,44 +122,33 @@ - (void)onDisplayLink:(CADisplayLink*)link {
std::unique_ptr<flutter::FrameTimingsRecorder> recorder =
std::make_unique<flutter::FrameTimingsRecorder>();

current_refresh_rate_ = round(1 / (frame_target_time - frame_start_time).ToSecondsF());
_refreshRate = round(1 / (frame_target_time - frame_start_time).ToSecondsF());

recorder->RecordVsync(frame_start_time, frame_target_time);
if (_allowPauseAfterVsync) {
display_link_.get().paused = YES;
link.paused = YES;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use link passed into onDisplayLink: instead of grabbing it again.

}
callback_(std::move(recorder));
_callback(std::move(recorder));
}

- (void)invalidate {
[display_link_.get() invalidate];
}

- (void)dealloc {
[self invalidate];

[super dealloc];
}

- (double)getRefreshRate {
return current_refresh_rate_;
[_displayLink invalidate];
_displayLink = nil; // Break retain cycle.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test I wrote to validate the display link is released when VSyncClient invalidates and deallocs doesn't pass without this line (and also doesn't pass on main with the current code).

}

- (CADisplayLink*)getDisplayLink {
return display_link_.get();
return _displayLink;
}

@end

@implementation DisplayLinkManager

+ (double)displayRefreshRate {
fml::scoped_nsobject<CADisplayLink> display_link = fml::scoped_nsobject<CADisplayLink> {
[[CADisplayLink displayLinkWithTarget:[[[DisplayLinkManager alloc] init] autorelease]
selector:@selector(onDisplayLink:)] retain]
};
display_link.get().paused = YES;
auto preferredFPS = display_link.get().preferredFramesPerSecond;
CADisplayLink* displayLink = [CADisplayLink displayLinkWithTarget:[[[self class] alloc] init]
selector:@selector(onDisplayLink:)];
displayLink.paused = YES;
auto preferredFPS = displayLink.preferredFramesPerSecond;

// From Docs:
// The default value for preferredFramesPerSecond is 0. When this value is 0, the preferred
Expand All @@ -174,15 +159,15 @@ + (double)displayRefreshRate {
return preferredFPS;
}

return [UIScreen mainScreen].maximumFramesPerSecond;
return UIScreen.mainScreen.maximumFramesPerSecond;
}

- (void)onDisplayLink:(CADisplayLink*)link {
// no-op.
}

+ (BOOL)maxRefreshRateEnabledOnIPhone {
return [[[NSBundle mainBundle] objectForInfoDictionaryKey:@"CADisableMinimumFrameDurationOnPhone"]
return [[NSBundle.mainBundle objectForInfoDictionaryKey:@"CADisableMinimumFrameDurationOnPhone"]
boolValue];
}

Expand Down