Skip to content

Commit 859e0c8

Browse files
ychinychin
committed
MMTabline: Use cached images for hover buttons, fix closure memory leaks
MMHoverButton was a bit inefficient in its image management. It always made a new template image which then had to be converted into two other images (image and alternateImage) for every button. Cache at least the template images with weak references so we don't have to keep generating new ones. For now, allow setImage: to create new images but they could be changed to also just use cached images as well. Also, fix memory leaks in the tabs codebase due to improper closure usage in blocks. They were subtly capturing the self pointer which led to the tab line and hover buttons never getting destroyed. Fix to make sure we never accidentally capture self and try to capture as little as possible.
1 parent 426a061 commit 859e0c8

File tree

4 files changed

+66
-28
lines changed

4 files changed

+66
-28
lines changed

‎src/MacVim/MMTabline/MMHoverButton.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@
66

77
@property (nonatomic, retain) NSColor *fgColor;
88

9-
+ (NSImage *)imageNamed:(NSString *)name;
9+
typedef enum : NSUInteger {
10+
MMHoverButtonImageAddTab = 0,
11+
MMHoverButtonImageCloseTab,
12+
MMHoverButtonImageScrollLeft,
13+
MMHoverButtonImageScrollRight,
14+
MMHoverButtonImageCount
15+
} MMHoverButtonImage;
16+
17+
+ (NSImage *)imageFromType:(MMHoverButtonImage)imageType;
1018

1119
@end

‎src/MacVim/MMTabline/MMHoverButton.m‎

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,41 +6,64 @@ @implementation MMHoverButton
66
NSBox *_circle;
77
}
88

9-
+ (NSImage *)imageNamed:(NSString *)name
9+
+ (NSImage *)imageFromType:(MMHoverButtonImage)imageType
1010
{
11-
CGFloat size = [name isEqualToString:@"CloseTabButton"] ? 15 : 17;
12-
return [NSImage imageWithSize:NSMakeSize(size, size) flipped:NO drawingHandler:^BOOL(NSRect dstRect) {
13-
NSBezierPath *p = [NSBezierPath new];
14-
if ([name isEqualToString:@"AddTabButton"]) {
11+
if (imageType >= MMHoverButtonImageCount)
12+
return nil;
13+
14+
CGFloat size = imageType == MMHoverButtonImageCloseTab ? 15 : 17;
15+
16+
static __weak NSImage *imageCache[MMHoverButtonImageCount] = { nil };
17+
if (imageCache[imageType] != nil)
18+
return imageCache[imageType];
19+
20+
BOOL (^drawFuncs[MMHoverButtonImageCount])(NSRect) = {
21+
// AddTab
22+
^BOOL(NSRect dstRect) {
23+
NSBezierPath *p = [NSBezierPath new];
1524
[p moveToPoint:NSMakePoint( 8.5, 4.5)];
1625
[p lineToPoint:NSMakePoint( 8.5, 12.5)];
1726
[p moveToPoint:NSMakePoint( 4.5, 8.5)];
1827
[p lineToPoint:NSMakePoint(12.5, 8.5)];
1928
[p setLineWidth:1.2];
2029
[p stroke];
21-
}
22-
else if ([name isEqualToString:@"CloseTabButton"]) {
30+
return YES;
31+
},
32+
// CloseTab
33+
^BOOL(NSRect dstRect) {
34+
NSBezierPath *p = [NSBezierPath new];
2335
[p moveToPoint:NSMakePoint( 4.5, 4.5)];
2436
[p lineToPoint:NSMakePoint(10.5, 10.5)];
2537
[p moveToPoint:NSMakePoint( 4.5, 10.5)];
2638
[p lineToPoint:NSMakePoint(10.5, 4.5)];
2739
[p setLineWidth:1.2];
2840
[p stroke];
29-
}
30-
else if ([name isEqualToString:@"ScrollLeftButton"]) {
41+
return YES;
42+
},
43+
// ScrollLeft
44+
^BOOL(NSRect dstRect) {
45+
NSBezierPath *p = [NSBezierPath new];
3146
[p moveToPoint:NSMakePoint( 5.0, 8.5)];
3247
[p lineToPoint:NSMakePoint(10.0, 4.5)];
3348
[p lineToPoint:NSMakePoint(10.0, 12.5)];
3449
[p fill];
35-
}
36-
else if ([name isEqualToString:@"ScrollRightButton"]) {
50+
return YES;
51+
},
52+
// ScrollRight
53+
^BOOL(NSRect dstRect) {
54+
NSBezierPath *p = [NSBezierPath new];
3755
[p moveToPoint:NSMakePoint(12.0, 8.5)];
3856
[p lineToPoint:NSMakePoint( 7.0, 4.5)];
3957
[p lineToPoint:NSMakePoint( 7.0, 12.5)];
4058
[p fill];
59+
return YES;
4160
}
42-
return YES;
43-
}];
61+
};
62+
NSImage *img = [NSImage imageWithSize:NSMakeSize(size, size)
63+
flipped:NO
64+
drawingHandler:drawFuncs[imageType]];
65+
imageCache[imageType] = img;
66+
return img;
4467
}
4568

4669
- (instancetype)initWithFrame:(NSRect)frameRect
@@ -70,22 +93,28 @@ - (void)setFgColor:(NSColor *)color
7093
self.image = super.image;
7194
}
7295

73-
- (void)setImage:(NSImage *)image
96+
- (void)setImage:(NSImage *)imageTemplate
7497
{
75-
_circle.cornerRadius = image.size.width / 2.0;
98+
_circle.cornerRadius = imageTemplate.size.width / 2.0;
7699
NSColor *fillColor = self.fgColor ?: NSColor.controlTextColor;
77-
super.image = [NSImage imageWithSize:image.size flipped:NO drawingHandler:^BOOL(NSRect dstRect) {
78-
[image drawInRect:dstRect];
100+
NSImage *image = [NSImage imageWithSize:imageTemplate.size
101+
flipped:NO
102+
drawingHandler:^BOOL(NSRect dstRect) {
103+
[imageTemplate drawInRect:dstRect];
79104
[fillColor set];
80105
NSRectFillUsingOperation(dstRect, NSCompositingOperationSourceAtop);
81106
return YES;
82107
}];
83-
self.alternateImage = [NSImage imageWithSize:image.size flipped:NO drawingHandler:^BOOL(NSRect dstRect) {
108+
NSImage *alternateImage = [NSImage imageWithSize:imageTemplate.size
109+
flipped:NO
110+
drawingHandler:^BOOL(NSRect dstRect) {
84111
[[fillColor colorWithAlphaComponent:0.2] set];
85112
[[NSBezierPath bezierPathWithOvalInRect:dstRect] fill];
86-
[super.image drawInRect:dstRect];
113+
[image drawInRect:dstRect];
87114
return YES;
88115
}];
116+
super.image = image;
117+
self.alternateImage = alternateImage;
89118
}
90119

91120
- (void)setEnabled:(BOOL)enabled

‎src/MacVim/MMTabline/MMTab.m‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ - (instancetype)initWithFrame:(NSRect)frameRect tabline:(MMTabline *)tabline
3939
_tabline = tabline;
4040

4141
_closeButton = [MMHoverButton new];
42-
_closeButton.image = [MMHoverButton imageNamed:@"CloseTabButton"];
42+
_closeButton.image = [MMHoverButton imageFromType:MMHoverButtonImageCloseTab];
4343
_closeButton.target = self;
4444
_closeButton.action = @selector(closeTab:);
4545
_closeButton.translatesAutoresizingMaskIntoConstraints = NO;

‎src/MacVim/MMTabline/MMTabline.m‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
const CGFloat TabOverlap = 6;
1414
const CGFloat ScrollOneTabAllowance = 0.25; // If we are showing 75+% of the tab, consider it to be fully shown when deciding whether to scroll to next tab.
1515

16-
static MMHoverButton* MakeHoverButton(MMTabline *tabline, NSString *imageName, NSString *tooltip, SEL action, BOOL continuous) {
16+
static MMHoverButton* MakeHoverButton(MMTabline *tabline, MMHoverButtonImage imageType, NSString *tooltip, SEL action, BOOL continuous) {
1717
MMHoverButton *button = [MMHoverButton new];
18-
button.image = [MMHoverButton imageNamed:imageName];
18+
button.image = [MMHoverButton imageFromType:imageType];
1919
button.translatesAutoresizingMaskIntoConstraints = NO;
2020
button.target = tabline;
2121
button.action = action;
@@ -81,9 +81,9 @@ - (instancetype)initWithFrame:(NSRect)frameRect
8181
_scrollView.documentView = _tabsContainer;
8282
[self addSubview:_scrollView];
8383

84-
_addTabButton = MakeHoverButton(self, @"AddTabButton", @"New Tab (⌘T)", @selector(addTabAtEnd), NO);
85-
_leftScrollButton = MakeHoverButton(self, @"ScrollLeftButton", @"Scroll Tabs", @selector(scrollLeftOneTab), YES);
86-
_rightScrollButton = MakeHoverButton(self, @"ScrollRightButton", @"Scroll Tabs", @selector(scrollRightOneTab), YES);
84+
_addTabButton = MakeHoverButton(self, MMHoverButtonImageAddTab, @"New Tab (⌘T)", @selector(addTabAtEnd), NO);
85+
_leftScrollButton = MakeHoverButton(self, MMHoverButtonImageScrollLeft, @"Scroll Tabs", @selector(scrollLeftOneTab), YES);
86+
_rightScrollButton = MakeHoverButton(self, MMHoverButtonImageScrollRight, @"Scroll Tabs", @selector(scrollRightOneTab), YES);
8787

8888
[self addConstraints:[NSLayoutConstraint constraintsWithVisualFormat:@"H:[_leftScrollButton][_rightScrollButton]-5-[_scrollView]-5-[_addTabButton]" options:NSLayoutFormatAlignAllCenterY metrics:nil views:NSDictionaryOfVariableBindings(_scrollView, _leftScrollButton, _rightScrollButton, _addTabButton)]];
8989
[self addConstraints:[NSLayoutConstraint constraintsWithVisualFormat:@"V:|[_scrollView]|" options:0 metrics:nil views:@{@"_scrollView":_scrollView}]];
@@ -98,13 +98,14 @@ - (instancetype)initWithFrame:(NSRect)frameRect
9898

9999
// Monitor for scroll wheel events so we can scroll the tabline
100100
// horizontally without the user having to hold down SHIFT.
101+
__weak NSScrollView *weakScrollView = _scrollView;
101102
_scrollWheelEventMonitor = [NSEvent addLocalMonitorForEventsMatchingMask:NSEventMaskScrollWheel handler:^NSEvent * _Nullable(NSEvent * _Nonnull event) {
102-
NSPoint location = [_scrollView convertPoint:event.locationInWindow fromView:nil];
103+
NSPoint location = [weakScrollView convertPoint:event.locationInWindow fromView:nil];
103104
// We want events:
104105
// where the mouse is over the _scrollView
105106
// and where the user is not modifying it with the SHIFT key
106107
// and initiated by the scroll wheel and not the trackpad
107-
if ([_scrollView mouse:location inRect:_scrollView.bounds]
108+
if ([weakScrollView mouse:location inRect:weakScrollView.bounds]
108109
&& !event.modifierFlags
109110
&& !event.hasPreciseScrollingDeltas)
110111
{

0 commit comments

Comments
 (0)