From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Alan Third Newsgroups: gmane.emacs.bugs Subject: bug#32932: [PATCH v2] Fix more drawing bugs in NS port (bug#32932) Date: Fri, 23 Nov 2018 18:17:54 +0000 Message-ID: <20181123181754.GB24527@breton.holly.idiocy.org> References: <20181109090855.GA31723@breton.holly.idiocy.org> <20181113221336.GA65891@breton.holly.idiocy.org> <20181119223548.GA2325@breton.holly.idiocy.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="ew6BAiZeqk4r7MaW" Content-Transfer-Encoding: 8bit X-Trace: blaine.gmane.org 1542997913 2184 195.159.176.226 (23 Nov 2018 18:31:53 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 23 Nov 2018 18:31:53 +0000 (UTC) User-Agent: Mutt/1.10.1 (2018-07-13) Cc: Boris Buliga , 32932@debbugs.gnu.org To: Aaron Jensen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Nov 23 19:31:48 2018 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gQGF2-0000Q6-9n for geb-bug-gnu-emacs@m.gmane.org; Fri, 23 Nov 2018 19:31:48 +0100 Original-Received: from localhost ([::1]:53886 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gQGH8-0006ZS-Kp for geb-bug-gnu-emacs@m.gmane.org; Fri, 23 Nov 2018 13:33:58 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:34683) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gQGDT-0003sR-U0 for bug-gnu-emacs@gnu.org; Fri, 23 Nov 2018 13:30:15 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gQG2g-0007hM-7f for bug-gnu-emacs@gnu.org; Fri, 23 Nov 2018 13:19:06 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:40272) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gQG2g-0007gf-0M for bug-gnu-emacs@gnu.org; Fri, 23 Nov 2018 13:19:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1gQG2f-0004iu-P7 for bug-gnu-emacs@gnu.org; Fri, 23 Nov 2018 13:19:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Alan Third Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 23 Nov 2018 18:19:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32932 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 32932-submit@debbugs.gnu.org id=B32932.154299708618092 (code B ref 32932); Fri, 23 Nov 2018 18:19:01 +0000 Original-Received: (at 32932) by debbugs.gnu.org; 23 Nov 2018 18:18:06 +0000 Original-Received: from localhost ([127.0.0.1]:44530 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gQG1l-0004hj-Jz for submit@debbugs.gnu.org; Fri, 23 Nov 2018 13:18:06 -0500 Original-Received: from mail-wm1-f50.google.com ([209.85.128.50]:52307) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gQG1j-0004hC-9c for 32932@debbugs.gnu.org; Fri, 23 Nov 2018 13:18:04 -0500 Original-Received: by mail-wm1-f50.google.com with SMTP id r11-v6so12635636wmb.2 for <32932@debbugs.gnu.org>; Fri, 23 Nov 2018 10:18:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=CaFHx5R5n1NX48/LGhmjRNCbeCPPNgSA0/Lu8NHG98w=; b=tA0NM5bpVSFkU+AYqxeUZrkSkfE9X1bFRMYRbUZAXjT8keP6WH0VBQ320Cb2T8but1 WMNqsz51pe7pM97TxBLcCI8P4S1YVzqXuZPoQYw9dWateGdbhrM/viClxP8LR9B89P7N zf4mn1YkHywjjX2jQtstekkfG+FIhD83KdA5J53in9ko9sz/UCNFvc9cEOne2phv3VSR 6JSrdfEzAkxMN0f8K7tVu5LVLw8U/kA/tY6xa//Bgqy9U/Wv86lt71koEEvSdE9cECMZ GPoWTbK3Z4uDmyM/LRMRrBq8CzpKwd2ETS1yoIJgxDMiVLqlstDAHqofRKdf/rZd1jYu x3cA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition :content-transfer-encoding:in-reply-to:user-agent; bh=CaFHx5R5n1NX48/LGhmjRNCbeCPPNgSA0/Lu8NHG98w=; b=f4nbjlH8ihjxni3CUOGXQBygT8T1BkkxseNQE0ZAHzeI43Hb5Yy9h2r/A6Q25jv51q OLc5DEi7BN6s0OOG/YHSOqe/uxkUJtf+u0wHx2i8sCSZavhMAHmWVDU59BCiguQmKFvk Wl3O3xKyxa8PZO4DyWPCp/rWAW2Bho+9xnNR7WkQSXXtBiAsscGEp/u8LroM/eRAMLdL tzlxGoLfUfjBXQqU/IvttHkm0PheyTvI/wKxmL0D5HF6Xz3Bvvo8qkfVsFLgkBXx30Mr M2pOwuNCLrgHXxoW7xDj4TK/gtTmPTIDQ3JWnYfmC6aJKx7po+nZk0Ba0art8CEqAplO s8dA== X-Gm-Message-State: AGRZ1gLDl7ALdfbxlwq2W8xVOk4rUIlBV/vB8ccFt2TkzhCtMTiB72WD VvYZukioksxbI4wgk7MwbB0= X-Google-Smtp-Source: AFSGD/UuetnasjYetZjaf9fJbmg+nHMmAuPvCZXlfYOiYYZFiARkMPHZUXOdd+1ydIazSCG4Dx5eaQ== X-Received: by 2002:a1c:91d1:: with SMTP id t200mr14679718wmd.111.1542997077596; Fri, 23 Nov 2018 10:17:57 -0800 (PST) Original-Received: from breton.holly.idiocy.org (ip6-2001-08b0-03f8-8129-dce1-f8e2-3065-9b77.holly.idiocy.org. [2001:8b0:3f8:8129:dce1:f8e2:3065:9b77]) by smtp.gmail.com with ESMTPSA id 134-v6sm10240091wmp.6.2018.11.23.10.17.56 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 23 Nov 2018 10:17:56 -0800 (PST) Content-Disposition: inline In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 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.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:152718 Archived-At: --ew6BAiZeqk4r7MaW Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Mon, Nov 19, 2018 at 06:30:05PM -0800, Aaron Jensen wrote: > On November 19, 2018 at 2:35:51 PM, Alan Third > (alan@idiocy.org(mailto:alan@idiocy.org)) wrote: > > > Given that Emacs 26.2 is in pre‐release now I’m inclined to leave > > emacs-26 as‐is for now, unless we require the 8th of November patch > > (Further changes to NS drawing (bug#32932))? I don’t think we do as > > iirc it just solves a minor redrawing issue (i.e. Emacs is still > > usable without it). Is that right? > > Yes, it is still usable, though if I remember correctly it lessens the > overall blank glitches (which still happen even w/ the patch). So yes, > it’s usable, but you do occasionally have to resize the window or do > something else to get it to repaint. You still have to w/ the patch, > just less often. Final (hopefully) version of this patch attached. I guess there are still issues with it, but we’re running out of time to sort them for Emacs 26.2. >From a practical stand point I don’t think this is really any different from the last one, but it has some comment changes and a change to how we handle requests to copy parts of the screen, since they were the major issue still with ghosting cursors. I probably need to double check it’s OK on GNUstep, but I don’t foresee any issues there. -- Alan Third --ew6BAiZeqk4r7MaW Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="v3-0001-Fix-more-drawing-bugs-in-NS-port-bug-32932.patch" >From a55e5259341e054023727de4dc86b77c7a7d5db6 Mon Sep 17 00:00:00 2001 From: Alan Third Date: Mon, 29 Oct 2018 15:37:35 +0000 Subject: [PATCH v3] Fix more drawing bugs in NS port (bug#32932) * src/nsterm.m (ns_row_rect): New function. (ns_clip_to_row): Remove function. (ns_copy_bits): Fix mistake. (ns_shift_glyphs_for_insert): Mark the frame as dirty instead of directly copying. (ns_draw_fringe_bitmap): Stop using ns_clip_to_row. (ns_draw_window_cursor): Stop using ns_clip_to_row and perform a display when not in redisplay. (ns_update_window_begin): Remove redundant code that never executes. ([EmacsView drawRect:]): Show the rectangle being exposed in NSTRACE. * src/xdisp.c (expose_window_tree) [HAVE_NS]: (expose_frame) [HAVE_NS]: Redraw even if the frame is garbaged. --- src/nsterm.m | 149 +++++++++++++++++++++++++++------------------------ src/xdisp.c | 15 +++++- 2 files changed, 91 insertions(+), 73 deletions(-) diff --git a/src/nsterm.m b/src/nsterm.m index 4b5d025ee3..948dd1da2e 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -796,6 +796,27 @@ Free a pool and temporary objects it refers to (callable from C) } +static NSRect +ns_row_rect (struct window *w, struct glyph_row *row, + enum glyph_row_area area) +/* Get the row as an NSRect. */ +{ + struct frame *f = XFRAME (WINDOW_FRAME (w)); + NSRect rect; + int window_x, window_y, window_width; + + window_box (w, area, &window_x, &window_y, &window_width, 0); + + rect.origin.x = window_x; + rect.origin.y = WINDOW_TO_FRAME_PIXEL_Y (w, max (0, row->y)); + rect.origin.y = max (rect.origin.y, window_y); + rect.size.width = window_width; + rect.size.height = row->visible_height; + + return rect; +} + + /* ========================================================================== Focus (clipping) and screen update @@ -1048,29 +1069,6 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen) if (! tbar_visible != ! [toolbar isVisible]) [toolbar setVisible: tbar_visible]; } - - /* drawRect may have been called for say the minibuffer, and then clip path - is for the minibuffer. But the display engine may draw more because - we have set the frame as garbaged. So reset clip path to the whole - view. */ - /* FIXME: I don't think we need to do this. */ - if ([NSView focusView] == FRAME_NS_VIEW (f)) - { - NSBezierPath *bp; - NSRect r = [view frame]; - NSRect cr = [[view window] frame]; - /* If a large frame size is set, r may be larger than the window frame - before constrained. In that case don't change the clip path, as we - will clear in to the tool bar and title bar. */ - if (r.size.height - + FRAME_NS_TITLEBAR_HEIGHT (f) - + FRAME_TOOLBAR_HEIGHT (f) <= cr.size.height) - { - bp = [[NSBezierPath bezierPathWithRect: r] retain]; - [bp setClip]; - [bp release]; - } - } #endif } @@ -1206,28 +1204,6 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen) } -static BOOL -ns_clip_to_row (struct window *w, struct glyph_row *row, - enum glyph_row_area area, BOOL gc) -/* -------------------------------------------------------------------------- - Internal (but parallels other terms): Focus drawing on given row - -------------------------------------------------------------------------- */ -{ - struct frame *f = XFRAME (WINDOW_FRAME (w)); - NSRect clip_rect; - int window_x, window_y, window_width; - - window_box (w, area, &window_x, &window_y, &window_width, 0); - - clip_rect.origin.x = window_x; - clip_rect.origin.y = WINDOW_TO_FRAME_PIXEL_Y (w, max (0, row->y)); - clip_rect.origin.y = max (clip_rect.origin.y, window_y); - clip_rect.size.width = window_width; - clip_rect.size.height = row->visible_height; - - return ns_clip_to_rect (f, &clip_rect, 1); -} - /* ========================================================================== Visible bell and beep. @@ -2692,7 +2668,7 @@ so some key presses (TAB) are swallowed by the system. */ ns_copy_bits (struct frame *f, NSRect src, NSRect dest) { NSSize delta = NSMakeSize (dest.origin.x - src.origin.x, - dest.origin.y - src.origin.y) + dest.origin.y - src.origin.y); NSTRACE ("ns_copy_bits"); if (FRAME_NS_VIEW (f)) @@ -2825,12 +2801,20 @@ so some key presses (TAB) are swallowed by the system. */ External (RIF): copy an area horizontally, don't worry about clearing src -------------------------------------------------------------------------- */ { - NSRect srcRect = NSMakeRect (x, y, width, height); + //NSRect srcRect = NSMakeRect (x, y, width, height); NSRect dstRect = NSMakeRect (x+shift_by, y, width, height); NSTRACE ("ns_shift_glyphs_for_insert"); - ns_copy_bits (f, srcRect, dstRect); + /* This doesn't work now as we copy the "bits" before we've had a + chance to actually draw any changes to the screen. This means in + certain circumstances we end up with copies of the cursor all + over the place. Just mark the area dirty so it is redrawn later. + + FIXME: Work out how to do this properly. */ + // ns_copy_bits (f, srcRect, dstRect); + + [FRAME_NS_VIEW (f) setNeedsDisplayInRect:dstRect]; } @@ -2911,6 +2895,9 @@ so some key presses (TAB) are swallowed by the system. */ struct face *face = p->face; static EmacsImage **bimgs = NULL; static int nBimgs = 0; + NSRect clearRect = NSZeroRect; + NSRect imageRect = NSZeroRect; + NSRect rowRect = ns_row_rect (w, row, ANY_AREA); NSTRACE_WHEN (NSTRACE_GROUP_FRINGE, "ns_draw_fringe_bitmap"); NSTRACE_MSG ("which:%d cursor:%d overlay:%d width:%d height:%d period:%d", @@ -2925,25 +2912,40 @@ so some key presses (TAB) are swallowed by the system. */ nBimgs = max_used_fringe_bitmap; } - /* Must clip because of partially visible lines. */ - if (ns_clip_to_row (w, row, ANY_AREA, YES)) + /* Work out the rectangle we will composite into. */ + if (p->which) + imageRect = NSMakeRect (p->x, p->y, p->wd, p->h); + + /* Work out the rectangle we will need to clear. Because we're + compositing rather than blitting, we need to clear the area under + the image regardless of anything else. */ + if (!p->overlay_p) + { + clearRect = NSMakeRect (p->bx, p->by, p->nx, p->ny); + clearRect = NSUnionRect (clearRect, imageRect); + } + else + { + clearRect = imageRect; + } + + /* Handle partially visible rows. */ + clearRect = NSIntersectionRect (clearRect, rowRect); + + /* The visible portion of imageRect will always be contained within + clearRect. */ + if (ns_clip_to_rect (f, &clearRect, 1)) { - if (!p->overlay_p) + if (! NSIsEmptyRect (clearRect)) { - int bx = p->bx, by = p->by, nx = p->nx, ny = p->ny; + NSTRACE_RECT ("clearRect", clearRect); - if (bx >= 0 && nx > 0) - { - NSRect r = NSMakeRect (bx, by, nx, ny); - NSRectClip (r); - [ns_lookup_indexed_color (face->background, f) set]; - NSRectFill (r); - } + [ns_lookup_indexed_color(face->background, f) set]; + NSRectFill (clearRect); } if (p->which) { - NSRect r = NSMakeRect (p->x, p->y, p->wd, p->h); EmacsImage *img = bimgs[p->which - 1]; if (!img) @@ -2964,13 +2966,6 @@ so some key presses (TAB) are swallowed by the system. */ xfree (cbits); } - NSTRACE_RECT ("r", r); - - NSRectClip (r); - /* Since we composite the bitmap instead of just blitting it, we need - to erase the whole background. */ - [ns_lookup_indexed_color(face->background, f) set]; - NSRectFill (r); { NSColor *bm_color; @@ -2990,7 +2985,7 @@ so some key presses (TAB) are swallowed by the system. */ NSTRACE_RECT ("fromRect", fromRect); - [img drawInRect: r + [img drawInRect: imageRect fromRect: fromRect operation: NSCompositingOperationSourceOver fraction: 1.0 @@ -2998,7 +2993,7 @@ so some key presses (TAB) are swallowed by the system. */ hints: nil]; #else { - NSPoint pt = r.origin; + NSPoint pt = imageRect.origin; pt.y += p->h; [img compositeToPoint: pt operation: NSCompositingOperationSourceOver]; } @@ -3088,7 +3083,9 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors. r.size.width = w->phys_cursor_width; /* Prevent the cursor from being drawn outside the text area. */ - if (ns_clip_to_row (w, glyph_row, TEXT_AREA, NO)) + r = NSIntersectionRect (r, ns_row_rect (w, glyph_row, TEXT_AREA)); + + if (ns_clip_to_rect (f, &r, 1)) { face = FACE_FROM_ID_OR_NULL (f, phys_cursor_glyph->face_id); if (face && NS_FACE_BACKGROUND (face) @@ -3128,11 +3125,18 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors. NSRectFill (s); break; } - ns_reset_clipping (f); /* draw the character under the cursor */ if (cursor_type != NO_CURSOR) draw_phys_cursor_glyph (w, glyph_row, DRAW_CURSOR); + + ns_reset_clipping (f); + } + else if (! redisplaying_p) + { + /* If this function is called outside redisplay, it probably + means we need an immediate update. */ + [FRAME_NS_VIEW (f) display]; } } @@ -8096,6 +8100,9 @@ - (void)drawRect: (NSRect)rect for (int i = 0 ; i < numRects ; i++) { NSRect r = rectList[i]; + + NSTRACE_RECT ("r", r); + expose_frame (emacsframe, NSMinX (r), NSMinY (r), NSWidth (r), NSHeight (r)); diff --git a/src/xdisp.c b/src/xdisp.c index 357f0fb30c..808eab7e53 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -32258,7 +32258,14 @@ expose_window_tree (struct window *w, XRectangle *r) struct frame *f = XFRAME (w->frame); bool mouse_face_overwritten_p = false; - while (w && !FRAME_GARBAGED_P (f)) + /* NS toolkits may have aleady modified the frame in expectation of + a successful redraw, so don't bail out here if the frame is + garbaged. */ + while (w +#if !defined (HAVE_NS) + && !FRAME_GARBAGED_P (f) +#endif + ) { mouse_face_overwritten_p |= (WINDOWP (w->contents) @@ -32286,12 +32293,16 @@ expose_frame (struct frame *f, int x, int y, int w, int h) TRACE ((stderr, "expose_frame ")); - /* No need to redraw if frame will be redrawn soon. */ +#if !defined (HAVE_NS) + /* No need to redraw if frame will be redrawn soon except under NS + where the toolkit may have already modified the frame in + expectation of us redrawing it. */ if (FRAME_GARBAGED_P (f)) { TRACE ((stderr, " garbaged\n")); return; } +#endif /* If basic faces haven't been realized yet, there is no point in trying to redraw anything. This can happen when we get an expose -- 2.19.1 --ew6BAiZeqk4r7MaW--