Subject: bug#21662: 25.0.50; visible-bell causes display artifacts on OS X 10.11

From: Mustafa Kocaturk <[email-masked]>
Date: Thu, 22 Oct 2015 09:41:09 -0500

Implemented a patch with a few small corrections to the proposed solution:
> Create a temporary NSView object and attach it to the selected EmacsView object.

Not temporary, but a shared instance, a singleton.

> This new subview draws itself, starts an NSTimer, with a callback in the 
> current NS run loop. 

Instead of starting a timer, the subview sends itself a message with delay.

> When the timer expires the callback is entered, which updates, hides, and 
> finally removes the subview from its superview.
> The view is garbage collected, all as part of the NS run loop. 

The view is retained for re-use.

> The required change is small: Add a NSView-derived class in nsterm.m and 
> replace old cache/draw/restore code with new temporary local instance of this 
> new class.

As already mentioned, not temporary, but retained.

Patch follows.

diff --git a/src/nsterm.m b/src/nsterm.m
index c4bfd7c..f16ebe2 100644
--- src/nsterm.m~
+++ src/nsterm.m
@@ -987,6 +987,59 @@ ns_clip_to_row (struct window *w, struct glyph_row *row,
 }

+@interface BellView : NSImageView {
+    NSColor *bellColor;
+}
+
+@property(readwrite,copy) NSColor *bellColor;
+
+- (IBAction)ring:(id)sender;
+@end
+
+@implementation BellView
+@synthesize bellColor;
+
+- (id)initWithFrame:(NSRect)frame {
+  self = [super initWithFrame:frame];
+  if (self) { self.image=[NSApp applicationIconImage]; }
+  return self;
+}
+
++ (instancetype)sharedInstance {
+  static id _si=nil;
+  static dispatch_once_t onceT;
+  dispatch_once(&onceT, ^{ _si=[[self alloc] init]; });
+  return _si;
+}
+
+- (BOOL)isOpaque {
+  return NO;
+}
+
+static const CGFloat BELL_T = 0.2;
+
+- (IBAction)ring:(NSView*)nv color:(NSColor*)fg {
+  self.hidden=NO;
+  self.image=[NSImage imageNamed:NSImageNameCaution];
+  NSRect dr=[nv bounds];
+  dr.origin.x+=(dr.size.width-self.image.size.width)/2;
+  dr.origin.y+=(dr.size.height-self.image.size.height)/2;
+  [self setFrameOrigin:dr.origin];
+  [self setFrameSize:self.image.size];
+  /* find a use for color fg */
+  [nv addSubview:self];
+  [self setNeedsDisplay:YES];
+  [self performSelector:@selector(dismiss) withObject:self afterDelay:BELL_T];
+}
+
+- (void)dismiss {
+    self.hidden=YES;
+    [self setNeedsDisplay:YES];
+    [self removeFromSuperview];
+}
+
+@end
+
 static void
 ns_ring_bell (struct frame *f)
 /* --------------------------------------------------------------------------
@@ -996,41 +1049,18 @@ ns_ring_bell (struct frame *f)
   NSTRACE (ns_ring_bell);
   if (visible_bell)
     {
-      NSAutoreleasePool *pool;
       struct frame *frame = SELECTED_FRAME ();
       NSView *view;
 
       block_input ();
-      pool = [[NSAutoreleasePool alloc] init];
 
       view = FRAME_NS_VIEW (frame);
-      if (view != nil)
-        {
-          NSRect r, surr;
-          NSPoint dim = NSMakePoint (128, 128);
-
-          r = [view bounds];
-          r.origin.x += (r.size.width - dim.x) / 2;
-          r.origin.y += (r.size.height - dim.y) / 2;
-          r.size.width = dim.x;
-          r.size.height = dim.y;
-          surr = NSInsetRect (r, -2, -2);
-          ns_focus (frame, &surr, 1);
-          [[view window] cacheImageInRect: [view convertRect: surr toView:nil]];
-          [ns_lookup_indexed_color (NS_FACE_FOREGROUND
-                                      (FRAME_DEFAULT_FACE (frame)), frame) set];
-          NSRectFill (r);
-          [[view window] flushWindow];
-          ns_timeout (150000);
-          [[view window] restoreCachedImage];
-          [[view window] flushWindow];
-          ns_unfocus (frame);
+      if (view != nil) {
+	  [BellView.sharedInstance ring:view color:ns_lookup_indexed_color(NS_FACE_FOREGROUND(FRAME_DEFAULT_FACE (frame)), frame)];
         }
-      [pool release];
+      
       unblock_input ();
-    }
-  else
-    {
+    } else {
       NSBeep ();
     }
 }
diff --git a/src/nsterm.m b/src/nsterm.m
index c4bfd7c..f16ebe2 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -987,6 +987,59 @@ ns_clip_to_row (struct window *w, struct glyph_row *row,
 }

+@interface BellView : NSImageView {
+    NSColor *bellColor;
+}
+
+@property(readwrite,copy) NSColor *bellColor;
+
+- (IBAction)ring:(id)sender;
+@end
+
+@implementation BellView
+@synthesize bellColor;
+
+- (id)initWithFrame:(NSRect)frame {
+  self = [super initWithFrame:frame];
+  if (self) { self.image=[NSApp applicationIconImage]; }
+  return self;
+}
+
++ (instancetype)sharedInstance {
+  static id _si=nil;
+  static dispatch_once_t onceT;
+  dispatch_once(&onceT, ^{ _si=[[self alloc] init]; });
+  return _si;
+}
+
+- (BOOL)isOpaque {
+  return NO;
+}
+
+static const CGFloat BELL_T = 0.2;
+
+- (IBAction)ring:(NSView*)nv color:(NSColor*)fg {
+  self.hidden=NO;
+  self.image=[NSImage imageNamed:NSImageNameCaution];
+  NSRect dr=[nv bounds];
+  dr.origin.x+=(dr.size.width-self.image.size.width)/2;
+  dr.origin.y+=(dr.size.height-self.image.size.height)/2;
+  [self setFrameOrigin:dr.origin];
+  [self setFrameSize:self.image.size];
+  /* find a use for color fg */
+  [nv addSubview:self];
+  [self setNeedsDisplay:YES];
+  [self performSelector:@selector(dismiss) withObject:self afterDelay:BELL_T];
+}
+
+- (void)dismiss {
+    self.hidden=YES;
+    [self setNeedsDisplay:YES];
+    [self removeFromSuperview];
+}
+
+@end
+
 static void
 ns_ring_bell (struct frame *f)
 /* --------------------------------------------------------------------------
@@ -996,41 +1049,18 @@ ns_ring_bell (struct frame *f)
   NSTRACE (ns_ring_bell);
   if (visible_bell)
     {
-      NSAutoreleasePool *pool;
       struct frame *frame = SELECTED_FRAME ();
       NSView *view;
 
       block_input ();
-      pool = [[NSAutoreleasePool alloc] init];
 
       view = FRAME_NS_VIEW (frame);
-      if (view != nil)
-        {
-          NSRect r, surr;
-          NSPoint dim = NSMakePoint (128, 128);
-
-          r = [view bounds];
-          r.origin.x += (r.size.width - dim.x) / 2;
-          r.origin.y += (r.size.height - dim.y) / 2;
-          r.size.width = dim.x;
-          r.size.height = dim.y;
-          surr = NSInsetRect (r, -2, -2);
-          ns_focus (frame, &surr, 1);
-          [[view window] cacheImageInRect: [view convertRect: surr toView:nil]];
-          [ns_lookup_indexed_color (NS_FACE_FOREGROUND
-                                      (FRAME_DEFAULT_FACE (frame)), frame) set];
-          NSRectFill (r);
-          [[view window] flushWindow];
-          ns_timeout (150000);
-          [[view window] restoreCachedImage];
-          [[view window] flushWindow];
-          ns_unfocus (frame);
+      if (view != nil) {
+	  [BellView.sharedInstance ring:view color:ns_lookup_indexed_color(NS_FACE_FOREGROUND(FRAME_DEFAULT_FACE (frame)), frame)];
         }
-      [pool release];
+      
       unblock_input ();
-    }
-  else
-    {
+    } else {
       NSBeep ();
     }
 }

This displays an image named NSImageNameCaution, a small yellow triangle with an exclamation mark, for ~200 ms in the center of the selected frame.
It is possible to choose another image from the workspace, wide open to improvement (e.g., select favorite image by name in lisp ?).
The current foreground color is also sent to the BellView object, but no use case is implemented (ignored) in this patch, also wide open to improvement.

For use as appropriate,
Mustafa