From: Eli Zaretskii <eliz@gnu.org>
To: "Dmitry Antipov" <dmantipov@yandex.ru>, "Jan Djärv" <jan.h.d@swipnet.se>
Cc: emacs-devel@gnu.org
Subject: Re: Bidi reordering engine upgraded
Date: Thu, 16 Oct 2014 10:21:52 +0300 [thread overview]
Message-ID: <83oatc4gz3.fsf@gnu.org> (raw)
In-Reply-To: <543F41C9.3000507@yandex.ru>
> Date: Thu, 16 Oct 2014 07:55:53 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> 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?
next prev parent reply other threads:[~2014-10-16 7:21 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-15 14:51 Bidi reordering engine upgraded Eli Zaretskii
2014-10-15 15:22 ` Dmitry Antipov
2014-10-15 15:47 ` Eli Zaretskii
2014-10-15 16:00 ` Dmitry Antipov
2014-10-15 16:31 ` Eli Zaretskii
2014-10-15 17:28 ` Eli Zaretskii
2014-10-15 17:50 ` Eli Zaretskii
2014-10-16 3:55 ` Dmitry Antipov
2014-10-16 7:21 ` Eli Zaretskii [this message]
2014-10-16 9:42 ` Thien-Thi Nguyen
2014-10-16 10:15 ` Eli Zaretskii
2014-10-16 13:27 ` Thien-Thi Nguyen
2014-10-16 13:51 ` Eli Zaretskii
2014-10-17 5:42 ` Thien-Thi Nguyen
2014-10-17 6:16 ` Eli Zaretskii
2014-10-17 7:50 ` Thien-Thi Nguyen
2014-10-17 8:25 ` Eli Zaretskii
2014-10-17 10:27 ` Thien-Thi Nguyen
2014-10-17 10:31 ` Eli Zaretskii
2014-10-16 9:51 ` Dmitry Antipov
2014-10-16 10:20 ` Eli Zaretskii
2014-10-16 11:28 ` Eli Zaretskii
2014-10-17 6:46 ` Eli Zaretskii
2014-10-17 17:45 ` Jan Djärv
2014-10-17 18:45 ` Eli Zaretskii
2014-10-17 19:32 ` Stefan Monnier
2014-10-18 12:34 ` Jan Djärv
2014-10-18 12:54 ` Eli Zaretskii
2014-10-18 13:13 ` Eli Zaretskii
2014-10-19 11:45 ` Jan Djärv
2014-10-19 13:19 ` Eli Zaretskii
2014-10-19 11:49 ` Jan Djärv
2014-10-19 13:20 ` Eli Zaretskii
-- strict thread matches above, loose matches on Subject: below --
2014-10-17 11:12 grischka
2014-10-17 11:42 ` martin rudalics
2014-10-17 11:47 ` David Kastrup
2014-10-17 13:21 ` grischka
2014-10-17 13:30 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=83oatc4gz3.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=dmantipov@yandex.ru \
--cc=emacs-devel@gnu.org \
--cc=jan.h.d@swipnet.se \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).