code: drawterm

Info  •  Files  •  Log

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

+++ b/gui-cocoa/screen.m

@@ -16,14 +16,44 @@

 #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,14 +206,7 @@

 	});

 }

-@interface AppDelegate : NSObject <NSApplicationDelegate>

-@end

-@interface AppDelegate ()

-@property (assign) IBOutlet NSWindow *window;

-@property (assign) IBOutlet NSOpenGLView *view;

-@end

-

 @implementation AppDelegate

 void

@@ -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