From: Ari Roponen <ari.roponen@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 11464@debbugs.gnu.org, mwd@cert.org
Subject: bug#11464: 24.1.50; pos-visible-in-window-p returns a false positive with bidi text
Date: Fri, 18 May 2012 13:47:26 +0300 [thread overview]
Message-ID: <87obplepfl.fsf@gmail.com> (raw)
In-Reply-To: <83vcjtswst.fsf@gnu.org> (Eli Zaretskii's message of "Fri, 18 May 2012 11:44:34 +0300")
Eli Zaretskii <eliz@gnu.org> writes:
> And if that happens, the correction code below, viz.:
>
> if (bottom_y <= it.last_visible_y
> && it.bidi_p && it.bidi_it.scan_dir == -1
> && IT_CHARPOS (it) < charpos)
>
> would evaluate to false, and visible_p would have stayed at its 1
> value, which is wrong.
>
> So could you please clarify this case?
Yes, pos_visible_p indeed returns incorrect value (1), but it is called
from Fpos_visible_in_window_p, which returns the correct value. For
longer description, see below.
>
> Also, what was the value of top_y in your first case, i.e.:
>
>> bottom_y = 300
>> it.last_visible_y = 304
That case has the following values now (in emacs-24 rev. 108005):
bottom_y = 302
it.last_visible_y = 304
top_y = 285
I'm not sure why bottom_y has changed its value. I guess that is
because I installed some new fonts.
>
> Again, sorry for bothering you with the details. I just must
> understand what is going on here and where did I err in my original
> change.
No worries. Here are some logs from my debugging sessions. I don't
pretend to know the code, but this shows why the bug didn't happen in
the last case ("emacs -fn fixed").
I tried emacs-24 revision 108005, compiled with "-O0 -ggdb".
File bug.el contains this:
(progn
(pop-to-buffer (get-buffer-create "test"))
(erase-buffer)
(dotimes (i (* 2 (window-height)))
(insert "\u05d0\n")) ; HEBREW LETTER ALEF
(insert "Last line\n")
(goto-char (point-min))
(insert "a")
(message "Should be nil: %S" (pos-visible-in-window-p (point-max))))
The function pos_visible_p returns the incorrect value (1):
[src]$ gdb --quiet --args ./emacs -Q -fn fixed -l bug.el
Reading symbols from /usr/local/repos/bzr/emacs/emacs-24/src/emacs...done.
warning: File "/usr/local/repos/bzr/emacs/emacs-24/src/.gdbinit" auto-loading has been declined by your `auto-load safe-path' set to "/usr/share/gdb/auto-load:/usr/lib/debug:/usr/bin/mono-gdb.py".
(gdb) source .gdbinit
SIGINT is used by the debugger.
Are you sure you want to change it? (y or n) [answered Y; input not from terminal]
DISPLAY = :0
TERM = xterm
Breakpoint 1 at 0x5623fd: file emacs.c, line 394.
Temporary breakpoint 2 at 0x588153: file sysdep.c, line 859.
(gdb) b xdisp.c:1311
Breakpoint 3 at 0x4311d1: file xdisp.c, line 1311.
(gdb) b xdisp.c:1566
Breakpoint 4 at 0x4324d2: file xdisp.c, line 1566.
(gdb) r
Starting program: /usr/local/repos/bzr/emacs/emacs-24/src/emacs -Q -fn fixed -l bug.el
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x7fffe4578700 (LWP 28308)]
[New Thread 0x7fffe3d77700 (LWP 28309)]
Breakpoint 3, pos_visible_p (w=0x13dbd90, charpos=80, x=0x7fffffffb78c, y=
0x7fffffffb788, rtop=0x7fffffffb79c, rbot=0x7fffffffb798, rowh=
0x7fffffffb794, vpos=0x7fffffffb790) at xdisp.c:1312
1312 if (top_y < window_top_y)
Missing separate debuginfos, use: debuginfo-install [...]
(gdb) p visible_p
$1 = 0
(gdb) n
1314 else if (top_y < it.last_visible_y)
(gdb)
1315 visible_p = 1;
(gdb)
1316 if (bottom_y <= it.last_visible_y
(gdb)
1341 if (visible_p)
(gdb) p visible_p
$2 = 1
(gdb) c
Continuing.
Breakpoint 4, pos_visible_p (w=0x13dbd90, charpos=80, x=0x7fffffffb78c, y=
0x7fffffffb788, rtop=0x7fffffffb79c, rbot=0x7fffffffb798, rowh=
0x7fffffffb794, vpos=0x7fffffffb790) at xdisp.c:1566
1566 return visible_p;
(gdb) p visible_p
$3 = 1
(gdb)
The reason the test case works is that the function
Fpos_visible_in_window_p contains this:
/* If position is above window start or outside buffer boundaries,
or if window start is out of range, position is not visible. */
if ((EQ (pos, Qt)
|| (posint >= CHARPOS (top) && posint <= BUF_ZV (buf)))
&& CHARPOS (top) >= BUF_BEGV (buf)
&& CHARPOS (top) <= BUF_ZV (buf)
&& pos_visible_p (w, posint, &x, &y, &rtop, &rbot, &rowh, &vpos)
&& (fully_p = !rtop && !rbot, (!NILP (partially) || fully_p)))
in_window = Qt;
Here pos_visible_p returns 1, but the next condition evaluates to false, since
(!NILP (partially) || fully_p) is false.
[src]$ gdb --quiet --args ./emacs -Q -fn fixed -l bug.el
Reading symbols from /usr/local/repos/bzr/emacs/emacs-24/src/emacs...done.
warning: File "/usr/local/repos/bzr/emacs/emacs-24/src/.gdbinit" auto-loading has been declined by your `auto-load safe-path' set to "/usr/share/gdb/auto-load:/usr/lib/debug:/usr/bin/mono-gdb.py".
(gdb) source .gdbinit
SIGINT is used by the debugger.
Are you sure you want to change it? (y or n) [answered Y; input not from terminal]
DISPLAY = :0
TERM = xterm
Breakpoint 1 at 0x5623fd: file emacs.c, line 394.
Temporary breakpoint 2 at 0x588153: file sysdep.c, line 859.
(gdb) b Fpos_visible_in_window_p
Breakpoint 3 at 0x48a5db: file window.c, line 1431.
(gdb) r
Starting program: /usr/local/repos/bzr/emacs/emacs-24/src/emacs -Q -fn fixed -l bug.el
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x7fffe4578700 (LWP 28471)]
[New Thread 0x7fffe3d77700 (LWP 28472)]
Breakpoint 3, Fpos_visible_in_window_p (pos=320, window=12763682, partially=
12763682) at window.c:1431
1431 Lisp_Object in_window = Qnil;
Missing separate debuginfos, use: debuginfo-install [..]
(gdb) n
1432 int rtop, rbot, rowh, vpos, fully_p = 1;
(gdb)
1435 w = decode_window (window);
(gdb)
1436 buf = XBUFFER (w->buffer);
(gdb)
1437 SET_TEXT_POS_FROM_MARKER (top, w->start);
(gdb)
1439 if (EQ (pos, Qt))
(gdb)
1441 else if (!NILP (pos))
(gdb)
1443 CHECK_NUMBER_COERCE_MARKER (pos);
(gdb)
1444 posint = XINT (pos);
(gdb)
1453 if ((EQ (pos, Qt)
(gdb)
1454 || (posint >= CHARPOS (top) && posint <= BUF_ZV (buf)))
(gdb)
1455 && CHARPOS (top) >= BUF_BEGV (buf)
(gdb)
1456 && CHARPOS (top) <= BUF_ZV (buf)
(gdb)
1457 && pos_visible_p (w, posint, &x, &y, &rtop, &rbot, &rowh, &vpos)
(gdb)
1458 && (fully_p = !rtop && !rbot, (!NILP (partially) || fully_p)))
(gdb)
1461 if (!NILP (in_window) && !NILP (partially))
(gdb) xprintsym in_window
"nil"(gdb) p fully_p
$1 = 0
(gdb) xprintsym partially
"nil"(gdb) n
1471 return in_window;
(gdb)
--
Ari Roponen
next prev parent reply other threads:[~2012-05-18 10:47 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-13 15:54 bug#11464: 24.1.50; pos-visible-in-window-p returns a false positive with bidi text Ari Roponen
2012-05-13 18:26 ` Eli Zaretskii
2012-05-15 10:07 ` Ari Roponen
2012-05-15 16:19 ` Eli Zaretskii
2012-05-16 5:21 ` Ari Roponen
2012-05-16 15:15 ` Eli Zaretskii
2012-05-17 4:52 ` Ari Roponen
2012-05-17 16:23 ` Eli Zaretskii
2012-05-17 17:54 ` Ari Roponen
2012-05-17 17:56 ` Michael Welsh Duggan
2012-05-17 21:11 ` Eli Zaretskii
2012-05-17 21:22 ` Michael Welsh Duggan
2012-05-18 7:42 ` Eli Zaretskii
2012-05-18 8:03 ` Ari Roponen
2012-05-18 8:26 ` Ari Roponen
2012-05-18 8:44 ` Eli Zaretskii
2012-05-18 10:47 ` Ari Roponen [this message]
2012-05-18 11:32 ` Eli Zaretskii
2012-05-18 11:47 ` Ari Roponen
2012-05-18 14:02 ` Eli Zaretskii
2012-05-18 14:39 ` Ari Roponen
2012-05-18 14:59 ` Eli Zaretskii
2012-05-19 12:26 ` Eli Zaretskii
2012-05-19 12:32 ` Ari Roponen
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=87obplepfl.fsf@gmail.com \
--to=ari.roponen@gmail.com \
--cc=11464@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=mwd@cert.org \
/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).