shithub: drawterm

Download patch

ref: c97fe4693f6112504d6f13fab46f7cc8b27685c1
parent: df9c9a8f7c088cb52d5feb0757b51e69bffd380d
author: Sigrid Solveig Haflínudóttir <ftrvxmtrx@gmail.com>
date: Mon Jun 28 15:29:39 EDT 2021

drawterm/gui-cocoa/screen.m: fix SEGFAULT during cursor updates (thanks Igor)

The patch contains the following modifications:

  1. Fix declaration of `currentCursor` declaring it as a
     property of the `DrawtermView` using the `retain`
     attribute. This prevents drawterm from crashing as
     the OSX graphical event loop refrains from deallocating
     it while it is still in use.

  2. Modification of gui-cocoa/screen.m:/^setcursor to use
     'getBitmapDataPlanes' API function instead of
     'static unsigned char data[64]' container.
     (https://developer.apple.com/documentation/appkit/nsbitmapimagerep/1395490-getbitmapdataplanes?language=objc)
     The idea is to avoid using static variables for memory
     managed/supplied by OSX APIs.

  3. Replace 'static GLuint tex;' and declare it as a
     property of the `DrawtermView` as it is part of the
     `DrawtermView`.

  4. Move up declaration of 'DrawtermView' within the file.
     Once the static variables like 'currentCursor'
     and 'tex' have been moved to be properties of the DrawtermView
     this change made sense.

This patch has been tested on OSX Catalina and Big Sur.

--- a/gui-cocoa/screen.m	Sun Jun 20 10:47:03 2021
+++ b/gui-cocoa/screen.m	Mon Jun 28 15:29:39 2021
@@ -16,13 +16,43 @@
 #include "screen.h"
 #include "keyboard.h"
 
-Memimage *gscreen;
+@interface AppDelegate : NSObject <NSApplicationDelegate>
+@end
 
-static NSOpenGLView *myview;
-static NSSize winsize;
-static NSCursor *currentCursor;
+@interface AppDelegate ()
+@property (assign) IBOutlet NSWindow *window;
+@property (assign) IBOutlet NSOpenGLView *view;
+@end
 
-static GLuint tex;
+@interface DrawtermView : NSOpenGLView
+@property (nonatomic, retain) NSCursor *currentCursor;
+@property (nonatomic, assign) GLuint tex;
+
+- (void) drawRect:(NSRect)rect;
+- (void) keyDown:(NSEvent*)event;
+- (void) flagsChanged:(NSEvent*)event;
+- (void) keyUp:(NSEvent*)event;
+- (void) mouseDown:(NSEvent*)event;
+- (void) mouseDragged:(NSEvent*)event;
+- (void) mouseUp:(NSEvent*)event;
+- (void) mouseMoved:(NSEvent*)event;
+- (void) rightMouseDown:(NSEvent*)event;
+- (void) rightMouseDragged:(NSEvent*)event;
+- (void) rightMouseUp:(NSEvent*)event;
+- (void) otherMouseDown:(NSEvent*)event;
+- (void) otherMouseDragged:(NSEvent*)event;
+- (void) otherMouseUp:(NSEvent*)event;
+- (void) scrollWheel:(NSEvent*)event;
+- (BOOL) acceptsFirstResponder;
+- (void) reshape;
+- (BOOL) acceptsMouseMovedEvents;
+- (void) prepareOpenGL;
+- (void) resetCursorRects;
+@end
+
+Memimage *gscreen;
+static DrawtermView *myview;
+static NSSize winsize;
 
 void
 guimain(void)
@@ -102,7 +132,7 @@
 	unloadmemimage(gscreen, r, buf, sz);
 	dispatch_async(dispatch_get_main_queue(), ^(void){
 		[[myview openGLContext] makeCurrentContext];
-		glBindTexture(GL_TEXTURE_2D, tex);
+		glBindTexture(GL_TEXTURE_2D, myview.tex);
 		glTexSubImage2D(GL_TEXTURE_2D, 0, r.min.x, r.min.y, Dx(r), Dy(r), GL_RGBA, GL_UNSIGNED_BYTE, buf);
 		free(buf);
 		[NSOpenGLContext clearCurrentContext];
@@ -123,17 +153,13 @@
 void
 setcursor(void)
 {
-	static unsigned char data[64];
-	unsigned char *planes[2] = {&data[0], &data[32]};
-	int i;
+	NSPoint p;
+	NSBitmapImageRep *r;
+	unsigned char *plane[5];
+	int b;
 
-	lock(&cursor.lk);
-	for(i = 0; i < 32; i++){
-		data[i] = ~cursor.set[i] & cursor.clr[i];
-		data[i+32] = cursor.set[i] | cursor.clr[i];
-	}
-	NSBitmapImageRep *rep = [[NSBitmapImageRep alloc]
-		initWithBitmapDataPlanes:planes
+	r = [[NSBitmapImageRep alloc]
+		initWithBitmapDataPlanes:nil
 		pixelsWide:16
 		pixelsHigh:16
 		bitsPerSample:1
@@ -144,11 +170,22 @@
 		bitmapFormat:0
 		bytesPerRow:2
 		bitsPerPixel:0];
-	NSImage *img = [[NSImage alloc] initWithSize:NSMakeSize(16, 16)];
-	[img addRepresentation:rep];
-	currentCursor = [[NSCursor alloc] initWithImage:img hotSpot:NSMakePoint(-cursor.offset.x, -cursor.offset.y)];
+	[r getBitmapDataPlanes:plane];
+	assert(plane[0]!=nil && plane[1]!=nil);
+
+	lock(&cursor.lk);
+	for(b=0; b < nelem(cursor.set); b++){
+		plane[0][b] = ~cursor.set[b] & cursor.clr[b];
+		plane[1][b] =  cursor.set[b] | cursor.clr[b];
+	}
+	p = NSMakePoint(-cursor.offset.x, -cursor.offset.y);
 	unlock(&cursor.lk);
 
+	NSImage *i = [[NSImage alloc] initWithSize:NSMakeSize(16, 16)];
+	[i addRepresentation:r];
+
+	myview.currentCursor = [[NSCursor alloc] initWithImage:i hotSpot:p];
+
 	dispatch_async(dispatch_get_main_queue(), ^(void){
 		[[myview window] invalidateCursorRectsForView:myview];
 	});
@@ -169,13 +206,6 @@
 	});
 }
 
-@interface AppDelegate : NSObject <NSApplicationDelegate>
-@end
-
-@interface AppDelegate ()
-@property (assign) IBOutlet NSWindow *window;
-@property (assign) IBOutlet NSOpenGLView *view;
-@end
 
 @implementation AppDelegate
 
@@ -204,34 +234,12 @@
 
 @end
 
-@interface DrawtermView : NSOpenGLView
-- (void) drawRect:(NSRect)rect;
-- (void) keyDown:(NSEvent*)event;
-- (void) flagsChanged:(NSEvent*)event;
-- (void) keyUp:(NSEvent*)event;
-- (void) mouseDown:(NSEvent*)event;
-- (void) mouseDragged:(NSEvent*)event;
-- (void) mouseUp:(NSEvent*)event;
-- (void) mouseMoved:(NSEvent*)event;
-- (void) rightMouseDown:(NSEvent*)event;
-- (void) rightMouseDragged:(NSEvent*)event;
-- (void) rightMouseUp:(NSEvent*)event;
-- (void) otherMouseDown:(NSEvent*)event;
-- (void) otherMouseDragged:(NSEvent*)event;
-- (void) otherMouseUp:(NSEvent*)event;
-- (void) scrollWheel:(NSEvent*)event;                                                                                                                                                                                                                                                                                                                                                                                                   
-- (BOOL) acceptsFirstResponder;
-- (void) reshape;
-- (BOOL) acceptsMouseMovedEvents;
-- (void) prepareOpenGL;
-- (void) resetCursorRects;
-@end
 
 @implementation DrawtermView
 
 - (void) prepareOpenGL {
-	glGenTextures(1, &tex);
-	glBindTexture(GL_TEXTURE_2D, tex);
+	glGenTextures(1, &_tex);
+	glBindTexture(GL_TEXTURE_2D, _tex);
 	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
 	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
 	glEnable(GL_TEXTURE_2D);
@@ -244,7 +252,7 @@
 
 - (void) drawRect:(NSRect)rect
 {
-	glBindTexture(GL_TEXTURE_2D, tex);
+	glBindTexture(GL_TEXTURE_2D, _tex);
 	glClearColor(0, 0, 0, 0);
 	glClear(GL_COLOR_BUFFER_BIT);
 	glColor4f(1.0, 1.0, 1.0, 1.0);
@@ -456,10 +464,11 @@
 }
 
 - (void) reshape {
+	[super reshape];
 	winsize = self.frame.size;
 	NSOpenGLContext *ctxt = [NSOpenGLContext currentContext];
 	[[myview openGLContext] makeCurrentContext];
-	glBindTexture(GL_TEXTURE_2D, tex);
+	glBindTexture(GL_TEXTURE_2D, _tex);
 	glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, self.frame.size.width, self.frame.size.height, 0, GL_RGBA, GL_UNSIGNED_BYTE, NULL);
 	if(ctxt == nil)
 		[NSOpenGLContext clearCurrentContext];
@@ -477,6 +486,7 @@
 
 - (void)resetCursorRects {
 	[super resetCursorRects];
-	[self addCursorRect:self.bounds cursor:currentCursor];
+	if (self.currentCursor != nil)
+		[self addCursorRect:self.bounds cursor:self.currentCursor];
 }
 @end