From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Alan Third Newsgroups: gmane.emacs.bugs Subject: bug#65817: 30.0.50; Abort with NSInvalidArgumentException on macOS Big Sur Date: Fri, 8 Sep 2023 19:53:50 +0100 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="m9KWJ/KQ+eywibp3" Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="7345"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 65817@debbugs.gnu.org To: Gerd =?UTF-8?Q?M=C3=B6llmann?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Sep 08 20:55:14 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qegdN-0001f1-TX for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 08 Sep 2023 20:55:14 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qegdC-0002Cl-GJ; Fri, 08 Sep 2023 14:55:02 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qegdA-00025h-83 for bug-gnu-emacs@gnu.org; Fri, 08 Sep 2023 14:55:00 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qegd9-0005OF-Su for bug-gnu-emacs@gnu.org; Fri, 08 Sep 2023 14:54:59 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qegdC-0002CR-10 for bug-gnu-emacs@gnu.org; Fri, 08 Sep 2023 14:55:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Alan Third Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 08 Sep 2023 18:55:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 65817 X-GNU-PR-Package: emacs Original-Received: via spool by 65817-submit@debbugs.gnu.org id=B65817.16941992488391 (code B ref 65817); Fri, 08 Sep 2023 18:55:01 +0000 Original-Received: (at 65817) by debbugs.gnu.org; 8 Sep 2023 18:54:08 +0000 Original-Received: from localhost ([127.0.0.1]:45607 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qegcJ-0002BG-HJ for submit@debbugs.gnu.org; Fri, 08 Sep 2023 14:54:08 -0400 Original-Received: from dane.soverin.net ([2a10:de80:1:4091:b9e9:2218:0:1]:59451) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qegcF-0002Ah-BA for 65817@debbugs.gnu.org; Fri, 08 Sep 2023 14:54:06 -0400 Original-Received: from smtp.soverin.net (c04smtp-lb01.int.sover.in [10.10.4.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by dane.soverin.net (Postfix) with ESMTPS id 4Rj4yS6nGnz10J2; Fri, 8 Sep 2023 18:53:52 +0000 (UTC) Original-Received: from smtp.soverin.net (smtp.soverin.net [10.10.4.99]) by soverin.net (Postfix) with ESMTPSA id 4Rj4yR6gBLzKx; Fri, 8 Sep 2023 18:53:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=idiocy.org; s=soverin; t=1694199232; bh=A50dIKcOcGnoPtZB8ALNxjgAaIQa0+aj7AxC3uIOydQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=h81D1I7TRFwy/RgG7IuZD/MG6d4NQxxbSUFgcqFjoR4mDBNqCm0HAfXDbiMxCFZWk QjW96goEFj62IL+cBHb9nYo7suOUerxSoVS1MZe/yFG9F6n6tXOoYzybz5t5TD4uKQ cU0bsAgpFPNp7XH0mUb7ohG/4UE0kWyOXdhHPE+MYt404Svk6ZzBUlknT8sawrwVcY P1oq54RgzihegZCIX+xDB9KpWYFsa3EDFpXWT8FohLiuwT7injWQ+NLuZaLMRBe1/P dTeB3MrKqoF26QPFBdC330ckJUMxfQYQS9cfI37rs3kWuD2BxtYGYZO3coF7bZ4SbH ivAcu/gpuBLQA== Original-Received: from alan by faroe.holly.idiocy.org with local (Exim 4.96) (envelope-from ) id 1qegc3-001OCJ-03; Fri, 08 Sep 2023 19:53:51 +0100 X-Soverin-Authenticated: true Mail-Followup-To: Alan Third , Gerd =?UTF-8?Q?M=C3=B6llmann?= , 65817@debbugs.gnu.org Content-Disposition: inline In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:269815 Archived-At: --m9KWJ/KQ+eywibp3 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Fri, Sep 08, 2023 at 09:01:06AM +0200, Gerd Möllmann wrote: > System Description: macOS 11.7.9 > > This is a 10 year old Mac. I can't reproduce it at will. Below is what > was printed to the terminal, then Emacs aborted. This is odd. The system is trying to draw the frame to the screen *long* before we've finished creating it. I don't know why this would be happening, it's possible that we've hit an edge case where we accidentally mark the view for display early, but I don't know how we would be doing that. > 5 emacs 0x000000010033d481 -[EmacsView lockFocus] + 49 > 6 emacs 0x000000010032ee27 ns_focus + 87 > 7 emacs 0x000000010033df80 ns_clear_frame_area + 400 > 8 emacs 0x000000010033dda1 -[EmacsView drawRect:] + 321 > 21 AppKit 0x00007fff2314c06f -[NSWindow addChildWindow:ordered:] + 640 > 22 emacs 0x000000010033fe08 -[EmacsWindow setParentChildRelationships] + 696 > 23 emacs 0x000000010033f2ad -[EmacsWindow initWithEmacsFrame:fullscreen:screen:] + 1485 > 24 emacs 0x000000010033ecd0 -[EmacsWindow initWithEmacsFrame:] + 48 > 25 emacs 0x000000010033ace5 -[EmacsView initFrameFromEmacs:] + 1045 > 26 emacs 0x00000001003579a1 Fx_create_frame + 7937 At the point lockFocus is called here, we haven't yet set up the layer we want to draw into. I can't find any indication that addChildWindow may trigger a display, but this is Apple's documentation I'm talking about... I've attached a patch that reorders some of the initialisation, hopefully avoiding this problem. I've also attached another patch that you may need to apply first to make the second one apply cleanly. -- Alan Third --m9KWJ/KQ+eywibp3 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0001-Simplify-the-EmacsLayer-double-buffering-code-bug.patch" >From 72b13fa67981c40f0e3c63092aa796525fe61344 Mon Sep 17 00:00:00 2001 From: Alan Third Date: Sun, 23 Jul 2023 12:00:30 +0100 Subject: [PATCH v2] Simplify the EmacsLayer double buffering code (bug#63187) --- src/nsfns.m | 30 +++++++++++++++++++++++++++++- src/nsterm.h | 13 ++++++++++++- src/nsterm.m | 50 ++++++++++++++++++++++++++------------------------ 3 files changed, 67 insertions(+), 26 deletions(-) diff --git a/src/nsfns.m b/src/nsfns.m index a79892f73b6..082e06698b2 100644 --- a/src/nsfns.m +++ b/src/nsfns.m @@ -799,6 +799,26 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side. } } +static void +ns_set_inhibit_double_buffering (struct frame *f, + Lisp_Object new_value, + Lisp_Object old_value) +{ +#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 + if (!EQ (new_value, old_value)) + { + FRAME_DOUBLE_BUFFERED (f) = NILP (new_value); + + /* If the view or layer haven't been created yet this will be a + noop. */ + [(EmacsLayer *)[FRAME_NS_VIEW (f) layer] + setDoubleBuffered:FRAME_DOUBLE_BUFFERED (f)]; + + SET_FRAME_GARBAGED (f); + } +#endif +} + static void ns_set_internal_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval) { @@ -1073,7 +1093,7 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side. gui_set_alpha, 0, /* x_set_sticky */ ns_set_tool_bar_position, - 0, /* x_set_inhibit_double_buffering */ + ns_set_inhibit_double_buffering, ns_set_undecorated, ns_set_parent_frame, 0, /* x_set_skip_taskbar */ @@ -1461,6 +1481,14 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side. gui_default_parameter (f, parms, Qtitle, Qnil, "title", "Title", RES_TYPE_STRING); +#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 + tem = gui_display_get_arg (dpyinfo, parms, Qinhibit_double_buffering, NULL, NULL, + RES_TYPE_BOOLEAN); + FRAME_DOUBLE_BUFFERED (f) = NILP (tem) || EQ (tem, Qunbound); + store_frame_param (f, Qinhibit_double_buffering, + FRAME_DOUBLE_BUFFERED (f) ? Qnil : Qt); +#endif + parms = get_geometry_from_preferences (dpyinfo, parms); window_prompting = gui_figure_window_size (f, parms, false, true); diff --git a/src/nsterm.h b/src/nsterm.h index b6e5a813a6d..8d6c58290cc 100644 --- a/src/nsterm.h +++ b/src/nsterm.h @@ -746,9 +746,11 @@ #define NSTRACE_UNSILENCE() CGColorSpaceRef colorSpace; IOSurfaceRef currentSurface; CGContextRef context; + bool doubleBuffered; } -- (id) initWithColorSpace: (CGColorSpaceRef)cs; +- (id) initWithColorSpace: (CGColorSpaceRef)cs doubleBuffered: (bool)db; - (void) setColorSpace: (CGColorSpaceRef)cs; +- (void) setDoubleBuffered: (bool)db; - (CGContextRef) getContext; @end #endif @@ -996,6 +998,11 @@ #define KEY_NS_SHOW_PREFS ((1<<28)|(0<<16)|14) /* Non-zero if we are doing an animation, e.g. toggling the tool bar. */ int in_animation; +#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 + /* Is the frame double buffered? */ + bool double_buffered; +#endif + #ifdef NS_IMPL_GNUSTEP /* Zero if this is the first time a toolbar has been updated on this frame. */ @@ -1030,6 +1037,10 @@ #define FRAME_POINTER_TYPE(f) ((f)->output_data.ns->current_pointer) #define FRAME_FONT(f) ((f)->output_data.ns->font) +#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 +#define FRAME_DOUBLE_BUFFERED(f) ((f)->output_data.ns->double_buffered) +#endif + #ifdef __OBJC__ #define XNS_SCROLL_BAR(vec) ((id) xmint_pointer (vec)) #else diff --git a/src/nsterm.m b/src/nsterm.m index 78089906752..28502ad1a2a 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 } @@ -8607,7 +8606,8 @@ - (instancetype)toggleToolbar: (id)sender - (CALayer *)makeBackingLayer { EmacsLayer *l = [[EmacsLayer alloc] - initWithColorSpace:[[[self window] colorSpace] CGColorSpace]]; + initWithColorSpace:[[[self window] colorSpace] CGColorSpace] + doubleBuffered:FRAME_DOUBLE_BUFFERED (emacsframe)]; [l setDelegate:(id)self]; [l setContentsScale:[[self window] backingScaleFactor]]; @@ -8664,8 +8664,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); @@ -10435,22 +10437,20 @@ @implementation EmacsLayer cache. If no free surfaces are found in the cache then a new one is created. */ -#define CACHE_MAX_SIZE 2 - - (id) initWithColorSpace: (CGColorSpaceRef)cs + doubleBuffered: (bool)db { - NSTRACE ("[EmacsLayer initWithColorSpace:]"); + NSTRACE ("[EmacsLayer initWithColorSpace:doubleBuffered:]"); self = [super init]; if (self) { - cache = [[NSMutableArray arrayWithCapacity:CACHE_MAX_SIZE] retain]; [self setColorSpace:cs]; + [self setDoubleBuffered:db]; + cache = [[NSMutableArray arrayWithCapacity:(doubleBuffered ? 2 : 1)] retain]; } else - { - return nil; - } + return nil; return self; } @@ -10467,6 +10467,15 @@ - (void) setColorSpace: (CGColorSpaceRef)cs } +- (void) setDoubleBuffered: (bool)db +{ + if (doubleBuffered != db) + [self releaseSurfaces]; + + doubleBuffered = db; +} + + - (void) dealloc { [self releaseSurfaces]; @@ -10538,7 +10547,7 @@ - (CGContextRef) getContext } } - if (!surface && [cache count] >= CACHE_MAX_SIZE) + if (!surface && [cache count] >= (doubleBuffered ? 2 : 1)) { /* Just grab the first one off the cache. This may result in tearing effects. The alternative is to wait for one @@ -10591,7 +10600,7 @@ - (CGContextRef) getContext return nil; } - CGContextTranslateCTM(context, 0, IOSurfaceGetHeight (currentSurface)); + CGContextTranslateCTM(context, 0, IOSurfaceGetHeight (surface)); CGContextScaleCTM(context, scale, -scale); } @@ -10608,6 +10617,7 @@ - (void) releaseContext if (!context) return; + CGContextFlush (context); CGContextRelease (context); context = NULL; @@ -10621,26 +10631,18 @@ - (void) display { NSTRACE_WHEN (NSTRACE_GROUP_FOCUS, "[EmacsLayer display]"); - if (context) + if (context && context != [[NSGraphicsContext currentContext] CGContext]) { [self releaseContext]; -#if CACHE_MAX_SIZE == 1 - /* This forces the layer to see the surface as updated. */ + /* This forces the layer to see the surface as updated even if + we replace it with itself. */ [self setContents:nil]; -#endif - [self setContents:(id)currentSurface]; /* Put currentSurface back on the end of the cache. */ [cache addObject:(id)currentSurface]; currentSurface = NULL; - - /* 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]; } } -- 2.40.1 --m9KWJ/KQ+eywibp3 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-Fix-crash-on-child-frame-creation-bug-65817.patch" >From 893f079f51c3bc81d8719c48fe539f72b1025fb8 Mon Sep 17 00:00:00 2001 From: Alan Third Date: Fri, 8 Sep 2023 19:33:06 +0100 Subject: [PATCH] Fix crash on child frame creation (bug#65817) * src/nsterm.m ([EmacsView initFrameFromEmacs:]): Reorder the way the frame and layers are created. ([EmacsView makeBackingLayer]): Change to the newly renamed method below. ([EmacsLayer initWithColorSpace:doubleBuffered:]): ([EmacsLayer initWithDoubleBuffered:]): Rename the method and remove the colorspace argument as it's no longer able to be set on initial creation. * src/nsterm.h: Use new method prototype. --- src/nsterm.h | 2 +- src/nsterm.m | 25 +++++++++++++------------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/nsterm.h b/src/nsterm.h index 8d6c58290cc..cb162039ad8 100644 --- a/src/nsterm.h +++ b/src/nsterm.h @@ -748,7 +748,7 @@ #define NSTRACE_UNSILENCE() CGContextRef context; bool doubleBuffered; } -- (id) initWithColorSpace: (CGColorSpaceRef)cs doubleBuffered: (bool)db; +- (id) initWithDoubleBuffered: (bool)db; - (void) setColorSpace: (CGColorSpaceRef)cs; - (void) setDoubleBuffered: (bool)db; - (CGContextRef) getContext; diff --git a/src/nsterm.m b/src/nsterm.m index 28502ad1a2a..a3603171bb0 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -7921,8 +7921,6 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f maximizing_resize = NO; #endif - [[EmacsWindow alloc] initWithEmacsFrame:f]; - #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 /* These settings mean AppKit will retain the contents of the frame on resize. Unfortunately it also means the frame will not be @@ -7933,9 +7931,14 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f NSViewLayerContentsRedrawOnSetNeedsDisplay]; [self setLayerContentsPlacement:NSViewLayerContentsPlacementTopLeft]; - /* initWithEmacsFrame can't create the toolbar before the layer is - set, so have another go at creating the toolbar here. */ - [(EmacsWindow*)[self window] createToolbar:f]; + [[EmacsWindow alloc] initWithEmacsFrame:f]; + + /* Now the NSWindow has been created, we can finish up configuring + the layer. */ + [(EmacsLayer *)[self layer] setColorSpace:[[[self window] colorSpace] CGColorSpace]]; + [(EmacsLayer *)[self layer] setContentsScale:[[self window] backingScaleFactor]]; +#else + [[EmacsWindow alloc] initWithEmacsFrame:f]; #endif if (ns_drag_types) @@ -8606,10 +8609,9 @@ - (instancetype)toggleToolbar: (id)sender - (CALayer *)makeBackingLayer { EmacsLayer *l = [[EmacsLayer alloc] - initWithColorSpace:[[[self window] colorSpace] CGColorSpace] - doubleBuffered:FRAME_DOUBLE_BUFFERED (emacsframe)]; + initWithDoubleBuffered:FRAME_DOUBLE_BUFFERED (emacsframe)]; + [l setDelegate:(id)self]; - [l setContentsScale:[[self window] backingScaleFactor]]; return l; } @@ -10437,15 +10439,14 @@ @implementation EmacsLayer cache. If no free surfaces are found in the cache then a new one is created. */ -- (id) initWithColorSpace: (CGColorSpaceRef)cs - doubleBuffered: (bool)db +- (id) initWithDoubleBuffered: (bool)db { - NSTRACE ("[EmacsLayer initWithColorSpace:doubleBuffered:]"); + NSTRACE ("[EmacsLayer initWithDoubleBuffered:]"); self = [super init]; if (self) { - [self setColorSpace:cs]; + [self setColorSpace:nil]; [self setDoubleBuffered:db]; cache = [[NSMutableArray arrayWithCapacity:(doubleBuffered ? 2 : 1)] retain]; } -- 2.40.1 --m9KWJ/KQ+eywibp3--