From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Bidi reordering engine upgraded Date: Thu, 16 Oct 2014 10:21:52 +0300 Message-ID: <83oatc4gz3.fsf@gnu.org> References: <834mv55quj.fsf@gnu.org> <543E9122.6070605@yandex.ru> <8338ap5o7l.fsf@gnu.org> <543E9A1C.2010904@yandex.ru> <831tq95m6x.fsf@gnu.org> <83zjcx450f.fsf@gnu.org> <83y4sh43zq.fsf@gnu.org> <543F41C9.3000507@yandex.ru> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: ger.gmane.org 1413444138 18754 80.91.229.3 (16 Oct 2014 07:22:18 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 16 Oct 2014 07:22:18 +0000 (UTC) Cc: emacs-devel@gnu.org To: Dmitry Antipov , Jan =?utf-8?Q?Dj=C3=A4rv?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Oct 16 09:22:11 2014 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1XefNv-0001ne-MD for ged-emacs-devel@m.gmane.org; Thu, 16 Oct 2014 09:22:07 +0200 Original-Received: from localhost ([::1]:48845 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XefNv-0005JT-89 for ged-emacs-devel@m.gmane.org; Thu, 16 Oct 2014 03:22:07 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:48799) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XefNm-0005Eu-Uk for emacs-devel@gnu.org; Thu, 16 Oct 2014 03:22:04 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XefNh-0005r2-Hk for emacs-devel@gnu.org; Thu, 16 Oct 2014 03:21:58 -0400 Original-Received: from mtaout22.012.net.il ([80.179.55.172]:34403) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XefNh-0005qm-8U for emacs-devel@gnu.org; Thu, 16 Oct 2014 03:21:53 -0400 Original-Received: from conversion-daemon.a-mtaout22.012.net.il by a-mtaout22.012.net.il (HyperSendmail v2007.08) id <0NDJ00000047RZ00@a-mtaout22.012.net.il> for emacs-devel@gnu.org; Thu, 16 Oct 2014 10:21:51 +0300 (IDT) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout22.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0NDJ000D10GFHC90@a-mtaout22.012.net.il>; Thu, 16 Oct 2014 10:21:51 +0300 (IDT) In-reply-to: <543F41C9.3000507@yandex.ru> X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: Solaris 10 X-Received-From: 80.179.55.172 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:175443 Archived-At: > Date: Thu, 16 Oct 2014 07:55:53 +0400 > From: Dmitry Antipov > CC: emacs-devel@gnu.org > > On 10/15/2014 09:50 PM, Eli Zaretskii wrote: > > > So once again, I need to ask you to step through x_draw_hollow_cursor, > > and see what goes wrong there in this case. > > 7950 x_draw_hollow_cursor (struct window *w, struct glyph_row *row) > 7951 { > 7952 struct frame *f = XFRAME (WINDOW_FRAME (w)); > 7953 struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (f); > 7954 Display *dpy = FRAME_X_DISPLAY (f); > 7955 int x, y, wd, h; > 7956 XGCValues xgcv; > 7957 struct glyph *cursor_glyph; > 7958 GC gc; > 7959 > 7960 /* Get the glyph the cursor is on. If we can't tell because > 7961 the current matrix is invalid or such, give up. */ > 1 7962 cursor_glyph = get_phys_cursor_glyph (w); > 7963 if (cursor_glyph == NULL) > 7964 return; > 7965 > 7966 /* Compute frame-relative coordinates for phys cursor. */ > 7967 get_phys_cursor_geometry (w, row, cursor_glyph, &x, &y, &h); > 2 7968 wd = w->phys_cursor_width; > 7969 > 7970 /* The foreground of cursor_gc is typically the same as the normal > 7971 background color, which can cause the cursor box to be invisible. */ > 7972 xgcv.foreground = f->output_data.x->cursor_pixel; > 7973 if (dpyinfo->scratch_cursor_gc) > 7974 XChangeGC (dpy, dpyinfo->scratch_cursor_gc, GCForeground, &xgcv); > 7975 else > 7976 dpyinfo->scratch_cursor_gc = XCreateGC (dpy, FRAME_X_WINDOW (f), > 7977 GCForeground, &xgcv); > 7978 gc = dpyinfo->scratch_cursor_gc; > 7979 > 7980 /* When on R2L character, show cursor at the right edge of the > 7981 glyph, unless the cursor box is as wide as the glyph or wider > 7982 (the latter happens when x-stretch-cursor is non-nil). */ > 7983 if ((cursor_glyph->resolved_level & 1) != 0 > 7984 && cursor_glyph->pixel_width > w->phys_cursor_width) > 7985 { > 7986 x += cursor_glyph->pixel_width - w->phys_cursor_width; > 3 7987 wd -= 1; > 7988 } > 7989 /* Set clipping, draw the rectangle, and reset clipping again. */ > 7990 x_clip_to_row (w, row, TEXT_AREA, gc); > 4 7991 XDrawRectangle (dpy, FRAME_X_WINDOW (f), gc, x, y, wd, h - 1); > 7992 XSetClipMask (dpy, gc, None); > 7993 } > > At 1), cursor_glyph is GLYPHLESS_GLYPH with pixel_width 1; > at 2), wd is 0; > at 3), wd is -1; > at 4), there is an integer overflow because XDrawRectangle accepts > width and height as unsigned. Thanks. So this is not really a result of my merge-commit in r118121, this problem should have existed before, since r117923, right? The other question I have is: what does XDrawRectangle do when its 6th argument 'width' is zero? That's what it was getting before r117923 on this character, AFAIU. Does it enlarge the rectangle to 1 pixel, or does it not show the hollow cursor at all? (You could probably simulate what was happening before r117923 by setting bidi-display-reordering to nil.) Anyway, it looks like the right fix for this is as follows: === modified file 'src/xdisp.c' --- src/xdisp.c 2014-10-14 18:10:37 +0000 +++ src/xdisp.c 2014-10-16 07:16:49 +0000 @@ -2303,9 +2303,6 @@ get_phys_cursor_geometry (struct window rectangle as wide as the glyph, but use a canonical character width instead. */ wd = glyph->pixel_width - 1; -#if defined (HAVE_NTGUI) || defined (HAVE_NS) - wd++; /* Why? */ -#endif x = w->phys_cursor.x; if (x < 0) I never understood why we subtract 1 pixel from the cursor glyph's pixel_width, anyway, and w32 and ns countermanded that, as you see. Maybe we should also limit 'wd' from below, so it is at least 1. Jan, can you comment on these issues and on the proposed patch, please?