From 4a486d74acb5f8f17266d65629aa7f884efb2834 Mon Sep 17 00:00:00 2001 From: Alan Third Date: Sun, 23 Jul 2023 12:00:30 +0100 Subject: [PATCH] Simplify the EmacsLayer double buffering code (bug#63187) * src/nsterm.h (EmacsLayer): Remove cache and replace with two IOSurface variables. * src/nsterm.m (ns_scroll_run): Remove redundant code. ([EmacsView copyRect:to:]): Ensure the context is flushed before we start messign directly with the pixel buffer. ([EmacsLayer initWithColorSpace:]): ([EmacsLayer dealloc]): ([EmacsLayer releaseSurfaces]): ([EmacsLayer checkDimensions]): ([EmacsLayer getContext]): ([EmacsLayer releaseContext]): ([EmacsLayer display]): ([EmacsLayer copyContentsTo:]): Remove cache and replace with two IOSurface variables. --- src/nsterm.h | 3 +- src/nsterm.m | 147 +++++++++++++++++++-------------------------------- 2 files changed, 56 insertions(+), 94 deletions(-) diff --git a/src/nsterm.h b/src/nsterm.h index b6e5a813a6d..22dbf2d8f27 100644 --- a/src/nsterm.h +++ b/src/nsterm.h @@ -742,9 +742,8 @@ #define NSTRACE_UNSILENCE() #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 @interface EmacsLayer : CALayer { - NSMutableArray *cache; CGColorSpaceRef colorSpace; - IOSurfaceRef currentSurface; + IOSurfaceRef frontSurface, backSurface; CGContextRef context; } - (id) initWithColorSpace: (CGColorSpaceRef)cs; diff --git a/src/nsterm.m b/src/nsterm.m index 78089906752..c23238b7dec 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -2704,11 +2704,10 @@ Hide the window (X11 semantics) { NSRect srcRect = NSMakeRect (x, from_y, width, height); NSPoint dest = NSMakePoint (x, to_y); - NSRect destRect = NSMakeRect (x, from_y, width, height); EmacsView *view = FRAME_NS_VIEW (f); [view copyRect:srcRect to:dest]; -#ifdef NS_IMPL_COCOA +#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED < 101400 [view setNeedsDisplayInRect:destRect]; #endif } @@ -8664,8 +8663,10 @@ - (void)copyRect:(NSRect)srcRect to:(NSPoint)dest NSHeight (srcRect)); #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 - double scale = [[self window] backingScaleFactor]; CGContextRef context = [(EmacsLayer *)[self layer] getContext]; + CGContextFlush (context); + + double scale = [[self window] backingScaleFactor]; int bpp = CGBitmapContextGetBitsPerPixel (context) / 8; void *pixels = CGBitmapContextGetData (context); int rowSize = CGBitmapContextGetBytesPerRow (context); @@ -10421,21 +10422,16 @@ @implementation EmacsLayer of the output. To avoid this problem we can check if the surface is "in use", and if it is then avoid using it. Unfortunately to avoid writing to a surface that's in use, but still maintain the - ability to draw to the screen at any time, we need to keep a cache - of multiple surfaces that we can use at will. + ability to draw to the screen at any time, we need to mantain + multiple surfaces that we can use at will. - The EmacsLayer class maintains this cache of surfaces, and - handles the conversion to a CGGraphicsContext that AppKit can use - to draw on. + The EmacsLayer class maintains these surfaces, and handles the + conversion to a CGGraphicsContext that AppKit can use to draw on. - The cache is simple: if a free surface is found it is removed from - the cache and set as the "current" surface. Emacs draws to the - surface and when the layer wants to update the screen we set it's - contents to the surface and then add it back on to the end of the - cache. If no free surfaces are found in the cache then a new one - is created. */ + We simply have a "back" surface, which we can draw to, and a + "front" buffer that is currently being displayed on the screen. */ -#define CACHE_MAX_SIZE 2 +#define EMACSLAYER_DOUBLE_BUFFERED 1 - (id) initWithColorSpace: (CGColorSpaceRef)cs { @@ -10443,14 +10439,9 @@ - (id) initWithColorSpace: (CGColorSpaceRef)cs self = [super init]; if (self) - { - cache = [[NSMutableArray arrayWithCapacity:CACHE_MAX_SIZE] retain]; - [self setColorSpace:cs]; - } + [self setColorSpace:cs]; else - { - return nil; - } + return nil; return self; } @@ -10470,8 +10461,6 @@ - (void) setColorSpace: (CGColorSpaceRef)cs - (void) dealloc { [self releaseSurfaces]; - [cache release]; - [super dealloc]; } @@ -10481,18 +10470,16 @@ - (void) releaseSurfaces [self setContents:nil]; [self releaseContext]; - if (currentSurface) + if (frontSurface) { - CFRelease (currentSurface); - currentSurface = nil; + CFRelease (frontSurface); + frontSurface = nil; } - if (cache) + if (backSurface) { - for (id object in cache) - CFRelease ((IOSurfaceRef)object); - - [cache removeAllObjects]; + CFRelease (backSurface); + backSurface = nil; } } @@ -10503,8 +10490,7 @@ - (BOOL) checkDimensions { int width = NSWidth ([self bounds]) * [self contentsScale]; int height = NSHeight ([self bounds]) * [self contentsScale]; - IOSurfaceRef s = currentSurface ? currentSurface - : (IOSurfaceRef)[cache firstObject]; + IOSurfaceRef s = backSurface ? backSurface : frontSurface; return !s || (IOSurfaceGetWidth (s) == width && IOSurfaceGetHeight (s) == height); @@ -10517,41 +10503,21 @@ - (CGContextRef) getContext CGFloat scale = [self contentsScale]; NSTRACE_WHEN (NSTRACE_GROUP_FOCUS, "[EmacsLayer getContext]"); - NSTRACE_MSG ("IOSurface count: %lu", [cache count] + (currentSurface ? 1 : 0)); if (![self checkDimensions]) [self releaseSurfaces]; if (!context) { - IOSurfaceRef surface = NULL; int width = NSWidth ([self bounds]) * scale; int height = NSHeight ([self bounds]) * scale; - for (id object in cache) - { - if (!IOSurfaceIsInUse ((IOSurfaceRef)object)) - { - surface = (IOSurfaceRef)object; - [cache removeObject:object]; - break; - } - } - - if (!surface && [cache count] >= CACHE_MAX_SIZE) - { - /* Just grab the first one off the cache. This may result - in tearing effects. The alternative is to wait for one - of the surfaces to become free. */ - surface = (IOSurfaceRef)[cache firstObject]; - [cache removeObject:(id)surface]; - } - else if (!surface) + if (!backSurface) { int bytesPerRow = IOSurfaceAlignProperty (kIOSurfaceBytesPerRow, width * 4); - surface = IOSurfaceCreate + backSurface = IOSurfaceCreate ((CFDictionaryRef)@{(id)kIOSurfaceWidth:[NSNumber numberWithInt:width], (id)kIOSurfaceHeight:[NSNumber numberWithInt:height], (id)kIOSurfaceBytesPerRow:[NSNumber numberWithInt:bytesPerRow], @@ -10559,25 +10525,23 @@ - (CGContextRef) getContext (id)kIOSurfacePixelFormat:[NSNumber numberWithUnsignedInt:'BGRA']}); } - if (!surface) + if (!backSurface) { NSLog (@"Failed to create IOSurface for frame %@", [self delegate]); return nil; } - IOReturn lockStatus = IOSurfaceLock (surface, 0, nil); + IOReturn lockStatus = IOSurfaceLock (backSurface, 0, nil); if (lockStatus != kIOReturnSuccess) NSLog (@"Failed to lock surface: %x", lockStatus); - [self copyContentsTo:surface]; + [self copyContentsTo:backSurface]; - currentSurface = surface; - - context = CGBitmapContextCreate (IOSurfaceGetBaseAddress (currentSurface), - IOSurfaceGetWidth (currentSurface), - IOSurfaceGetHeight (currentSurface), + context = CGBitmapContextCreate (IOSurfaceGetBaseAddress (backSurface), + IOSurfaceGetWidth (backSurface), + IOSurfaceGetHeight (backSurface), 8, - IOSurfaceGetBytesPerRow (currentSurface), + IOSurfaceGetBytesPerRow (backSurface), colorSpace, (kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host)); @@ -10585,13 +10549,13 @@ - (CGContextRef) getContext if (!context) { NSLog (@"Failed to create context for frame %@", [self delegate]); - IOSurfaceUnlock (currentSurface, 0, nil); - CFRelease (currentSurface); - currentSurface = nil; + IOSurfaceUnlock (backSurface, 0, nil); + CFRelease (backSurface); + backSurface = nil; return nil; } - CGContextTranslateCTM(context, 0, IOSurfaceGetHeight (currentSurface)); + CGContextTranslateCTM(context, 0, IOSurfaceGetHeight (backSurface)); CGContextScaleCTM(context, scale, -scale); } @@ -10608,10 +10572,11 @@ - (void) releaseContext if (!context) return; + CGContextFlush (context); CGContextRelease (context); context = NULL; - IOReturn lockStatus = IOSurfaceUnlock (currentSurface, 0, nil); + IOReturn lockStatus = IOSurfaceUnlock (backSurface, 0, nil); if (lockStatus != kIOReturnSuccess) NSLog (@"Failed to unlock surface: %x", lockStatus); } @@ -10621,63 +10586,61 @@ - (void) display { NSTRACE_WHEN (NSTRACE_GROUP_FOCUS, "[EmacsLayer display]"); - if (context) + if (context && context != [[NSGraphicsContext currentContext] CGContext]) { [self releaseContext]; -#if CACHE_MAX_SIZE == 1 +#ifdef EMACSLAYER_DOUBLE_BUFFERED + IOSurfaceRef tmp = frontSurface; + frontSurface = backSurface; + backSurface = tmp; + + [self setContents:(id)frontSurface]; +#else /* This forces the layer to see the surface as updated. */ [self setContents:nil]; -#endif - - [self setContents:(id)currentSurface]; - /* Put currentSurface back on the end of the cache. */ - [cache addObject:(id)currentSurface]; - currentSurface = NULL; + /* Since we're not doible buffering, just use the back + surface. */ + [self setContents:(id)backSurface]; - /* Schedule a run of getContext so that if Emacs is idle it will - perform the buffer copy, etc. */ - [self performSelectorOnMainThread:@selector (getContext) - withObject:nil - waitUntilDone:NO]; +#endif } } -/* Copy the contents of lastSurface to DESTINATION. This is required +/* Copy the contents of frontSurface to DESTINATION. This is required every time we want to use an IOSurface as its contents are probably blanks (if it's new), or stale. */ - (void) copyContentsTo: (IOSurfaceRef) destination { IOReturn lockStatus; - IOSurfaceRef source = (IOSurfaceRef)[self contents]; - void *sourceData, *destinationData; + void *frontSurfaceData, *destinationData; int numBytes = IOSurfaceGetAllocSize (destination); NSTRACE_WHEN (NSTRACE_GROUP_FOCUS, "[EmacsLayer copyContentsTo:]"); - if (!source || source == destination) + if (!frontSurface || frontSurface == destination) return; - lockStatus = IOSurfaceLock (source, kIOSurfaceLockReadOnly, nil); + lockStatus = IOSurfaceLock (frontSurface, kIOSurfaceLockReadOnly, nil); if (lockStatus != kIOReturnSuccess) NSLog (@"Failed to lock source surface: %x", lockStatus); - sourceData = IOSurfaceGetBaseAddress (source); + frontSurfaceData = IOSurfaceGetBaseAddress (frontSurface); destinationData = IOSurfaceGetBaseAddress (destination); /* Since every IOSurface should have the exact same settings, a memcpy seems like the fastest way to copy the data from one to the other. */ - memcpy (destinationData, sourceData, numBytes); + memcpy (destinationData, frontSurfaceData, numBytes); - lockStatus = IOSurfaceUnlock (source, kIOSurfaceLockReadOnly, nil); + lockStatus = IOSurfaceUnlock (frontSurface, kIOSurfaceLockReadOnly, nil); if (lockStatus != kIOReturnSuccess) NSLog (@"Failed to unlock source surface: %x", lockStatus); } -#undef CACHE_MAX_SIZE +#undef EMACSLAYER_DOUBLE_BUFFERED @end /* EmacsLayer */ -- 2.40.1