Re: master c22b735: (string-pixel-width): Rewrite to avoid side effects

Re: master c22b735: (string-pixel-width): Rewrite to avoid side effects

[Eli Zaretskii]

>     * src/xdisp.c (Fwindow_text_pixel_size): Allow `window` to be a buffer.
>     * lisp/emacs-lisp/subr-x.el (string-pixel-width): Simplify accordingly.

Bother: this doesn't make sure w->contents is the current buffer,
which I think many display routines assume without checking (or barf
if it isn't true).  So I think this is unsafe.

Re: master c22b735: (string-pixel-width): Rewrite to avoid side effects

[martin rudalics]

 >>      * src/xdisp.c (Fwindow_text_pixel_size): Allow `window` to be a buffer.
 >>      * lisp/emacs-lisp/subr-x.el (string-pixel-width): Simplify accordingly.
 >
 > Bother: this doesn't make sure w->contents is the current buffer,
 > which I think many display routines assume without checking (or barf
 > if it isn't true).  So I think this is unsafe.

If WINDOW is a buffer (a not very tasteful and quite confusing idea
IMHO), 'window-text-pixel-size' will make that buffer current.  The
problem is rather that the window that function operates on usually does
not show WINDOW and I have no idea which implications that could have.

martin

Re: master c22b735: (string-pixel-width): Rewrite to avoid side effects

[martin rudalics]

 > If WINDOW is a buffer (a not very tasteful and quite confusing idea
 > IMHO), 'window-text-pixel-size' will make that buffer current.  The
 > problem is rather that the window that function operates on usually does
 > not show WINDOW and I have no idea which implications that could have.

The implications might be manifold.  With master and emacs -Q do M-:
then yank the form below into the minibuffer and hit RET.

(progn
   (erase-buffer)
   (let ((foo (get-buffer-create "*foo*")))
     (with-current-buffer foo (insert "foo\nbar"))
     (window-text-pixel-size foo)))

Here this gives:

Thread 1 "emacs" hit Breakpoint 5, xsignal1 (error_symbol=XIL(0x2cd0), arg=make_fixnum(4)) at ../../src/eval.c:1949
1949	  xsignal (error_symbol, list1 (arg));
(gdb) bt
#0  0x0000000000692e19 in xsignal1 (error_symbol=XIL(0x2cd0), arg=make_fixnum(4)) at ../../src/eval.c:1949
#1  0x0000000000724a38 in get_char_property_and_overlay (position=make_fixnum(4), prop=XIL(0x9690), object=XIL(0x7ffff3c5c70d), overlay=0x0) at ../../src/textprop.c:634
#2  0x0000000000724d77 in Fget_char_property (position=make_fixnum(4), prop=XIL(0x9690), object=XIL(0x114474d)) at ../../src/textprop.c:680
#3  0x00000000004925b9 in get_it_property (it=0x7fffffffb920, prop=XIL(0x9690)) at ../../src/xdisp.c:22830
#4  0x00000000004ae8b7 in gui_produce_glyphs (it=0x7fffffffb920) at ../../src/xdisp.c:30734
#5  0x00000000004656f8 in move_it_in_display_line_to (it=0x7fffffffb920, to_charpos=8, to_x=-1, op=MOVE_TO_POS) at ../../src/xdisp.c:9362
#6  0x0000000000468691 in move_it_to (it=0x7fffffffb920, to_charpos=8, to_x=-1, to_y=2147483647, to_vpos=-1, op=10) at ../../src/xdisp.c:9971
#7  0x000000000046ad0e in Fwindow_text_pixel_size (window=XIL(0x1b1a0fd), from=XIL(0), to=XIL(0), x_limit=XIL(0), y_limit=XIL(0), mode_lines=XIL(0)) at ../../src/xdisp.c:10808
#8  0x0000000000694d55 in eval_sub (form=XIL(0x157a773)) at ../../src/eval.c:2562
#9  0x000000000068e9e9 in Fprogn (body=XIL(0)) at ../../src/eval.c:469
#10 0x0000000000690b40 in Flet (args=XIL(0x15785c3)) at ../../src/eval.c:1055
#11 0x0000000000694856 in eval_sub (form=XIL(0x15785d3)) at ../../src/eval.c:2496
#12 0x000000000068e9e9 in Fprogn (body=XIL(0)) at ../../src/eval.c:469
#13 0x0000000000694856 in eval_sub (form=XIL(0x15785e3)) at ../../src/eval.c:2496
#14 0x00000000006941bb in Feval (form=XIL(0x15785e3), lexical=XIL(0x30)) at ../../src/eval.c:2372
#15 0x000000000069691f in funcall_subr (subr=0xc67740 <Seval>, numargs=2, args=0x7fffffffd2b0) at ../../src/eval.c:3145
#16 0x00000000006963b8 in Ffuncall (nargs=3, args=0x7fffffffd2a8) at ../../src/eval.c:3068
#17 0x00000000006f0ff5 in exec_byte_code (bytestr=XIL(0x7ffff3fe62e4), vector=XIL(0x7ffff3fe5af5), maxdepth=make_fixnum(11), args_template=make_fixnum(1025), nargs=4, args=0x7fffffffd920) at ../../src/bytecode.c:632
#18 0x0000000000696b80 in fetch_and_exec_byte_code (fun=XIL(0x7ffff3fe5a25), syms_left=make_fixnum(1025), nargs=4, args=0x7fffffffd900) at ../../src/eval.c:3192
#19 0x0000000000697006 in funcall_lambda (fun=XIL(0x7ffff3fe5a25), nargs=4, arg_vector=0x7fffffffd900) at ../../src/eval.c:3273
#20 0x000000000069640c in Ffuncall (nargs=5, args=0x7fffffffd8f8) at ../../src/eval.c:3072
#21 0x0000000000689df5 in Ffuncall_interactively (nargs=5, args=0x7fffffffd8f8) at ../../src/callint.c:260
#22 0x00000000006967f2 in funcall_subr (subr=0xc66f40 <Sfuncall_interactively>, numargs=5, args=0x7fffffffd8f8) at ../../src/eval.c:3123
#23 0x00000000006963b8 in Ffuncall (nargs=6, args=0x7fffffffd8f0) at ../../src/eval.c:3068
#24 0x00000000006955e1 in Fapply (nargs=3, args=0x7fffffffda80) at ../../src/eval.c:2698
#25 0x000000000068a27d in Fcall_interactively (function=XIL(0x7ffff32f7570), record_flag=XIL(0), keys=XIL(0x7ffff434dc05)) at ../../src/callint.c:353
#26 0x000000000069694b in funcall_subr (subr=0xc66f80 <Scall_interactively>, numargs=3, args=0x7fffffffdd40) at ../../src/eval.c:3148
#27 0x00000000006963b8 in Ffuncall (nargs=4, args=0x7fffffffdd38) at ../../src/eval.c:3068
#28 0x00000000006f0ff5 in exec_byte_code (bytestr=XIL(0x7ffff3d8fe0c), vector=XIL(0x7ffff3d8fa75), maxdepth=make_fixnum(13), args_template=make_fixnum(1025), nargs=1, args=0x7fffffffe2b0) at ../../src/bytecode.c:632
#29 0x0000000000696b80 in fetch_and_exec_byte_code (fun=XIL(0x7ffff3d8fa45), syms_left=make_fixnum(1025), nargs=1, args=0x7fffffffe2a8) at ../../src/eval.c:3192
#30 0x0000000000697006 in funcall_lambda (fun=XIL(0x7ffff3d8fa45), nargs=1, arg_vector=0x7fffffffe2a8) at ../../src/eval.c:3273
#31 0x000000000069640c in Ffuncall (nargs=2, args=0x7fffffffe2a0) at ../../src/eval.c:3072
#32 0x0000000000695cd5 in call1 (fn=XIL(0x44a0), arg1=XIL(0x7ffff32f7570)) at ../../src/eval.c:2928
#33 0x00000000005b7182 in command_loop_1 () at ../../src/keyboard.c:1505
#34 0x0000000000691e6a in internal_condition_case (bfun=0x5b6929 <command_loop_1>, handlers=XIL(0x90), hfun=0x5b5dab <cmd_error>) at ../../src/eval.c:1495
#35 0x00000000005b650e in command_loop_2 (handlers=XIL(0x90)) at ../../src/keyboard.c:1133
#36 0x00000000006910ef in internal_catch (tag=XIL(0xe580), func=0x5b64e7 <command_loop_2>, arg=XIL(0x90)) at ../../src/eval.c:1226
#37 0x00000000005b64b2 in command_loop () at ../../src/keyboard.c:1111
#38 0x00000000005b5876 in recursive_edit_1 () at ../../src/keyboard.c:720
#39 0x00000000005b5a8a in Frecursive_edit () at ../../src/keyboard.c:803
#40 0x00000000005b1945 in main (argc=2, argv=0x7fffffffe7b8) at ../../src/emacs.c:2345

Lisp Backtrace:
"window-text-pixel-size" (0xffffccf0)
"let" (0xffffcf50)
"progn" (0xffffd0a0)
"eval" (0xffffd2b0)
"eval-expression" (0xffffd900)
"funcall-interactively" (0xffffd8f8)
"call-interactively" (0xffffdd40)
"command-execute" (0xffffe2a8)


Some xdisp.c routines called by 'window-text-pixel-size' may fail when
the window investigated does not show the current buffer.  We are just
lucky that assertions like

   /* W must display the current buffer.  We could write this function
      to use the frame and buffer of W, but right now it doesn't.  */
   /* eassert (XBUFFER (w->contents) == current_buffer); */

in face_at_buffer_position are currently commented out.


Just to see how a text property in the selected window's buffer can
affect the result of 'string-pixel-width', evaluate with emacs -Q

(defface foo '((t (:height 2.0))) "Foo")

(let ((foo (get-buffer-create "*foo*")))
   (set-window-buffer nil foo)
   (with-current-buffer foo
     (erase-buffer)
     (insert (propertize "foo" 'face 'foo))))

then do via M-:

(string-pixel-width "foo")

Now evaluate

(face-spec-set 'foo '((t (:height 3.0))))

and repeat the preceding two steps.  The value you get varies with the
character width of the text in *foo*.


I'll try to fix this using unwind_with_echo_area_buffer and
with_echo_area_buffer_unwind_data but I can assure you that this will be
a pretty hairy enterprise.  Hence, maybe I'll factor this out in a new
function, say 'string-text-pixel-size', unless Eli has a better idea.
Nothing for the faint at heart ...

martin