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

[iOS] Fixes DisplayLinkManager leaks #22194

Merged
merged 3 commits into from
Jan 21, 2021
Merged

[iOS] Fixes DisplayLinkManager leaks #22194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9b9f73c
[iOS] Fixes DisplayLinkManager leaks
zhongwuzw Oct 29, 2020
d89ac27
Merge commit '66a6b4a56c7f0b926c0c637b300926f14c91d428' into fix_disp…
zhongwuzw Nov 21, 2020
17cbe33
Remove unnecessary reset
zhongwuzw Nov 21, 2020
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
2 changes: 1 addition & 1 deletion shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ - (BOOL)createShell:(NSString*)entrypoint
}

- (void)initializeDisplays {
double refresh_rate = [[[DisplayLinkManager alloc] init] displayRefreshRate];
double refresh_rate = [DisplayLinkManager displayRefreshRate];
auto display = flutter::Display(refresh_rate);
_shell->OnDisplayUpdates(flutter::DisplayUpdateType::kStartup, {display});
}
Expand Down
4 changes: 1 addition & 3 deletions shell/platform/darwin/ios/framework/Source/vsync_waiter_ios.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@

@interface DisplayLinkManager : NSObject

- (instancetype)init;

//------------------------------------------------------------------------------
/// @brief The display refresh rate used for reporting purposes. The engine does not care
/// about this for frame scheduling. It is only used by tools for instrumentation. The
Expand All @@ -23,7 +21,7 @@
///
/// @return The refresh rate in frames per second.
///
- (double)displayRefreshRate;
+ (double)displayRefreshRate;

@end

Expand Down
32 changes: 8 additions & 24 deletions shell/platform/darwin/ios/framework/Source/vsync_waiter_ios.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,26 +91,16 @@ - (void)dealloc {

@end

@implementation DisplayLinkManager {
fml::scoped_nsobject<CADisplayLink> display_link_;
Copy link
Member

@chinmaygarde chinmaygarde Nov 12, 2020

Choose a reason for hiding this comment

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

You still need to hold an ivar to the display link to invalidate it later. The display link retains the target and without invalidation, the back reference to it will not be cleared.

Copy link
Member Author

@zhongwuzw zhongwuzw Nov 13, 2020

Choose a reason for hiding this comment

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

@chinmaygarde we release the display link https://github.com/flutter/engine/pull/22194/files#diff-ca0ba764dd3a3c6d10dfd05ea010d65973a4882b7f2549352367b97944a1fd8fR105, the reference to DisplayLinkManager would be nilled automatically.

Copy link
Member

@chinmaygarde chinmaygarde Nov 20, 2020

Choose a reason for hiding this comment

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

The reference to the DisplayLinkManager will only be nilled when the display link is unscheduled from the event loop right? That won't happen till you invalidate the link. So it is not entirely automatic.

Copy link
Member Author

@zhongwuzw zhongwuzw Nov 21, 2020

Choose a reason for hiding this comment

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

@chinmaygarde You're right, but we don't schedule the display link to runloop, and we also don't need invalidate it .

}

- (instancetype)init {
self = [super init];

if (self) {
display_link_ = fml::scoped_nsobject<CADisplayLink> {
[[CADisplayLink displayLinkWithTarget:self selector:@selector(onDisplayLink:)] retain]
};
display_link_.get().paused = YES;
}
@implementation DisplayLinkManager

return self;
}

- (double)displayRefreshRate {
+ (double)displayRefreshRate {
if (@available(iOS 10.3, *)) {
auto preferredFPS = display_link_.get().preferredFramesPerSecond; // iOS 10.0
fml::scoped_nsobject<CADisplayLink> display_link = fml::scoped_nsobject<CADisplayLink> {
[[CADisplayLink displayLinkWithTarget:[[DisplayLinkManager new] autorelease]
selector:@selector(onDisplayLink:)] retain]
};
display_link.get().paused = YES;
auto preferredFPS = display_link.get().preferredFramesPerSecond; // iOS 10.0

// From Docs:
// The default value for preferredFramesPerSecond is 0. When this value is 0, the preferred
Expand All @@ -131,10 +121,4 @@ - (void)onDisplayLink:(CADisplayLink*)link {
// no-op.
}

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

[super dealloc];
}

@end