Changes in frame/window code

Changes in frame/window code

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 881 bytes --]

Dear people

In the next days I intend to install the attached patch.  Its primary
aims are to:

(1) Finish the transition to pixelwise frame/window size handling (for
     example, removing the necessity that fringe-sizes are an integral
     multiple of columns or the remnants of the "extended" scrollbar).

(2) Fix the "toolbar/menubar height is part of the frame text height"
     issue (which the recent change in revision 117561 doesn't).

(3) Make the frame size resilient to menu-/tool-/scrollbar/font/fringes
     changes when it's either maximized/fullscreen or the user generally
     dislikes implied size changes.

(4) Add horizontal scroll bars for the Gtk/Motif/Lucid/Windows builds.

Kindly refrain from substantial frame/window code changes in the next
few days to make the transition as smooth as possible.  And, obviously,
please try the patch.

Thanks, martin

[-- Attachment #2: frames-windows.diff.gz --]
[-- Type: application/gzip, Size: 93064 bytes --]

Re: Changes in frame/window code

[Dmitry Antipov]

[-- Attachment #1: Type: text/plain, Size: 4706 bytes --]

On 07/22/2014 05:42 PM, martin rudalics wrote:

> (2) Fix the "toolbar/menubar height is part of the frame text height"
>      issue (which the recent change in revision 117561 doesn't).

What's wrong with that?

> (4) Add horizontal scroll bars for the Gtk/Motif/Lucid/Windows builds.
>
> Kindly refrain from substantial frame/window code changes in the next
> few days to make the transition as smooth as possible.  And, obviously,
> please try the patch.

0) Debug stubs (Vmy_debug_it in widget.c)?

1) It doesn't even compile with --enable-gcc-warnings (didn't you use it for
    development?) due to missing 'x_clear_under_internal_border' prototype.

2) It crashes with both Lucid and Motif (after a few attempts to toggle
    with M-x horizontal-scroll-bar-mode and drag):

#0  0x000000379220f62b in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:37
#1  0x000000000056bcbc in terminate_due_to_signal (sig=6, backtrace_limit=2147483647) at ../../trunk/src/emacs.c:387
#2  0x00000000005f406d in die (msg=0x70acfe "VECTORLIKEP (a)", file=0x70acb8 "../../trunk/src/lisp.h", line=866)
     at ../../trunk/src/alloc.c:7102
#3  0x0000000000566ba5 in XVECTOR (a=...) at ../../trunk/src/lisp.h:866
#4  0x0000000000538106 in xt_action_hook (widget=0xd10060, client_data=0x0, action_name=0x138e2ec "EndScroll", event=0x7fffd5134100,
     params=0x0, num_params=0x372f066290 <dummyAction.9840+16>) at ../../trunk/src/xterm.c:4248
#5  0x000000372ee48cae in HandleActions (w=w@entry=0xd10060, event=0x7fffd5134100, accelWidget=<optimized out>, procs=0xdc71d8,
     actions=actions@entry=0x372f066280 <dummyAction.9840>, stateTree=<optimized out>) at TMstate.c:634
#6  0x000000372ee49114 in HandleSimpleState (w=w@entry=0xd10060, tmRecPtr=tmRecPtr@entry=0xd100a8,
     curEventPtr=curEventPtr@entry=0x7fffd51336e0) at TMstate.c:884
#7  0x000000372ee49f7c in _XtTranslateEvent (w=w@entry=0xd10060, event=event@entry=0x7fffd5134100) at TMstate.c:1101
#8  0x000000372ee22233 in XtDispatchEventToWidget (widget=widget@entry=0xd10060, event=event@entry=0x7fffd5134100) at Event.c:906
#9  0x000000372ee22950 in _XtDefaultDispatcher (event=0x7fffd5134100) at Event.c:1367
#10 0x000000372ee22a29 in XtDispatchEvent (event=0x7fffd5134100) at Event.c:1423
#11 0x000000000053eb04 in handle_one_xevent (dpyinfo=0x1363710, event=0x7fffd5134100, finish=0x7fffd51341cc,
     hold_quit=0x7fffd51341f0) at ../../trunk/src/xterm.c:7583
#12 0x000000000053ed7c in XTread_socket (terminal=0xf68fc8, hold_quit=0x7fffd51341f0) at ../../trunk/src/xterm.c:7682
#13 0x000000000057d88d in gobble_input () at ../../trunk/src/keyboard.c:6871
#14 0x000000000057de06 in handle_async_input () at ../../trunk/src/keyboard.c:7123
#15 0x000000000057de25 in process_pending_signals () at ../../trunk/src/keyboard.c:7137
#16 0x000000000057de64 in unblock_input_to (level=0) at ../../trunk/src/keyboard.c:7152
#17 0x000000000057de87 in unblock_input () at ../../trunk/src/keyboard.c:7171
#18 0x00000000006c2ae7 in xg_select (fds_lim=7, rfds=0x7fffd5134b00, wfds=0x7fffd5134a80, efds=0x0, timeout=0x7fffd5134a60,
     sigmask=0x0) at ../../trunk/src/xgselect.c:151
#19 0x000000000066c9a5 in wait_reading_process_output (time_limit=30, nsecs=0, read_kbd=-1, do_display=true, wait_for_cell=...,
     wait_proc=0x0, just_wait_proc=0) at ../../trunk/src/process.c:4595
#20 0x0000000000422a40 in sit_for (timeout=..., reading=true, display_option=1) at ../../trunk/src/dispnew.c:5752
#21 0x0000000000573fb6 in read_char (commandflag=1, map=..., prev_event=..., used_mouse_menu=0x7fffd513536f, end_time=0x0)
     at ../../trunk/src/keyboard.c:2799
#22 0x0000000000582e6e in read_key_sequence (keybuf=0x7fffd5135550, bufsize=30, prompt=..., dont_downcase_last=false,
     can_return_switch_frame=true, fix_current_buffer=true, prevent_redisplay=false) at ../../trunk/src/keyboard.c:9120
#23 0x000000000057064b in command_loop_1 () at ../../trunk/src/keyboard.c:1438
#24 0x0000000000612373 in internal_condition_case (bfun=0x570289 <command_loop_1>, handlers=..., hfun=0x56fa59 <cmd_error>)
     at ../../trunk/src/eval.c:1347
#25 0x000000000056ff27 in command_loop_2 (ignore=...) at ../../trunk/src/keyboard.c:1169
#26 0x00000000006117f6 in internal_catch (tag=..., func=0x56ff04 <command_loop_2>, arg=...) at ../../trunk/src/eval.c:1111
#27 0x000000000056fedb in command_loop () at ../../trunk/src/keyboard.c:1148
#28 0x000000000056f585 in recursive_edit_1 () at ../../trunk/src/keyboard.c:769
#29 0x000000000056f755 in Frecursive_edit () at ../../trunk/src/keyboard.c:840
#30 0x000000000056d65c in main (argc=2, argv=0x7fffd51359d8) at ../../trunk/src/emacs.c:1650

3) On Lucid, scroll bars are of the different colors (see screenshot).

Dmitry

[-- Attachment #2: lucid.png --]
[-- Type: image/png, Size: 24153 bytes --]

Re: Changes in frame/window code

[Dmitry Antipov]

On 07/22/2014 05:42 PM, martin rudalics wrote:

> In the next days I intend to install the attached patch.  Its primary
> aims are to:
>
> (1) Finish the transition to pixelwise frame/window size handling (for
>      example, removing the necessity that fringe-sizes are an integral
>      multiple of columns or the remnants of the "extended" scrollbar).
>
> (2) Fix the "toolbar/menubar height is part of the frame text height"
>      issue (which the recent change in revision 117561 doesn't).
>
> (3) Make the frame size resilient to menu-/tool-/scrollbar/font/fringes
>      changes when it's either maximized/fullscreen or the user generally
>      dislikes implied size changes.
>
> (4) Add horizontal scroll bars for the Gtk/Motif/Lucid/Windows builds.
>
> Kindly refrain from substantial frame/window code changes in the next
> few days to make the transition as smooth as possible.  And, obviously,
> please try the patch.

Maintenance/support notice: in general, I think that ~400K patch is just
a bad joke. IMO it's highly desirable to split 1), 2) and 3) in one
patch and apply it first. 4) should go next, and only after a more or
less representative feedback on 1), 2) and 3).

Dmitry

Re: Changes in frame/window code

[martin rudalics]

 >> (2) Fix the "toolbar/menubar height is part of the frame text height"
 >>      issue (which the recent change in revision 117561 doesn't).
 >
 > What's wrong with that?

With emacs -Q and an internal tool bar evaluate one after the other

(frame-height)
(set-frame-height nil (1+ (frame-height)))
(frame-height)

Here the `set-frame-height' shrinks the frame.

 > 0) Debug stubs (Vmy_debug_it in widget.c)?

Thanks for spotting them.  I have created the patch on Windows, so this
is indeed a leftover from the GNU/Linux platform debugging code.

 > 1) It doesn't even compile with --enable-gcc-warnings (didn't you use it for
 >     development?)

On GNU/Linux I always compile with --enable-gcc-warnings=yes
--enable-checking=yes --enable-check-lisp-object-type=yes

 > due to missing 'x_clear_under_internal_border' prototype.

Sorry.  It's in xterm.h here (and thusly also mentioned in the
ChangeLog) but got lost in the transition to my Windows machine.

 > 2) It crashes with both Lucid and Motif (after a few attempts to toggle
 >     with M-x horizontal-scroll-bar-mode and drag):
 > #0  0x000000379220f62b in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:37
 > #1  0x000000000056bcbc in terminate_due_to_signal (sig=6, backtrace_limit=2147483647) at ../../trunk/src/emacs.c:387
 > #2  0x00000000005f406d in die (msg=0x70acfe "VECTORLIKEP (a)", file=0x70acb8 "../../trunk/src/lisp.h", line=866)
 >      at ../../trunk/src/alloc.c:7102
 > #3  0x0000000000566ba5 in XVECTOR (a=...) at ../../trunk/src/lisp.h:866
 > #4  0x0000000000538106 in xt_action_hook (widget=0xd10060, client_data=0x0, action_name=0x138e2ec "EndScroll", event=0x7fffd5134100,
 >      params=0x0, num_params=0x372f066290 <dummyAction.9840+16>) at ../../trunk/src/xterm.c:4248

I can't reproduce that here - what precisely means "toggle with M-x
horizontal-scroll-bar-mode and drag"?  Dragging the slider with the
mouse?  Do you have to toggle more than once to make the bug happen?
Do you have any interpretation for what happened?

 > 3) On Lucid, scroll bars are of the different colors (see screenshot).

I can't see such a thing here.  Is this with Xaw3D?

Thanks for the feedback, martin

Re: Changes in frame/window code

[martin rudalics]

 >> (1) Finish the transition to pixelwise frame/window size handling (for
 >>      example, removing the necessity that fringe-sizes are an integral
 >>      multiple of columns or the remnants of the "extended" scrollbar).
 >>
 >> (2) Fix the "toolbar/menubar height is part of the frame text height"
 >>      issue (which the recent change in revision 117561 doesn't).
 >>
 >> (3) Make the frame size resilient to menu-/tool-/scrollbar/font/fringes
 >>      changes when it's either maximized/fullscreen or the user generally
 >>      dislikes implied size changes.
 >>
 >> (4) Add horizontal scroll bars for the Gtk/Motif/Lucid/Windows builds.
 >>
 >> Kindly refrain from substantial frame/window code changes in the next
 >> few days to make the transition as smooth as possible.  And, obviously,
 >> please try the patch.
 >
 > Maintenance/support notice: in general, I think that ~400K patch is just
 > a bad joke. IMO it's highly desirable to split 1), 2) and 3) in one
 > patch and apply it first. 4) should go next, and only after a more or
 > less representative feedback on 1), 2) and 3).

I agree with you.  But I started to do (4) first and only later added
the remaining steps (and a few more).  These are heavily interconnected
so I don't see a way to safely split them any more.  Moreover splitting
would take me a month at least and I won't be able to spend so much time
on this.

martin

set-frame-height glitch [Was: Re: Changes in frame/window code]

[Dmitry Antipov]

On 07/23/2014 04:48 PM, martin rudalics wrote:

> With emacs -Q and an internal tool bar evaluate one after the other
>
> (frame-height)
> (set-frame-height nil (1+ (frame-height)))
> (frame-height)
>
> Here the `set-frame-height' shrinks the frame.

Nice shot.  But, IIUC, this means that set-frame-height goes wrong
but should be:

=== modified file 'src/frame.c'
--- src/frame.c	2014-07-21 16:58:12 +0000
+++ src/frame.c	2014-07-23 15:01:44 +0000
@@ -2584,8 +2584,11 @@
      {
        if (NILP (pixelwise))
  	{
-	  if (XINT (height) != FRAME_LINES (f))
-	    x_set_window_size (f, 1, FRAME_COLS (f), XINT (height), 0);
+	  int actual = FRAME_LINES (f) - FRAME_TOP_MARGIN (f);
+	  int requested = XINT (height) + FRAME_TOP_MARGIN (f);
+
+	  if (requested != actual)
+	    x_set_window_size (f, 1, FRAME_COLS (f), requested, 0);

  	  do_pending_window_change (0);
  	}

Dmitry

Re: Changes in frame/window code

[martin rudalics]

 >  > 2) It crashes with both Lucid and Motif (after a few attempts to toggle
 >  >     with M-x horizontal-scroll-bar-mode and drag):
 >  > #0  0x000000379220f62b in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:37
 >  > #1  0x000000000056bcbc in terminate_due_to_signal (sig=6, backtrace_limit=2147483647) at ../../trunk/src/emacs.c:387
 >  > #2  0x00000000005f406d in die (msg=0x70acfe "VECTORLIKEP (a)", file=0x70acb8 "../../trunk/src/lisp.h", line=866)
 >  >      at ../../trunk/src/alloc.c:7102
 >  > #3  0x0000000000566ba5 in XVECTOR (a=...) at ../../trunk/src/lisp.h:866
 >  > #4  0x0000000000538106 in xt_action_hook (widget=0xd10060, client_data=0x0, action_name=0x138e2ec "EndScroll", event=0x7fffd5134100,
 >  >      params=0x0, num_params=0x372f066290 <dummyAction.9840+16>) at ../../trunk/src/xterm.c:4248
 >
 > I can't reproduce that here - what precisely means "toggle with M-x
 > horizontal-scroll-bar-mode and drag"?  Dragging the slider with the
 > mouse?  Do you have to toggle more than once to make the bug happen?
 > Do you have any interpretation for what happened?

I see now - xt_action_hook has a hardcoded

bar = XSCROLL_BAR (w->vertical_scroll_bar);

so you can trigger the bug any time by disabling vertical scrollbars and
dragging a horizontal slider.  I'll fix that.

Thanks again, martin

Re: set-frame-height glitch [Was: Re: Changes in frame/window code]

[martin rudalics]

 > Nice shot.  But, IIUC, this means that set-frame-height goes wrong
 > but should be:
 >
 > === modified file 'src/frame.c'
 > --- src/frame.c    2014-07-21 16:58:12 +0000
 > +++ src/frame.c    2014-07-23 15:01:44 +0000
 > @@ -2584,8 +2584,11 @@
 >       {
 >         if (NILP (pixelwise))
 >       {
 > -      if (XINT (height) != FRAME_LINES (f))
 > -        x_set_window_size (f, 1, FRAME_COLS (f), XINT (height), 0);
 > +      int actual = FRAME_LINES (f) - FRAME_TOP_MARGIN (f);
 > +      int requested = XINT (height) + FRAME_TOP_MARGIN (f);
 > +
 > +      if (requested != actual)
 > +        x_set_window_size (f, 1, FRAME_COLS (f), requested, 0);
 >
 >         do_pending_window_change (0);
 >       }

In some sort of sense, yes.  Have you looked at my code?  I wrote
adjust_frame_size to handle this and related cases in a uniform manner.

What I never got around to handle precisely is x_figure_window_size
which calculates the toolbar off by two pixels and the menubar off by
one pixel (the latter on either Lucid or Motif).  So my initial frame
may lack one or two pixels.

martin

Re: Changes in frame/window code

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 993 bytes --]

>  >  > 2) It crashes with both Lucid and Motif (after a few attempts to toggle
>  >  >     with M-x horizontal-scroll-bar-mode and drag):
>  >  > #0  0x000000379220f62b in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:37
>  >  > #1  0x000000000056bcbc in terminate_due_to_signal (sig=6, backtrace_limit=2147483647) at ../../trunk/src/emacs.c:387
>  >  > #2  0x00000000005f406d in die (msg=0x70acfe "VECTORLIKEP (a)", file=0x70acb8 "../../trunk/src/lisp.h", line=866)
>  >  >      at ../../trunk/src/alloc.c:7102
>  >  > #3  0x0000000000566ba5 in XVECTOR (a=...) at ../../trunk/src/lisp.h:866
>  >  > #4  0x0000000000538106 in xt_action_hook (widget=0xd10060, client_data=0x0, action_name=0x138e2ec "EndScroll", event=0x7fffd5134100,
>  >  >      params=0x0, num_params=0x372f066290 <dummyAction.9840+16>) at ../../trunk/src/xterm.c:4248

Should be fixed in the attached patch.  It should now also compile
without problems on GNU/Linux but contains some debugging code.

martin


[-- Attachment #2: frames-windows-GNU.diff.gz --]
[-- Type: application/gzip, Size: 93319 bytes --]

Re: Changes in frame/window code

[martin rudalics]

 > In the next days I intend to install the attached patch.  Its primary
 > aims are to:
 >
 > (1) Finish the transition to pixelwise frame/window size handling (for
 >      example, removing the necessity that fringe-sizes are an integral
 >      multiple of columns or the remnants of the "extended" scrollbar).
 >
 > (2) Fix the "toolbar/menubar height is part of the frame text height"
 >      issue (which the recent change in revision 117561 doesn't).
 >
 > (3) Make the frame size resilient to menu-/tool-/scrollbar/font/fringes
 >      changes when it's either maximized/fullscreen or the user generally
 >      dislikes implied size changes.
 >
 > (4) Add horizontal scroll bars for the Gtk/Motif/Lucid/Windows builds.

I have committed the changes now.  Although I gave them some thorough
testing with the Gtk/Lucid/Motif and Windows builds here, they very
likely might break with other window managers or toolkits.  I did some
light tests with GNUstep but am convinced that the NS build in
particular will run into more serious problems.

I'm aware that the changes should have been applied separately.  But as
I explained in another post, doing so would have cost me too much time.

Note that horizontal scrollbars are turned on by default on all builds
that support them.  This is the only way I see to get them at least some
initial coverage.  If you utterly dislike them (or do not use scrollbars
at all) please use the idiom

(when (fboundp 'horizontal-scroll-bar-mode)
   (horizontal-scroll-bar-mode -1))

I intend to disable horizontal scrollbars by default in the next weeks.

I plan to add the corresponding NEWS entries in the next few days.

Thanks for any feedback, martin

Re: Changes in frame/window code

[Jan Djärv]

Hello.

27 jul 2014 kl. 15:32 skrev martin rudalics <rudalics@gmx.at>:

> > In the next days I intend to install the attached patch.  Its primary
> > aims are to:
> >
> > (1) Finish the transition to pixelwise frame/window size handling (for
> >      example, removing the necessity that fringe-sizes are an integral
> >      multiple of columns or the remnants of the "extended" scrollbar).
> >
> > (2) Fix the "toolbar/menubar height is part of the frame text height"
> >      issue (which the recent change in revision 117561 doesn't).
> >
> > (3) Make the frame size resilient to menu-/tool-/scrollbar/font/fringes
> >      changes when it's either maximized/fullscreen or the user generally
> >      dislikes implied size changes.
> >
> > (4) Add horizontal scroll bars for the Gtk/Motif/Lucid/Windows builds.
> 
> I have committed the changes now.

Why did you remove unrelated code from nsterm.m having to do with antialias threshold?  Do you not check whats changed before you checkin?

>  Although I gave them some thorough
> testing with the Gtk/Lucid/Motif and Windows builds here, they very
> likely might break with other window managers or toolkits.  I did some
> light tests with GNUstep but am convinced that the NS build in
> particular will run into more serious problems.

Like what?  The NS port seems to work like before, i.e. no horizontal scroll bars.
Disabling and enabling them seems to be a no-op.

	Jan D.

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Sun, 27 Jul 2014 15:32:38 +0200
> From: martin rudalics <rudalics@gmx.at>
> 
>  > (4) Add horizontal scroll bars for the Gtk/Motif/Lucid/Windows builds.

Thanks!

Please remove the TODO item for this.

> I have committed the changes now.  Although I gave them some thorough
> testing with the Gtk/Lucid/Motif and Windows builds here, they very
> likely might break with other window managers or toolkits.  I did some
> light tests with GNUstep but am convinced that the NS build in
> particular will run into more serious problems.

The horizontal scrolling behaves strangely/incorrectly and erratically
with bidirectional text, especially when some paragraphs are L2R and
others R2L.  This aspect was never discussed in detail (I didn't even
know you were working on this), so I'm not surprised it doesn't DTRT.

Re: Changes in frame/window code

[Stefan Monnier]

> I intend to disable horizontal scrollbars by default in the next weeks.

How 'bout hiding the scrollbar when all lines are fully displayed?


        Stefan

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Sun, 27 Jul 2014 17:50:40 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> The horizontal scrolling behaves strangely/incorrectly and erratically
> with bidirectional text, especially when some paragraphs are L2R and
> others R2L.

I added something simple to make the issue less painful.

Re: Changes in frame/window code

[martin rudalics]

 > Why did you remove unrelated code from nsterm.m having to do with antialias threshold?  Do you not check whats changed before you checkin?

Sorry for that.  It crept into one of the changes I merged back from my
other machine.  Thank you very much for fixing this.  Hopefully, it's
the only mistake I made in this area.  Please have one more look.

 > Like what?  The NS port seems to work like before, i.e. no horizontal scroll bars.
 > Disabling and enabling them seems to be a no-op.

Sure.  But the frame size adjustments might cause problems.

martin

Re: Changes in frame/window code

[martin rudalics]

 > The horizontal scrolling behaves strangely/incorrectly and erratically
 > with bidirectional text, especially when some paragraphs are L2R and
 > others R2L.  This aspect was never discussed in detail (I didn't even
 > know you were working on this), so I'm not surprised it doesn't DTRT.

Arrgh...  Have you really fixed this already?

martin

Re: Changes in frame/window code

[martin rudalics]

 > How 'bout hiding the scrollbar when all lines are fully displayed?

I spent a couple of hours working on that and gave up.  It's pretty
difficult to do because we don't draw scrollbars on top but reserve in
advance the space for them.  But I'll give it another look.

martin

Re: Changes in frame/window code

[martin rudalics]

 >> The horizontal scrolling behaves strangely/incorrectly and erratically
 >> with bidirectional text, especially when some paragraphs are L2R and
 >> others R2L.
 >
 > I added something simple to make the issue less painful.

Sounds pretty hairy, tho ...  With bidi text as above do you have a
fixed text width?  In which (virtual L2R) column does R2L text start?

martin

Re: Changes in frame/window code

[Glenn Morris]

Looks like it fails --without-x:

http://hydra.nixos.org/build/12785148/log/raw

frame.o: In function `make_frame':
frame.o: In function `Fscroll_bar_height':
frame.c:2829: undefined reference to `FRAME_HAS_HORIZONTAL_SCROLL_BARS'
frame.c:221: undefined reference to `get_frame_param'

etc.

Re: Changes in frame/window code

[Dmitry Antipov]

[-- Attachment #1: Type: text/plain, Size: 1302 bytes --]

1) What about different scroll bar colors on Lucid?  If that matters, this is Xaw3d-1.6.2.

2) Did you ever run 'make check'?  I tried and got 55 core files with the same crash backtrace:

#0  0x000000379220f62b in raise (sig=11) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:37
#1  0x00000000005738ba in terminate_due_to_signal (sig=11, backtrace_limit=40) at ../../trunk/src/emacs.c:387
#2  0x000000000059da36 in handle_fatal_signal (sig=11) at ../../trunk/src/sysdep.c:1697
#3  0x000000000059da0b in deliver_thread_signal (sig=11, handler=0x59da1c <handle_fatal_signal>) at ../../trunk/src/sysdep.c:1671
#4  0x000000000059da6c in deliver_fatal_thread_signal (sig=11) at ../../trunk/src/sysdep.c:1709
#5  <signal handler called>
#6  0x0000000000428979 in adjust_frame_size (f=0xb9deb8, new_width=-1, new_height=-1, inhibit=1, pretend=false)
     at ../../trunk/src/frame.c:490
#7  0x00000000004c24ae in Fset_window_configuration (configuration=...) at ../../trunk/src/window.c:6371
#8  0x000000000061d8f3 in Ffuncall (nargs=2, args=0x7fff084ea648) at ../../trunk/src/eval.c:2808
#9  0x0000000000667e33 in exec_byte_code (bytestr=..., vector=..., maxdepth=..., args_template=..., nargs=0, args=0x7fff084eae78)
     at ../../trunk/src/bytecode.c:918

(Note -1 new_width and new_height at #6).

Dmitry


[-- Attachment #2: lucid.png --]
[-- Type: image/png, Size: 27536 bytes --]

Re: Changes in frame/window code

[martin rudalics]

 >> Sounds pretty hairy, tho ...  With bidi text as above do you have a
 >> fixed text width?
 >
 > I said "simple", didn't I?  The width is not fixed.
 >
 >> In which (virtual L2R) column does R2L text start?
 >
 > Sorry, I don't understand the question.  Are you asking about the
 > "normal" display of R2L text, or are you asking about scrolling?

If you have


L2R text here ....

            ..... and R2L text here what is the L2R position after here?


If the text width is not fixed how can I calculate the width of a R2L
line as it is displayed including any space between the left edge of the
display until where the text ends?

 > What I added only works well if the entire visible portion of the
 > buffer has the same paragraph direction, either L2R or R2L.  If you
 > have paragraphs in both directions visible, they will scroll in
 > opposite directions (because columns are counted in logical order, not
 > in visual order).

IIUC this means that the slider width must be calculated paragraph-wise
and position and size of the slider are meaningful only with respect to
the paragraph point is currently in.

 > Horizontal scrolling of mixed-direction paragraphs is a hard problem.
 > A WYSIWYG word processor could behave as if the text width were fixed
 > (that's what happens on paper), but Emacs is not a WYSIWYG word
 > processor and supports line continuation.

Admittedly, horizontal scroll bars are not very useful when line
continuation is on.

 > So it's not clear what to
 > do here.  The code I added is a stop-gap: it at least will DTRT when
 > the text on screen has the same base direction.  We must have at least
 > that, or we will be lynched.

Does it work now in that case?

martin

Re: Changes in frame/window code

[martin rudalics]

 > 1) What about different scroll bar colors on Lucid?  If that matters, this is Xaw3d-1.6.2.

Hmmm... how to reproduce that here?  Basically, you are the first person
on this list I would ask for a fix if I ever ran into such a problem.

 > 2) Did you ever run 'make check'?

Well, I did so now for the first time.  What does 'make check' do in
principle?  That is, what kind of build does it use?

 > I tried and got 55 core files with the same crash backtrace:
 >
 > #0  0x000000379220f62b in raise (sig=11) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:37
 > #1  0x00000000005738ba in terminate_due_to_signal (sig=11, backtrace_limit=40) at ../../trunk/src/emacs.c:387
 > #2  0x000000000059da36 in handle_fatal_signal (sig=11) at ../../trunk/src/sysdep.c:1697
 > #3  0x000000000059da0b in deliver_thread_signal (sig=11, handler=0x59da1c <handle_fatal_signal>) at ../../trunk/src/sysdep.c:1671
 > #4  0x000000000059da6c in deliver_fatal_thread_signal (sig=11) at ../../trunk/src/sysdep.c:1709
 > #5  <signal handler called>
 > #6  0x0000000000428979 in adjust_frame_size (f=0xb9deb8, new_width=-1, new_height=-1, inhibit=1, pretend=false)
 >      at ../../trunk/src/frame.c:490
 > #7  0x00000000004c24ae in Fset_window_configuration (configuration=...) at ../../trunk/src/window.c:6371
 > #8  0x000000000061d8f3 in Ffuncall (nargs=2, args=0x7fff084ea648) at ../../trunk/src/eval.c:2808
 > #9  0x0000000000667e33 in exec_byte_code (bytestr=..., vector=..., maxdepth=..., args_template=..., nargs=0, args=0x7fff084eae78)
 >      at ../../trunk/src/bytecode.c:918
 >
 > (Note -1 new_width and new_height at #6).

These values are perfectly legitimate.  Can this be related to the
without-x issue?

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Mon, 28 Jul 2014 11:26:02 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  >> Sounds pretty hairy, tho ...  With bidi text as above do you have a
>  >> fixed text width?
>  >
>  > I said "simple", didn't I?  The width is not fixed.
>  >
>  >> In which (virtual L2R) column does R2L text start?
>  >
>  > Sorry, I don't understand the question.  Are you asking about the
>  > "normal" display of R2L text, or are you asking about scrolling?
> 
> If you have
> 
> 
> L2R text here ....
> 
>             ..... and R2L text here what is the L2R position after here?
> 
> 
> If the text width is not fixed how can I calculate the width of a R2L
> line as it is displayed including any space between the left edge of the
> display until where the text ends?

If you include the space on the left, then the width of an R2L display
line is always exactly equal to the window width.  This is how the
display engine computes the width of the stretch glyph that represents
"the space on the left".

If you want the line width without the stretch, then you calculate it
exactly like with L2R lines: columns and line widths are computed in
the logical order.

>  > What I added only works well if the entire visible portion of the
>  > buffer has the same paragraph direction, either L2R or R2L.  If you
>  > have paragraphs in both directions visible, they will scroll in
>  > opposite directions (because columns are counted in logical order, not
>  > in visual order).
> 
> IIUC this means that the slider width must be calculated paragraph-wise
> and position and size of the slider are meaningful only with respect to
> the paragraph point is currently in.

Yes.  But that is true for strictly L2R text as well, no?  I mean, the
size of the scroll-bar thumb should depend on the width of the line at
point, right?

>  > Horizontal scrolling of mixed-direction paragraphs is a hard problem.
>  > A WYSIWYG word processor could behave as if the text width were fixed
>  > (that's what happens on paper), but Emacs is not a WYSIWYG word
>  > processor and supports line continuation.
> 
> Admittedly, horizontal scroll bars are not very useful when line
> continuation is on.

Of course, but that's not what I meant.  I meant to point out how
Emacs in its default text display modes differs from WYSIWYG word
processors, where fixed width of the text comes in naturally.

>  > So it's not clear what to
>  > do here.  The code I added is a stop-gap: it at least will DTRT when
>  > the text on screen has the same base direction.  We must have at least
>  > that, or we will be lynched.
> 
> Does it work now in that case?

It does work correctly when you click on the scroll-bar arrows or
between them and the slider.

It doesn't work when you drag the slider.  When you drag the slider,
the text is scrolled in the wrong direction.  The underlying problem
is that the slider starts in a wrong position -- it should be all the
way to the right, not to the left.  AFAIU, this can only be fixed on
the C level.

(Btw, shouldn't GTK scroll bars already support bidirectional text out
of the box?  Perhaps you need to set some flag(s) to get that.)

Re: Changes in frame/window code

[martin rudalics]

 > If you include the space on the left, then the width of an R2L display
 > line is always exactly equal to the window width.  This is how the
 > display engine computes the width of the stretch glyph that represents
 > "the space on the left".

But if that R2L line starts on the right of the right window edge it's
larger, I presume.

 > If you want the line width without the stretch, then you calculate it
 > exactly like with L2R lines: columns and line widths are computed in
 > the logical order.

Hmmm...  That would mean my calculations in

       whole = move_it_to (&it, -1, INT_MAX, window_box_height (w), -1,
			  MOVE_TO_X | MOVE_TO_Y);

of set_horizontal_scroll_bar have a flaw.

 >> IIUC this means that the slider width must be calculated paragraph-wise
 >> and position and size of the slider are meaningful only with respect to
 >> the paragraph point is currently in.
 >
 > Yes.  But that is true for strictly L2R text as well, no?  I mean, the
 > size of the scroll-bar thumb should depend on the width of the line at
 > point, right?

No.  The slider size repesents the width of the window with respect to
the longest line shown in the window.  Try moving point through the
first column within a window - the slider size should not change.

 > It doesn't work when you drag the slider.  When you drag the slider,
 > the text is scrolled in the wrong direction.  The underlying problem
 > is that the slider starts in a wrong position -- it should be all the
 > way to the right, not to the left.  AFAIU, this can only be fixed on
 > the C level.

Is the size of the slider correct in the sense described above?  Then
fixing the position should not be that difficult.

 > (Btw, shouldn't GTK scroll bars already support bidirectional text out
 > of the box?  Perhaps you need to set some flag(s) to get that.)

No idea.  In any case I would have to tell GTK whether the "current
text" (whatever that is) is L2R or R2L I suppose.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Mon, 28 Jul 2014 15:57:15 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  > If you include the space on the left, then the width of an R2L display
>  > line is always exactly equal to the window width.  This is how the
>  > display engine computes the width of the stretch glyph that represents
>  > "the space on the left".
> 
> But if that R2L line starts on the right of the right window edge it's
> larger, I presume.

You mean, if it's hscrolled?  Yes, of course -- exactly like an L2R
line.

I feel there's some misunderstanding here, because I don't believe
you'd ask about such trivia.  What am I missing?  What's bothering
you?

Perhaps it will help if I say (something that should be obvious, but
maybe it's only obvious to me...) that an R2L line behaves wrt
hscrolling the same as an L2R line, except that it is reversed on
display.  E.g., when the amount of hscroll gets larger, and R2L line
moves to the right, thus removing from sight the first (rightmost on
display) characters of the line.

>  > If you want the line width without the stretch, then you calculate it
>  > exactly like with L2R lines: columns and line widths are computed in
>  > the logical order.
> 
> Hmmm...  That would mean my calculations in
> 
>        whole = move_it_to (&it, -1, INT_MAX, window_box_height (w), -1,
> 			  MOVE_TO_X | MOVE_TO_Y);
> 
> of set_horizontal_scroll_bar have a flaw.

No, I don't think there's a flaw.  The move_it_* family of functions
doesn't know about reversal of R2L lines on display, they think an R2L
line is displayed starting at the left margin of the window.  See the
commentary about that at the beginning of xdisp.c.

>  >> IIUC this means that the slider width must be calculated paragraph-wise
>  >> and position and size of the slider are meaningful only with respect to
>  >> the paragraph point is currently in.
>  >
>  > Yes.  But that is true for strictly L2R text as well, no?  I mean, the
>  > size of the scroll-bar thumb should depend on the width of the line at
>  > point, right?
> 
> No.  The slider size repesents the width of the window with respect to
> the longest line shown in the window.  Try moving point through the
> first column within a window - the slider size should not change.

Then I don't see why you think this should change when there's
bidirectional text in the buffer.  The same arguments work in that
case as well.  R2L lines do not affect the window width in any way
that L2R lines don't.

>  > It doesn't work when you drag the slider.  When you drag the slider,
>  > the text is scrolled in the wrong direction.  The underlying problem
>  > is that the slider starts in a wrong position -- it should be all the
>  > way to the right, not to the left.  AFAIU, this can only be fixed on
>  > the C level.
> 
> Is the size of the slider correct in the sense described above?

Yes.

> Then fixing the position should not be that difficult.

I never said it was difficult, just that it has to be on the C level,
not on the Lisp level, where I fixed the clicks on the scroll bar.

>  > (Btw, shouldn't GTK scroll bars already support bidirectional text out
>  > of the box?  Perhaps you need to set some flag(s) to get that.)
> 
> No idea.

Maybe Jan (CC'ed) can help us out.

> In any case I would have to tell GTK whether the "current text"
> (whatever that is) is L2R or R2L I suppose.

Yes, but we have current-bidi-paragraph-direction for that.

Re: Changes in frame/window code

[martin rudalics]

 >> But if that R2L line starts on the right of the right window edge it's
 >> larger, I presume.
 >
 > You mean, if it's hscrolled?  Yes, of course -- exactly like an L2R
 > line.

I still have no feeling for how hscrolling works with bidi text.  IIUC
with L2R and R2L paragraphs in the same window, like

LLLLLLLLLLLLLLLLLLLLLL2RRRRRRRRRRRRRRRRRRR

                RRRRRRR2LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL

hscrolling will NOT preserve the aspect that the two "2" visually appear
above each other.  Correct?

 > I feel there's some misunderstanding here, because I don't believe
 > you'd ask about such trivia.  What am I missing?  What's bothering
 > you?

What you said earlier, that "Horizontal scrolling of mixed-direction
paragraphs is a hard problem".

 >> Is the size of the slider correct in the sense described above?
 >
 > Yes.

Did you check with a window containg say one huge L2R line and all
other lines short and one huge R2L line and all other lines short?

 >> Then fixing the position should not be that difficult.
 >
 > I never said it was difficult, just that it has to be on the C level,
 > not on the Lisp level, where I fixed the clicks on the scroll bar.

Sure.

 >> In any case I would have to tell GTK whether the "current text"
 >> (whatever that is) is L2R or R2L I suppose.
 >
 > Yes, but we have current-bidi-paragraph-direction for that.

The whole idea would be then to change the positions of

       start = w->hscroll * FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w));

and

       end = start + box_width;

on lines 15808 and 15810 of xdisp.c according to the value of
`current-bidi-paragraph-direction'.  But how?

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Mon, 28 Jul 2014 19:27:34 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
> I still have no feeling for how hscrolling works with bidi text.  IIUC
> with L2R and R2L paragraphs in the same window, like
> 
> LLLLLLLLLLLLLLLLLLLLLL2RRRRRRRRRRRRRRRRRRR
> 
>                 RRRRRRR2LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL
> 
> hscrolling will NOT preserve the aspect that the two "2" visually appear
> above each other.  Correct?

Yes.  As the hscroll amount grows, the characters in the L2R lines
will move to the left, while characters in the R2L lines will move to
the right.

>  > I feel there's some misunderstanding here, because I don't believe
>  > you'd ask about such trivia.  What am I missing?  What's bothering
>  > you?
> 
> What you said earlier, that "Horizontal scrolling of mixed-direction
> paragraphs is a hard problem".

It's a hard problem primarily because it's hard to know what is TRT,
and there's no "prior art" to follow.

>  >> Is the size of the slider correct in the sense described above?
>  >
>  > Yes.
> 
> Did you check with a window containg say one huge L2R line and all
> other lines short and one huge R2L line and all other lines short?

Yes.  Again, this is just normal hscroll, which works with
bidirectional text since v24.1.  The fact that hscroll is triggered by
clicking the scroll bar does not matter, as long as the direction of
the scroll is interpreted correctly.  The changes I made simply took
care of translating the scroll-bar clicks to hscroll amount.

> The whole idea would be then to change the positions of
> 
>        start = w->hscroll * FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w));
> 
> and
> 
>        end = start + box_width;
> 
> on lines 15808 and 15810 of xdisp.c according to the value of
> `current-bidi-paragraph-direction'.  But how?

You need to reverse the meaning of START and END for the R2L case:

   end = whole - w->hscroll * FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w));
   start = end - box_width;

(Note that current-bidi-paragraph-direction returns the results at
buffer's point, so you will need to temporarily go to the window's
point marker.)

Hint on Xaw3d scroll bar colors issue [Was: Re: Changes in frame/window code]

[Dmitry Antipov]

On 07/28/2014 01:27 PM, martin rudalics wrote:

> Hmmm... how to reproduce that here?  Basically, you are the first person
> on this list I would ask for a fix if I ever ran into such a problem.

This is what I'm doing:

In Xaw3d sources, find these functions and install breakpoints
at [HERE]:

static void
AllocTopShadowPixel (Widget new)
{
     XColor set_c;
     ThreeDWidget tdw = (ThreeDWidget) new;
     Display *dpy = XtDisplay (new);
     Colormap cmap = new->core.colormap;

     Xaw3dComputeTopShadowRGB (new, &set_c);
     (void) XAllocColor (dpy, cmap, &set_c);
     tdw->threeD.top_shadow_pixel = set_c.pixel; /* HERE */
}

static void
AllocBotShadowPixel (Widget new)
{
     XColor set_c;
     ThreeDWidget tdw = (ThreeDWidget) new;
     Display *dpy = XtDisplay (new);
     Colormap cmap = new->core.colormap;

     Xaw3dComputeBottomShadowRGB (new, &set_c);
     (void) XAllocColor (dpy, cmap, &set_c);
     tdw->threeD.bot_shadow_pixel = set_c.pixel; /* HERE */
}

Next install breakpoints on x_create_toolkit_scroll_bar and
x_create_horizontal_toolkit_scroll_bar and trace calls to
AllocTopShadowPixel/AllocBotShadowPixel from these functions.

I'm seeing that for the vertical scroll bar, top_shadow_pixel
is 0xe6e6e6 and bot_shadow_pixel is 0x737373.  But for the
horizontal one, top_shadow_pixel is 0xcccccc and bot_shadow_pixel
is 0x666666.  No ideas why they're different.  Funny old toolkits.

Can you replay this debugging sequence and check your top_shadow_pixel
and bot_shadow_pixel values for both scroll bars?

Dmitry

Re: Changes in frame/window code

[martin rudalics]

 > 2) Did you ever run 'make check'?  I tried and got 55 core files with the same crash backtrace:
 >
 > #0  0x000000379220f62b in raise (sig=11) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:37
 > #1  0x00000000005738ba in terminate_due_to_signal (sig=11, backtrace_limit=40) at ../../trunk/src/emacs.c:387
 > #2  0x000000000059da36 in handle_fatal_signal (sig=11) at ../../trunk/src/sysdep.c:1697
 > #3  0x000000000059da0b in deliver_thread_signal (sig=11, handler=0x59da1c <handle_fatal_signal>) at ../../trunk/src/sysdep.c:1671
 > #4  0x000000000059da6c in deliver_fatal_thread_signal (sig=11) at ../../trunk/src/sysdep.c:1709
 > #5  <signal handler called>
 > #6  0x0000000000428979 in adjust_frame_size (f=0xb9deb8, new_width=-1, new_height=-1, inhibit=1, pretend=false)
 >      at ../../trunk/src/frame.c:490
 > #7  0x00000000004c24ae in Fset_window_configuration (configuration=...) at ../../trunk/src/window.c:6371
 > #8  0x000000000061d8f3 in Ffuncall (nargs=2, args=0x7fff084ea648) at ../../trunk/src/eval.c:2808
 > #9  0x0000000000667e33 in exec_byte_code (bytestr=..., vector=..., maxdepth=..., args_template=..., nargs=0, args=0x7fff084eae78)
 >      at ../../trunk/src/bytecode.c:918

Should be fixed now.  Can you please check again?

Thanks, martin

Re: Changes in frame/window code

[martin rudalics]

 >> I still have no feeling for how hscrolling works with bidi text.  IIUC
 >> with L2R and R2L paragraphs in the same window, like
 >>
 >> LLLLLLLLLLLLLLLLLLLLLL2RRRRRRRRRRRRRRRRRRR
 >>
 >>                RRRRRRR2LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL
 >>
 >> hscrolling will NOT preserve the aspect that the two "2" visually appear
 >> above each other.  Correct?
 >
 > Yes.  As the hscroll amount grows, the characters in the L2R lines
 > will move to the left, while characters in the R2L lines will move to
 > the right.

I have to think about the possible implications of this.  Maybe there
are none but putting this model into my brain will take some time.

 > It's a hard problem primarily because it's hard to know what is TRT,
 > and there's no "prior art" to follow.

Because all the others keep the "2"s from my example fixed above each
other, I presume.

 >> Did you check with a window containg say one huge L2R line and all
 >> other lines short and one huge R2L line and all other lines short?
 >
 > Yes.  Again, this is just normal hscroll, which works with
 > bidirectional text since v24.1.  The fact that hscroll is triggered by
 > clicking the scroll bar does not matter, as long as the direction of
 > the scroll is interpreted correctly.  The changes I made simply took
 > care of translating the scroll-bar clicks to hscroll amount.

I didn't mean that you should check whether hscroll works.  What I meant
was to check whether the slider/thumb size really relates to the size of
the longest line of text, disregarding the fact that it is L2R or R2L.

 > You need to reverse the meaning of START and END for the R2L case:
 >
 >     end = whole - w->hscroll * FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w));
 >     start = end - box_width;
 >
 > (Note that current-bidi-paragraph-direction returns the results at
 > buffer's point, so you will need to temporarily go to the window's
 > point marker.)

As soon as you have some spare time (I know you never do) please try it.

martin

Re: Hint on Xaw3d scroll bar colors issue [Was: Re: Changes in frame/window code]

[martin rudalics]

 > In Xaw3d sources, find these functions and install breakpoints
 > at [HERE]:

Where can an ignorant like me find the sources (I'm on Debian)?

 > static void
 > AllocTopShadowPixel (Widget new)
 > {
 >      XColor set_c;
 >      ThreeDWidget tdw = (ThreeDWidget) new;
 >      Display *dpy = XtDisplay (new);
 >      Colormap cmap = new->core.colormap;
 >
 >      Xaw3dComputeTopShadowRGB (new, &set_c);
 >      (void) XAllocColor (dpy, cmap, &set_c);
 >      tdw->threeD.top_shadow_pixel = set_c.pixel; /* HERE */
 > }
 >
 > static void
 > AllocBotShadowPixel (Widget new)
 > {
 >      XColor set_c;
 >      ThreeDWidget tdw = (ThreeDWidget) new;
 >      Display *dpy = XtDisplay (new);
 >      Colormap cmap = new->core.colormap;
 >
 >      Xaw3dComputeBottomShadowRGB (new, &set_c);
 >      (void) XAllocColor (dpy, cmap, &set_c);
 >      tdw->threeD.bot_shadow_pixel = set_c.pixel; /* HERE */
 > }
 >
 > Next install breakpoints on x_create_toolkit_scroll_bar and
 > x_create_horizontal_toolkit_scroll_bar and trace calls to
 > AllocTopShadowPixel/AllocBotShadowPixel from these functions.
 >
 > I'm seeing that for the vertical scroll bar, top_shadow_pixel
 > is 0xe6e6e6 and bot_shadow_pixel is 0x737373.  But for the
 > horizontal one, top_shadow_pixel is 0xcccccc and bot_shadow_pixel
 > is 0x666666.  No ideas why they're different.  Funny old toolkits.

Did you trace Xaw3dComputeTopShadowRGB and Xaw3dComputeBottomShadowRGB
why they compute different values?  Or XAllocColor?

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Tue, 29 Jul 2014 11:20:24 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
>  >> I still have no feeling for how hscrolling works with bidi text.  IIUC
>  >> with L2R and R2L paragraphs in the same window, like
>  >>
>  >> LLLLLLLLLLLLLLLLLLLLLL2RRRRRRRRRRRRRRRRRRR
>  >>
>  >>                RRRRRRR2LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL
>  >>
>  >> hscrolling will NOT preserve the aspect that the two "2" visually appear
>  >> above each other.  Correct?
>  >
>  > Yes.  As the hscroll amount grows, the characters in the L2R lines
>  > will move to the left, while characters in the R2L lines will move to
>  > the right.
> 
> I have to think about the possible implications of this.  Maybe there
> are none but putting this model into my brain will take some time.

As long as we use logical-order columns, there really isn't any
alternative to this behavior.

>  > It's a hard problem primarily because it's hard to know what is TRT,
>  > and there's no "prior art" to follow.
> 
> Because all the others keep the "2"s from my example fixed above each
> other, I presume.

Yes, because all the others (those I know of) are WYSIWYG word
processors, where the page size has a well-defined meaning and is
fixed within a given document.

> I didn't mean that you should check whether hscroll works.  What I meant
> was to check whether the slider/thumb size really relates to the size of
> the longest line of text, disregarding the fact that it is L2R or R2L.

Line width calculations, like column calculations, are unaffected by
bidi.  These calculations are done using buffer text in its logical
order, so the way text is displayed cannot have any effect on that.

So if we can correctly compute the length of the longest line at all,
we can do it with any text, bidi or not.  Therefore, the size of the
thumb will always be true (and is, I checked that, of course).

>  > You need to reverse the meaning of START and END for the R2L case:
>  >
>  >     end = whole - w->hscroll * FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w));
>  >     start = end - box_width;
>  >
>  > (Note that current-bidi-paragraph-direction returns the results at
>  > buffer's point, so you will need to temporarily go to the window's
>  > point marker.)
> 
> As soon as you have some spare time (I know you never do) please try it.

I can only try this on MS-Windows.

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Tue, 29 Jul 2014 12:41:02 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
> >  > You need to reverse the meaning of START and END for the R2L case:
> >  >
> >  >     end = whole - w->hscroll * FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w));
> >  >     start = end - box_width;
> >  >
> >  > (Note that current-bidi-paragraph-direction returns the results at
> >  > buffer's point, so you will need to temporarily go to the window's
> >  > point marker.)
> > 
> > As soon as you have some spare time (I know you never do) please try it.
> 
> I can only try this on MS-Windows.

Btw, I'm not sure I follow the logic of this snippet:


      /* The following is needed to ensure that if after maximizing a
	 window we get hscroll > 0, we can still drag the thumb to the
	 left.  */
      whole = max (whole, w->hscroll + box_width);
      whole = max (whole, end - start);

First, hscroll is in columns, while box_width is in pixels, so adding
them sounds incorrect.

Second, what problem does this try to solve, and why would this
double-maxing solve it?

And third, since end - start is exactly equal to box_width, why do you
need the second max?  What am I missing?

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Tue, 29 Jul 2014 12:41:02 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
> >  > You need to reverse the meaning of START and END for the R2L case:
> >  >
> >  >     end = whole - w->hscroll * FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w));
> >  >     start = end - box_width;
> >  >
> >  > (Note that current-bidi-paragraph-direction returns the results at
> >  > buffer's point, so you will need to temporarily go to the window's
> >  > point marker.)
> > 
> > As soon as you have some spare time (I know you never do) please try it.
> 
> I can only try this on MS-Windows.

The patch below almost works.  "Almost" because of 2 issues that still
need to be taken care of:

 . As soon as I click on the scroll-bar thumb, which is correctly
   drawn flushed all the way to the right when point is an a R2L line,
   the scroll bar "jumps" to the left edge.  Where's the code which
   does that?

 . The code below is for the selected window, where point is up to
   date.  But what about non-selected windows, should we use
   window-point there?

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2014-07-27 13:21:30 +0000
+++ src/xdisp.c	2014-07-29 10:17:58 +0000
@@ -15814,6 +15814,17 @@ set_horizontal_scroll_bar (struct window
 	 left.  */
       whole = max (whole, w->hscroll + box_width);
       whole = max (whole, end - start);
+      if (it.bidi_p)
+	{
+	  Lisp_Object pdir;
+
+	  pdir = Fcurrent_bidi_paragraph_direction (Qnil);
+	  if (EQ (pdir, Qright_to_left))
+	    {
+	      end = whole - w->hscroll * FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w));
+	      start = end - box_width;
+	    }
+	}
 
       if (old_buffer)
 	set_buffer_internal (old_buffer);

Re: Changes in frame/window code

[martin rudalics]

 > As long as we use logical-order columns, there really isn't any
 > alternative to this behavior.

I doubt it can be improved.  After all it seems to make better use of
the available screen space.

 > Yes, because all the others (those I know of) are WYSIWYG word
 > processors, where the page size has a well-defined meaning and is
 > fixed within a given document.

I do understand that now.

 > So if we can correctly compute the length of the longest line at all,
 > we can do it with any text, bidi or not.  Therefore, the size of the
 > thumb will always be true (and is, I checked that, of course).

Thanks.

 > I can only try this on MS-Windows.

Thanks.  I'll try to translate it to the other builds then.

martin

Re: Changes in frame/window code

[martin rudalics]

 > Btw, I'm not sure I follow the logic of this snippet:
 >
 >
 >        /* The following is needed to ensure that if after maximizing a
 > 	 window we get hscroll > 0, we can still drag the thumb to the
 > 	 left.  */
 >        whole = max (whole, w->hscroll + box_width);
 >        whole = max (whole, end - start);
 >
 > First, hscroll is in columns, while box_width is in pixels, so adding
 > them sounds incorrect.
 >
 > Second, what problem does this try to solve, and why would this
 > double-maxing solve it?
 >
 > And third, since end - start is exactly equal to box_width, why do you
 > need the second max?  What am I missing?

IIUC what I wrote in my comment it seems that I had the following
problem: In a normal frame I had hscroll > 0 and after maximizing the
frame hscroll was still > 0 but the longest line was now less than the
width of the window so inherently it became impossible to drag the
scrollbar because the slider now occupied the full width.  This means I
could not scroll the text back to make its left part visible using the
scrollbar alone.  So I added this hack to do that.  But you observations
clearly show that there's some silliness in it.

martin

Re: Changes in frame/window code

[martin rudalics]

 > The patch below almost works.  "Almost" because of 2 issues that still
 > need to be taken care of:
 >
 >   . As soon as I click on the scroll-bar thumb, which is correctly
 >     drawn flushed all the way to the right when point is an a R2L line,
 >     the scroll bar "jumps" to the left edge.  Where's the code which
 >     does that?
 >
 >   . The code below is for the selected window, where point is up to
 >     date.  But what about non-selected windows, should we use
 >     window-point there?

Can you please send me some sample bidi text and some guidance to test
it.

Thanks, martin

Re: Hint on Xaw3d scroll bar colors issue [Was: Re: Changes in frame/window code]

[Dmitry Antipov]

On 07/29/2014 01:21 PM, martin rudalics wrote:

> Where can an ignorant like me find the sources (I'm on Debian)?

Not sure about Debian. Usually there should be a kind of a debug package.
On my Fedora 20 system, I have:

Xaw3d-devel-1.6.2-5.fc20.x86_64
Xaw3d-1.6.2-5.fc20.x86_64
Xaw3d-debuginfo-1.6.2-5.fc20.x86_64

The last one installs Xaw3d sources under /usr/src/debug/libXaw3d-1.6.2,
and gdb uses them while stepping through source code.

Do you use Xaw3d or Xaw? If any, what version?

If that matters, this is even more interesting debugging session:

(gdb) b x_create_toolkit_scroll_bar
Breakpoint 1 at 0x538f6c: file ../../trunk/src/xterm.c, line 4795.
(gdb) b x_create_horizontal_toolkit_scroll_bar
Breakpoint 2 at 0x5395a5: file ../../trunk/src/xterm.c, line 4994.
(gdb) r -Q
Breakpoint 1, x_create_toolkit_scroll_bar (f=0x1130ce0, bar=0x11c5998) at ../../trunk/src/xterm.c:4795
4795	  int ac = 0;
(gdb) b Xaw3dComputeTopShadowRGB
Breakpoint 3 at 0x7ffff7da6690: file ThreeD.c, line 313.
(gdb) b Xaw3dComputeBottomShadowRGB
Breakpoint 4 at 0x7ffff7da6860: file ThreeD.c, line 359.
(gdb) c
Continuing.

Breakpoint 3, Xaw3dComputeTopShadowRGB (new=new@entry=0xd0f220, xcol_out=xcol_out@entry=0x7fffffffa0d0) at ThreeD.c:313
313	{
(gdb) n
314	    if (XtIsSubclass (new, threeDWidgetClass)) {
(gdb)
318		Display *dpy = XtDisplay (new);
(gdb)
322		get_c.pixel = tdw->core.background_pixel;
(gdb) p /x tdw->core.background_pixel
$1 = 0xbfbfbf
(gdb) c
Continuing.

Breakpoint 4, Xaw3dComputeBottomShadowRGB (new=new@entry=0xd0f220, xcol_out=xcol_out@entry=0x7fffffffa0d0) at ThreeD.c:359
359	{
(gdb) n
360	    if (XtIsSubclass (new, threeDWidgetClass)) {
(gdb)
364		Display *dpy = XtDisplay (new);
(gdb)
368		get_c.pixel = tdw->core.background_pixel;
(gdb) p /x tdw->core.background_pixel
$2 = 0xbfbfbf

So, vertical scroll bar's Core widget has background pixel 0xbfbfbf.

Next, we create horizontal scroll bar and see:

Breakpoint 2, x_create_horizontal_toolkit_scroll_bar (f=0x1130ce0, bar=0x11c6908) at ../../trunk/src/xterm.c:4994
4994	  int ac = 0;
(gdb) c
Continuing.

Breakpoint 3, Xaw3dComputeTopShadowRGB (new=new@entry=0xbe6a70, xcol_out=xcol_out@entry=0x7fffffff93a0) at ThreeD.c:313
313	{
(gdb) n
314	    if (XtIsSubclass (new, threeDWidgetClass)) {
(gdb)
318		Display *dpy = XtDisplay (new);
(gdb)
322		get_c.pixel = tdw->core.background_pixel;
(gdb) p /x tdw->core.background_pixel
$3 = 0xffffff
(gdb) c
Continuing.

Breakpoint 4, Xaw3dComputeBottomShadowRGB (new=new@entry=0xbe6a70, xcol_out=xcol_out@entry=0x7fffffff93a0) at ThreeD.c:359
359	{
(gdb) n
360	    if (XtIsSubclass (new, threeDWidgetClass)) {
(gdb)
364		Display *dpy = XtDisplay (new);
(gdb)
368		get_c.pixel = tdw->core.background_pixel;
(gdb) p /x tdw->core.background_pixel
$4 = 0xffffff

Here Core's background pixel is 0xffffff and so colors calculated by
Xaw3dComputeTopShadowRGB and Xaw3dComputeBottomShadowRGB for top_shadow_pixel
and bot_shadow_pixel of our new shiny horizontal scroll bar are very
different from the corresponding colors of vertical scroll bar.

I can use gdb to set horizontal scroll bar Core's tdw->core.background_pixel
to 0xbfbfbf and finally see horizontal scroll bar in an expected color.

BTW, x_default_scroll_bar_color_parameter in xfns.c should take horizontal
scroll bar into account, isn't it?

Dmitry

Re: Changes in frame/window code

[Dmitry Antipov]

On 07/29/2014 01:19 PM, martin rudalics wrote:

> Should be fixed now.  Can you please check again?

As of r117606, 'make check' works again.

Dmitry

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Tue, 29 Jul 2014 12:47:55 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
> Can you please send me some sample bidi text and some guidance to test
> it.

Just type "C-u C-h t hebrew RET", and you have your sample.  Most of
it is R2L lines, with some L2R lines at the end.

As for testing, I think it's quite straightforward:

 . the initial position of the thumb in a R2L line should be all the
   way to the left

 . clicking on the arrows at scroll-bar edges should cause the text to
   move in the direction you'd expect: to the right when you click on
   the arrow pointing leftward, and to the left when you click the
   other arrow.

 . likewise dragging the thumb: it should cause text to move in the
   direction opposite to the one you drag

You might see some abnormal scrolling in R2L lines that begin with a
TAB character.  This is a bug in hscrolling such lines, which I
already fixed on the emacs-24 branch, so it will soon be merged to the
trunk.  It has nothing to do with the horizontal scroll bars.

Re: Hint on Xaw3d scroll bar colors issue [Was: Re: Changes in frame/window code]

[martin rudalics]

 > Not sure about Debian. Usually there should be a kind of a debug package.
 > On my Fedora 20 system, I have:
 >
 > Xaw3d-devel-1.6.2-5.fc20.x86_64
 > Xaw3d-1.6.2-5.fc20.x86_64
 > Xaw3d-debuginfo-1.6.2-5.fc20.x86_64

I have

xaw3dg     1.5+E-18.2
xaw3dg-dev 1.5+E-18.2

Synaptic doesn't offer any debug package for xaw3d.

 > The last one installs Xaw3d sources under /usr/src/debug/libXaw3d-1.6.2,
 > and gdb uses them while stepping through source code.

These are obviously absent here.

 > Do you use Xaw3d or Xaw? If any, what version?
 >
 > If that matters, this is even more interesting debugging session:
[...]
 > So, vertical scroll bar's Core widget has background pixel 0xbfbfbf.
[...]
 > Here Core's background pixel is 0xffffff and so colors calculated by
 > Xaw3dComputeTopShadowRGB and Xaw3dComputeBottomShadowRGB for top_shadow_pixel
 > and bot_shadow_pixel of our new shiny horizontal scroll bar are very
 > different from the corresponding colors of vertical scroll bar.
 >
 > I can use gdb to set horizontal scroll bar Core's tdw->core.background_pixel
 > to 0xbfbfbf and finally see horizontal scroll bar in an expected color.

So there's nothing we even _could_ do here?  Do you have any other
application built with Lucid that has both types of scrollbars?

 > BTW, x_default_scroll_bar_color_parameter in xfns.c should take horizontal
 > scroll bar into account, isn't it?

Hmmm...  I missed the build_string ("verticalScrollBar") there.  Could
that be the cause?  Can you fix it in any case (if I do it, it would be
yet another exercise in trial and error ...).

martin

Re: Changes in frame/window code

[martin rudalics]

 > Just type "C-u C-h t hebrew RET", and you have your sample.  Most of
 > it is R2L lines, with some L2R lines at the end.

OK.

 >  . As soon as I click on the scroll-bar thumb, which is correctly
 >    drawn flushed all the way to the right when point is an a R2L line,
 >    the scroll bar "jumps" to the left edge.  Where's the code which
 >    does that?

I see the problem - it must be with auto-hscroll suspension.  This can
be a bit hairy to get right.  The idea is that when you drag the slider
and `point' is about to go off screen, usually auto-hscroll kicks in and
moves the text back to the previous position to make `point' visible
again (note that dragging the slider is not supposed to move `point').
Now the suspend_auto_hscroll flag of a window, when set, is supposed to
prevent that.  For some reason this seems to backfire here.

  . The code below is for the selected window, where point is up to
    date.  But what about non-selected windows, should we use
    window-point there?

There's no other choice I suppose.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Tue, 29 Jul 2014 16:02:41 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
>  >  . As soon as I click on the scroll-bar thumb, which is correctly
>  >    drawn flushed all the way to the right when point is an a R2L line,
>  >    the scroll bar "jumps" to the left edge.  Where's the code which
>  >    does that?
> 
> I see the problem - it must be with auto-hscroll suspension.  This can
> be a bit hairy to get right.  The idea is that when you drag the slider
> and `point' is about to go off screen, usually auto-hscroll kicks in and
> moves the text back to the previous position to make `point' visible
> again (note that dragging the slider is not supposed to move `point').
> Now the suspend_auto_hscroll flag of a window, when set, is supposed to
> prevent that.  For some reason this seems to backfire here.

Why doesn't it backfire in the L2R case?  IOW, where's the code that
moves the slider back to the left edge of the scroll bar?

>   . The code below is for the selected window, where point is up to
>     date.  But what about non-selected windows, should we use
>     window-point there?
> 
> There's no other choice I suppose.

I tried that, but then the feature stopped working altogether.  Maybe
I used the window point marker incorrectly, I don't know.

Re: Changes in frame/window code

[Jan Djärv]

Hi.

28 jul 2014 kl. 16:25 skrev Eli Zaretskii <eliz@gnu.org>:

>>> (Btw, shouldn't GTK scroll bars already support bidirectional text out
>>> of the box?  Perhaps you need to set some flag(s) to get that.)
>> 
>> No idea.
> 
> Maybe Jan (CC'ed) can help us out.
> 
>> In any case I would have to tell GTK whether the "current text"
>> (whatever that is) is L2R or R2L I suppose.
> 
> Yes, but we have current-bidi-paragraph-direction for that.

Not out of the box, but 
   gtk_widget_set_direction (w, GTK_TEXT_DIR_RTL)
and
  gtk_range_set_flippable (w, TRUE);

where w is the scrollbar should do it according to the documentation.  I have not tried it.
gtk_range_set_flippable is since 2.18, so you would have to put in a check for that.

	Jan D.

Re: Hint on Xaw3d scroll bar colors issue [Was: Re: Changes in frame/window code]

[Jan Djärv]

Hi.

29 jul 2014 kl. 16:01 skrev martin rudalics <rudalics@gmx.at>:
> 
> > BTW, x_default_scroll_bar_color_parameter in xfns.c should take horizontal
> > scroll bar into account, isn't it?
> 
> Hmmm...  I missed the build_string ("verticalScrollBar") there.  Could
> that be the cause?  Can you fix it in any case (if I do it, it would be
> yet another exercise in trial and error ...).
> 

The color is read from the X resource database by the Xaw widget library.  The resource database is seeded in xrdb.c with (for example)

  sprintf (line, "Emacs*verticalScrollBar.background: grey75");

but when the horizontal scroll bar is created it has the name horizontalScrollBar, so no wonder the colors are different.  Adding corresponding lines in xrdb.c for horizontalScrollBar should do it (3 places).

	Jan D.

Re: Changes in frame/window code

[martin rudalics]

 > Why doesn't it backfire in the L2R case?  IOW, where's the code that
 > moves the slider back to the left edge of the scroll bar?

It's the code that resets the window's suspend_auto_hscroll flag to zero
provided the window's point is not visible at that moment.  Typical
examples are any form of text change or setting a window's buffer.

 > I tried that, but then the feature stopped working altogether.

In which sense?

 > Maybe
 > I used the window point marker incorrectly, I don't know.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Tue, 29 Jul 2014 17:43:08 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
>  > Why doesn't it backfire in the L2R case?  IOW, where's the code that
>  > moves the slider back to the left edge of the scroll bar?
> 
> It's the code that resets the window's suspend_auto_hscroll flag to zero
> provided the window's point is not visible at that moment.  Typical
> examples are any form of text change or setting a window's buffer.

I'm confused: how does that get called when I click a mouse on the
scroll-bar slider?  Just click, no drag.

>  > I tried that, but then the feature stopped working altogether.
> 
> In which sense?

The slider didn't jump to the right when point was in a R2L paragraph.

Re: Changes in frame/window code

[Eli Zaretskii]

> From: Jan Djärv <jan.h.d@swipnet.se>
> Date: Tue, 29 Jul 2014 17:29:07 +0200
> Cc: martin rudalics <rudalics@gmx.at>,
>  emacs-devel@gnu.org
> 
> >>> (Btw, shouldn't GTK scroll bars already support bidirectional text out
> >>> of the box?  Perhaps you need to set some flag(s) to get that.)
> >> 
> >> No idea.
> > 
> > Maybe Jan (CC'ed) can help us out.
> > 
> >> In any case I would have to tell GTK whether the "current text"
> >> (whatever that is) is L2R or R2L I suppose.
> > 
> > Yes, but we have current-bidi-paragraph-direction for that.
> 
> Not out of the box, but 
>    gtk_widget_set_direction (w, GTK_TEXT_DIR_RTL)
> and
>   gtk_range_set_flippable (w, TRUE);
> 
> where w is the scrollbar should do it according to the documentation.  I have not tried it.
> gtk_range_set_flippable is since 2.18, so you would have to put in a check for that.

Thanks.

So we should call gtk_range_set_flippable, and then actually flip the
direction by calling gtk_widget_set_direction according to what
current-bidi-paragraph-direction returns, is that right?

Bingo [Re: Hint on Xaw3d scroll bar colors issue [Was: Re: Changes in frame/window code]]

[Dmitry Antipov]

On 07/29/2014 06:01 PM, martin rudalics wrote:

> So there's nothing we even _could_ do here?

It was so simple.  Fixed in r117609.

Dmitry

Re: Hint on Xaw3d scroll bar colors issue [Was: Re: Changes in frame/window code]

[Dmitry Antipov]

On 07/29/2014 07:42 PM, Jan Djärv wrote:

> The color is read from the X resource database by the Xaw widget library.
> The resource database is seeded in xrdb.c with (for example)
>
>    sprintf (line, "Emacs*verticalScrollBar.background: grey75");
>
> but when the horizontal scroll bar is created it has the name horizontalScrollBar,
> so no wonder the colors are different.  Adding corresponding lines in xrdb.c for
> horizontalScrollBar should do it (3 places).

Found that myself just a few minutes before your e-mail :-(.

Note that Motif build has identical scroll bar colors even without adding
'*horizontalScrollBar.background: xxx' resource, but Lucid hasn't.  Do you
have any ideas why it is so?

Dmitry

Re: Bingo [Re: Hint on Xaw3d scroll bar colors issue [Was: Re: Changes in frame/window code]]

[Jan Djärv]

Hi.

29 jul 2014 kl. 18:43 skrev Dmitry Antipov <dmantipov@yandex.ru>:

> On 07/29/2014 06:01 PM, martin rudalics wrote:
> 
>> So there's nothing we even _could_ do here?
> 
> It was so simple.  Fixed in r117609.

Why did you not fix it for the Motif case?

	Jan D.

Re: Hint on Xaw3d scroll bar colors issue [Was: Re: Changes in frame/window code]

[Jan Djärv]

Hi.

29 jul 2014 kl. 18:54 skrev Dmitry Antipov <dmantipov@yandex.ru>:

> On 07/29/2014 07:42 PM, Jan Djärv wrote:
> 
>> The color is read from the X resource database by the Xaw widget library.
>> The resource database is seeded in xrdb.c with (for example)
>> 
>>   sprintf (line, "Emacs*verticalScrollBar.background: grey75");
>> 
>> but when the horizontal scroll bar is created it has the name horizontalScrollBar,
>> so no wonder the colors are different.  Adding corresponding lines in xrdb.c for
>> horizontalScrollBar should do it (3 places).
> 
> Found that myself just a few minutes before your e-mail :-(.
> 
> Note that Motif build has identical scroll bar colors even without adding
> '*horizontalScrollBar.background: xxx' resource, but Lucid hasn't.  Do you
> have any ideas why it is so?

Motif uses XtDefaultBackground if nothing has been set.  XtDefaultBackground is grey75 (mostly).

	Jan D.

Re: Changes in frame/window code

[martin rudalics]

 > I'm confused: how does that get called when I click a mouse on the
 > scroll-bar slider?  Just click, no drag.

Do you mean that when you click on the slider the value of
suspend_auto_hscroll is reset?  That would be strange.  It should get
reset when you click into the text area or change point in some other
way.

 >>   > I tried that, but then the feature stopped working altogether.
 >>
 >> In which sense?
 >
 > The slider didn't jump to the right when point was in a R2L paragraph.

When precisely?  Usually you have to select the window (more or less) in
order to move point to a R2L paragraph in it and at that moment the
slider should jump to the right.  If you now select another window the
slider should stick to the right until you either explicitly change
`point' or text in that window.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Tue, 29 Jul 2014 20:23:44 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
>  > I'm confused: how does that get called when I click a mouse on the
>  > scroll-bar slider?  Just click, no drag.
> 
> Do you mean that when you click on the slider the value of
> suspend_auto_hscroll is reset?

No, I mean the slider jumps back to the left edge, whereas it started
at the right edge (which is correct for R2L lines).

> That would be strange.  It should get reset when you click into the
> text area or change point in some other way.

Well, it was you who mentioned suspend_auto_hscroll as the possible
cause for the slider behavior when I click on it.  If
suspend_auto_hscroll is not related to that problem, then I ask again:
where's the code which causes the slider to jump to the left when I
click on it?

>  >>   > I tried that, but then the feature stopped working altogether.
>  >>
>  >> In which sense?
>  >
>  > The slider didn't jump to the right when point was in a R2L paragraph.
> 
> When precisely?  Usually you have to select the window (more or less) in
> order to move point to a R2L paragraph in it and at that moment the
> slider should jump to the right.  If you now select another window the
> slider should stick to the right until you either explicitly change
> `point' or text in that window.

I experimented with only one window.  I intended to try with several
windows showing the same buffer after that.  However, testing with a
single window (which was obviously selected) failed to produce the
desired behavior, so I never proceeded to more than one window.

What I did was temporarily move point to the window-point, then call
current-bidi-paragraph-direction.  Maybe there's something wrong with
this.

Re: Changes in frame/window code

[Jan Djärv]

> So we should call gtk_range_set_flippable, and then actually flip the
> direction by calling gtk_widget_set_direction according to what
> current-bidi-paragraph-direction returns, is that right?

I guess so.

	Jan D.

Re: Changes in frame/window code

[martin rudalics]

 >> Do you mean that when you click on the slider the value of
 >> suspend_auto_hscroll is reset?
 >
 > No, I mean the slider jumps back to the left edge, whereas it started
 > at the right edge (which is correct for R2L lines).

The following pretty crude version of `scroll-bar-horizontal-drag-1' fixes this
here.

(defun scroll-bar-horizontal-drag-1 (event)
   (let* ((start-position (event-start event))
	 (window (nth 0 start-position))
	 (portion-whole (nth 2 start-position))
	 (unit (frame-char-width (window-frame window))))
     (if (eq (current-bidi-paragraph-direction) 'left-to-right)
	(set-window-hscroll
	 window (/ (1- (+ (car portion-whole) unit)) unit))
       (set-window-hscroll
        window (- (window-text-width) (/ (1- (+ (car portion-whole) unit)) unit))))))

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Wed, 30 Jul 2014 18:05:09 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
>  >> Do you mean that when you click on the slider the value of
>  >> suspend_auto_hscroll is reset?
>  >
>  > No, I mean the slider jumps back to the left edge, whereas it started
>  > at the right edge (which is correct for R2L lines).
> 
> The following pretty crude version of `scroll-bar-horizontal-drag-1' fixes this
> here.

Sounds right, please install (together with the C-level changes I sent
yesterday).

Thanks.

Re: Bingo [Re: Hint on Xaw3d scroll bar colors issue [Was: Re: Changes in frame/window code]]

[martin rudalics]

> It was so simple.  Fixed in r117609.

Thanks ;-)

Re: Hint on Xaw3d scroll bar colors issue [Was: Re: Changes in frame/window code]

[martin rudalics]

 > The color is read from the X resource database by the Xaw widget library.  The resource database is seeded in xrdb.c with (for example)
 >
 >    sprintf (line, "Emacs*verticalScrollBar.background: grey75");
 >
 > but when the horizontal scroll bar is created it has the name horizontalScrollBar, so no wonder the colors are different.  Adding corresponding lines in xrdb.c for horizontalScrollBar should do it (3 places).

I wonder why they were the same here.  I couldn't reproduce Dmitry's
scenario.

martin

Re: Changes in frame/window code

[martin rudalics]

 > Sounds right, please install (together with the C-level changes I sent
 > yesterday).

Not yet.  Sometimes I can't drag the slider far enough to make the first
column of R2L text appear.  So some tuning is still needed.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Wed, 30 Jul 2014 18:45:16 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
> Sometimes I can't drag the slider far enough to make the first
> column of R2L text appear.

Perhaps some off-by-one error?  (The rightmost column is one less than
window-text-width.)

Re: Changes in frame/window code

[martin rudalics]

 > Perhaps some off-by-one error?  (The rightmost column is one less than
 > window-text-width.)

Off-by-two, if I counted correctly (that is `window-hscroll' remains
stuck at 2).  Unfortunately, here the block cursor sometimes erratically
disappears behind the right fringe so visually it's hard to tell.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Wed, 30 Jul 2014 19:36:04 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
>  > Perhaps some off-by-one error?  (The rightmost column is one less than
>  > window-text-width.)
> 
> Off-by-two, if I counted correctly (that is `window-hscroll' remains
> stuck at 2).  Unfortunately, here the block cursor sometimes erratically
> disappears behind the right fringe so visually it's hard to tell.

If you mean that the cursor is sometimes not shown because point is
off-screen due to hscroll, then this is normal.

Re: Changes in frame/window code

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 227 bytes --]

 > If you mean that the cursor is sometimes not shown because point is
 > off-screen due to hscroll, then this is normal.

It sometimes appears as in the attached screenshot with an artifact
on the right of the fringe.

martin

[-- Attachment #2: bidi_cursor.png --]
[-- Type: image/png, Size: 29420 bytes --]

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Thu, 31 Jul 2014 10:49:20 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
> It sometimes appears as in the attached screenshot with an artifact
> on the right of the fringe.

That's a bug.  If you can provide a reproducible recipe, I'll take a
look.

Re: Changes in frame/window code

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

 > It sometimes appears as in the attached screenshot with an artifact
 > on the right of the fringe.

Actually the problem is deeper and seems to go back to my pixelwise
changes from last year.  Starting Emacs 24.4 with -Q can get me the
attached screenshot when dragging the divider between two side-by-side
windows.  IIUC this means that we don't align the first column of R2L
text correctly at the right edge of a window that is not the rightmost
one.  Do you understand what goes wrong here?

martin

[-- Attachment #2: bidi-cursor-2.png --]
[-- Type: image/png, Size: 34277 bytes --]

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Thu, 31 Jul 2014 12:09:08 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
> Actually the problem is deeper and seems to go back to my pixelwise
> changes from last year.  Starting Emacs 24.4 with -Q can get me the
> attached screenshot when dragging the divider between two side-by-side
> windows.  IIUC this means that we don't align the first column of R2L
> text correctly at the right edge of a window that is not the rightmost
> one.

Maybe somebody lies to us about the real value of window_box_width?
This is used in extend_face_to_end_of_line to compute the width of the
stretch glyph we put at the left edge of each R2L display line.

And the problem is not limited to non-rightmost windows, I see it in
the rightmost window as well.  Which doesn't surprise me, since the
calculations in extend_face_to_end_of_line don't depend on that.

Re: Changes in frame/window code

[martin rudalics]

 > Maybe somebody lies to us about the real value of window_box_width?
 > This is used in extend_face_to_end_of_line to compute the width of the
 > stretch glyph we put at the left edge of each R2L display line.
 >
 > And the problem is not limited to non-rightmost windows, I see it in
 > the rightmost window as well.  Which doesn't surprise me, since the
 > calculations in extend_face_to_end_of_line don't depend on that.

In any case the problem triggers here iff a right fringe is shown.

martin

Re: Changes in frame/window code

[Stefan Monnier]

Regarding the horizontal scrollbar, I find it really odd to have
a scrollbar when the text is wrapped.  So I think by default, it should
only be present when the text is truncated.


        Stefan

Re: Changes in frame/window code

[Tassilo Horn]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Regarding the horizontal scrollbar, I find it really odd to have a
> scrollbar when the text is wrapped.  So I think by default, it should
> only be present when the text is truncated.

I had the same feeling when I saw them first.  And ideally, they would
only show up when the text is actually truncated, i.e., `truncate-lines'
is t and there is a line that's longer than the window width.
Basically, that would also make sense for vertical scroll bars.  Why
should there be a scroll bar when the buffer text is smaller than the
window height?

Bye,
Tassilo

Re: Changes in frame/window code

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Thu, 31 Jul 2014 12:18:39 -0400
> 
> Regarding the horizontal scrollbar, I find it really odd to have
> a scrollbar when the text is wrapped.  So I think by default, it should
> only be present when the text is truncated.

I agree, but that's not what was being discussed here.

Re: Changes in frame/window code

[martin rudalics]

 > Regarding the horizontal scrollbar, I find it really odd to have
 > a scrollbar when the text is wrapped.  So I think by default, it should
 > only be present when the text is truncated.

Fully agreed.  I intend to turn them off by default since line
truncation is off by default.

martin

Re: Changes in frame/window code

[martin rudalics]

 > And ideally, they would
 > only show up when the text is actually truncated, i.e., `truncate-lines'
 > is t

Then we would need new optional values for that.  Patches welcome.

 > and there is a line that's longer than the window width.
 > Basically, that would also make sense for vertical scroll bars.  Why
 > should there be a scroll bar when the buffer text is smaller than the
 > window height?

I intend to write an auto-hide-scroll-bars mode to do that but it's not
entirely trivial and probably would not come without additional costs.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> From: Tassilo Horn <tsdh@gnu.org>
> Date: Thu, 31 Jul 2014 18:37:14 +0200
> Cc: martin rudalics <rudalics@gmx.at>, Eli Zaretskii <eliz@gnu.org>,
> 	emacs-devel@gnu.org
> 
> Why should there be a scroll bar when the buffer text is smaller
> than the window height?

Because it's simpler.

Patches to implement what you want are welcome.

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Thu, 31 Jul 2014 13:35:08 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
>  > Maybe somebody lies to us about the real value of window_box_width?
>  > This is used in extend_face_to_end_of_line to compute the width of the
>  > stretch glyph we put at the left edge of each R2L display line.
>  >
>  > And the problem is not limited to non-rightmost windows, I see it in
>  > the rightmost window as well.  Which doesn't surprise me, since the
>  > calculations in extend_face_to_end_of_line don't depend on that.
> 
> In any case the problem triggers here iff a right fringe is shown.

Yes, there were 2 bugs related to this situation, hopefully fixed in
r117417 on the emacs-24 branch.

Re: Changes in frame/window code

[martin rudalics]

 > Yes, there were 2 bugs related to this situation, hopefully fixed in
 > r117417 on the emacs-24 branch.

Looks very good now.  Can you please merge them to trunk - there are a
number of newer changes nearby so I'm afraid to break something if I
apply them manually.

Thanks, martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Fri, 01 Aug 2014 12:25:12 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
> Can you please merge them to trunk - there are a number of newer
> changes nearby so I'm afraid to break something if I apply them
> manually.

Done.

Re: Changes in frame/window code

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 298 bytes --]

 > Done.

Thanks.  I still see a tiny glitch with the line

‏<תו>-C  משמעותו לחץ והחזק מקש CONTROL ואז הקש על מקש <תו>.

possibly pasted badly here.  Can you spot the remnants of the cursor on
the right of the fringe in the attached screenshot?

martin

[-- Attachment #2: bidi-cursor-3.png --]
[-- Type: image/png, Size: 29316 bytes --]

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Fri, 01 Aug 2014 17:29:44 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
> Thanks.  I still see a tiny glitch with the line
> 
> ‏<תו>-C  משמעותו לחץ והחזק מקש CONTROL ואז הקש על מקש <תו>.
> 
> possibly pasted badly here.  Can you spot the remnants of the cursor on
> the right of the fringe in the attached screenshot?

I see it in your snapshot, but not on my system.

Re: Changes in frame/window code

[Jay Belanger]

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]


> Note that horizontal scrollbars are turned on by default on all builds
> that support them.  This is the only way I see to get them at least some
> initial coverage.  If you utterly dislike them (or do not use scrollbars
> at all) please use the idiom

I don't use scroll bars, but I'll keep them on while they're being
tested.  But is the dark bar above the mode line in the first picture
below a scroll bar?

> (when (fboundp 'horizontal-scroll-bar-mode)
>   (horizontal-scroll-bar-mode -1))

I ask, because applying this adds a small white space above the mode
line, as shown in the second picture.  (Both are from emacs started with
emacs -Q.)


[-- Attachment #2: dark bar --]
[-- Type: image/png, Size: 3591 bytes --]

[-- Attachment #3: dark bar with white bar --]
[-- Type: image/png, Size: 3258 bytes --]

Re: Changes in frame/window code

[martin rudalics]

 > I don't use scroll bars, but I'll keep them on while they're being
 > tested.

Thank you.

 > But is the dark bar above the mode line in the first picture
 > below a scroll bar?
 >
 >> (when (fboundp 'horizontal-scroll-bar-mode)
 >>    (horizontal-scroll-bar-mode -1))
 >
 > I ask, because applying this adds a small white space above the mode
 > line, as shown in the second picture.  (Both are from emacs started with
 > emacs -Q.)

This is a regression I introduced yesterday and only people who tested
this interactively could notice it easily.  Should be fixed now - please
try again.

Thanks a lot for spotting this, martin

Re: Changes in frame/window code

[Jay Belanger]

>>> (when (fboundp 'horizontal-scroll-bar-mode)
>>>    (horizontal-scroll-bar-mode -1))
>>
>> I ask, because applying this adds a small white space above the mode
>> line, as shown in the second picture.  (Both are from emacs started with
>> emacs -Q.)
>
> This is a regression I introduced yesterday and only people who tested
> this interactively could notice it easily.  Should be fixed now - please
> try again.

Works great; thanks.

Re: Changes in frame/window code

[martin rudalics]

> Works great; thanks.

Thanks for the heads-up, martin

Re: Changes in frame/window code

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 929 bytes --]

 > I see it in your snapshot, but not on my system.

Yes.  These are difficult to trigger.  Here's another problem.  The
snapshots are with 24.3 to make sure that no new feature creeps in, but
here the problem with current trunk is precisely the same.  The first
snapshot (mouse-click-ante) gives the tutorial in the left window in
some specific non-zero hscroll state.  I now mouse-click where the mouse
cursor appears, namely on the "I" of the word "EDIT".  This makes point
in that window move to where the block cursor appears in the second
snapshot (mouse-click-post) namely over the first "ו" (the fifth letter
of the word) "משמעותו".  Can you reproduce it?

Note that no such problem occurs when the window is not hscrolled.
Currently, I have some hard time mouse dragging lines with mixed
left-to-right and right-to-left text.  Maybe that problem is related to
the one I described here.

martin

[-- Attachment #2: mouse-click-ante.png --]
[-- Type: image/png, Size: 24854 bytes --]

[-- Attachment #3: mouse-click-post.png --]
[-- Type: image/png, Size: 21726 bytes --]

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Fri, 15 Aug 2014 17:52:53 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  > I see it in your snapshot, but not on my system.
> 
> Yes.  These are difficult to trigger.  Here's another problem.  The
> snapshots are with 24.3 to make sure that no new feature creeps in

It is not useful to show such problems in 24.3, because the bugfixes I
lately committed to the emacs-24 branch are not there, so display of
hscrolled R2L lines there is fundamentally broken.

> here the problem with current trunk is precisely the same.  The first
> snapshot (mouse-click-ante) gives the tutorial in the left window in
> some specific non-zero hscroll state.  I now mouse-click where the mouse
> cursor appears, namely on the "I" of the word "EDIT".  This makes point
> in that window move to where the block cursor appears in the second
> snapshot (mouse-click-post) namely over the first "ו" (the fifth letter
> of the word) "משמעותו".  Can you reproduce it?

Yes, but that's an entirely different problem: looks like moving point
by clicking the mouse doesn't work correctly at all in hscrolled R2L
lines.  Please file a bug report, and I will fix this.

> Note that no such problem occurs when the window is not hscrolled.

Hey, when I added bidi support, I did do a thorough job for the most
basic features, and mouse clicks are certainly part of that.

> Currently, I have some hard time mouse dragging lines with mixed
> left-to-right and right-to-left text.

What do you mean by "dragging lines"?

Re: Changes in frame/window code

[martin rudalics]

 > Please file a bug report, and I will fix this.

Filed as bug#18277.

 >> Currently, I have some hard time mouse dragging lines with mixed
 >> left-to-right and right-to-left text.
 >
 > What do you mean by "dragging lines"?

Dragging a line with the horizontal scroll bar.  We're still in the
subthread about horizontal scroll bars not working with bidirectional
text.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Sat, 16 Aug 2014 11:35:46 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  >> Currently, I have some hard time mouse dragging lines with mixed
>  >> left-to-right and right-to-left text.
>  >
>  > What do you mean by "dragging lines"?
> 
> Dragging a line with the horizontal scroll bar.

The changes to support that are still not in the trunk, are they?  At
least the scroll bar is on the wrong side here.

Commit the changes we were discussing, and we could then talk
meaningfully about whatever problems are left.  Right now, I cannot
even try reproducing those problems.

Re: Changes in frame/window code

[martin rudalics]

 >> Dragging a line with the horizontal scroll bar.
 >
 > The changes to support that are still not in the trunk, are they?

No.  Our earlier changes were too rudimentary.

 > At
 > least the scroll bar is on the wrong side here.

You mean the slider/thumb for hscroll equal zero?  That is the crucial
point.  Moving it to the other side causes all problems I've seen so
far.

 > Commit the changes we were discussing, and we could then talk
 > meaningfully about whatever problems are left.  Right now, I cannot
 > even try reproducing those problems.

I'll do that as soon as you fixed bug#18277.  I see three problems
currently:

(1) When dragging I cannot move the slider entirely to the right and
     make the first column of R2L lines visible until I release the mouse
     button.  I suppose this is due to some off-by-one failure here but
     there might be more to it.

(2) Sometimes during dragging the slider starts moving for-/backward in
     some erratic fashion.  I'm not yet sure whether this is caused by an
     hmargin issue or something else.

(3) Line 121 (written backwards as)

אם הסמן נמצא באמצע מילה, M-f זז לסוף המילה. אם הסמן נמצא בין שתי מלים,

    of the tutorial presents a special problem: When I'm repeatedly doing
    `next-line' on the rightmost column before that line and are about
    to move to that line I enter some strange sort of loop with the
    slider in the center of the scroll bar trying to move back- and
    forward.  C-g gets me out of it but I can't tell so far why I'm stuck
    there.

If (2) or (3) are caused by bug#18277 (or a common underlying problem)
it should be easier to resolve the remaining issues.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Sat, 16 Aug 2014 15:46:04 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  > At
>  > least the scroll bar is on the wrong side here.
> 
> You mean the slider/thumb for hscroll equal zero?  That is the crucial
> point.  Moving it to the other side causes all problems I've seen so
> far.

Understood, but I cannot say anything about how the scroll bar works
until it gets that initial position right.  The current state of
affairs is a non-starter.

>  > Commit the changes we were discussing, and we could then talk
>  > meaningfully about whatever problems are left.  Right now, I cannot
>  > even try reproducing those problems.
> 
> I'll do that as soon as you fixed bug#18277.

I don't see the relevance of 18277 to this issue.  That bug is about
clicking inside the text area, not about clicking on the scroll bar or
hscrolling.  Why do you think they are related?

> I see three problems currently:
> 
> (1) When dragging I cannot move the slider entirely to the right and
>      make the first column of R2L lines visible until I release the mouse
>      button.  I suppose this is due to some off-by-one failure here but
>      there might be more to it.
> 
> (2) Sometimes during dragging the slider starts moving for-/backward in
>      some erratic fashion.  I'm not yet sure whether this is caused by an
>      hmargin issue or something else.
> 
> (3) Line 121 (written backwards as)
> 
> אם הסמן נמצא באמצע מילה, M-f זז לסוף המילה. אם הסמן נמצא בין שתי מלים,
> 
>     of the tutorial presents a special problem: When I'm repeatedly doing
>     `next-line' on the rightmost column before that line and are about
>     to move to that line I enter some strange sort of loop with the
>     slider in the center of the scroll bar trying to move back- and
>     forward.  C-g gets me out of it but I can't tell so far why I'm stuck
>     there.
> 
> If (2) or (3) are caused by bug#18277 (or a common underlying problem)
> it should be easier to resolve the remaining issues.

I could be wrong, but I don't think they are related to 18277 at all.
E.g., there's nothing in common between vertical-motion (which is what
you do in (3)) and moving point by clicking the mouse.

As for 2, does this happen around line 655?  That line, and the one
that follows, are L2R, and so are lines 918 and 919.  This could
confuse the code.  Another possible reason could be short lines that
disappear entirely under some large enough hscroll.

To summarize, I think you should commit your changes, and we should
deal with the fallout with later commits.  After all, these problems
went unnoticed until now, and they only happen if one turns on
truncate-lines in the tutorial, which sounds an unlikely situation.
It makes little sense to me to delay the horizontal scroll-bar fixes
due to these anomalies, which are unlikely to be related to your
un-installed changes.

But it's your call.

Re: Changes in frame/window code

[martin rudalics]

 > I don't see the relevance of 18277 to this issue.  That bug is about
 > clicking inside the text area, not about clicking on the scroll bar or
 > hscrolling.  Why do you think they are related?

Because clicking changes the hscroll amount.

  I could be wrong, but I don't think they are related to 18277 at all.
 > E.g., there's nothing in common between vertical-motion (which is what
 > you do in (3)) and moving point by clicking the mouse.

Apparently vertical motion with partially scrolled text affects hscroll
in some strange way.

 > As for 2, does this happen around line 655?  That line, and the one
 > that follows, are L2R, and so are lines 918 and 919.  This could
 > confuse the code.  Another possible reason could be short lines that
 > disappear entirely under some large enough hscroll.

AFAICT neither of these are involved.  But I suppose that quite often
embedded L2R text is involved.

 > To summarize, I think you should commit your changes, and we should
 > deal with the fallout with later commits.  After all, these problems
 > went unnoticed until now, and they only happen if one turns on
 > truncate-lines in the tutorial, which sounds an unlikely situation.
 > It makes little sense to me to delay the horizontal scroll-bar fixes
 > due to these anomalies, which are unlikely to be related to your
 > un-installed changes.

OK.  I installed the changes so that they should work for the Windows
build (I don't want to mangle the other builds at this stage).  Please
have a look at the three problems from my last post.

Note: Problem (3) is reproducible only in the left of two side-by-side
windows.  Since this is the strangest bug maybe you could have a look.
I'll repeat how to reproduce it.  Load the tutorial, split the window
via C-x 3.  In the left window move point to the rightmost column of
bidi text before line 121.  Lean on your `next-line' key until you reach
line 121 - here redisplay hangs.  I can't reproduce this with any other
line.

martin

Re: Changes in frame/window code

[martin rudalics]

> I can't reproduce this with any other
> line.

And that was a lie.  I can reproduce it with other lines too.

martin

Re: Changes in frame/window code

[martin rudalics]

> And that was a lie.  I can reproduce it with other lines too.

And with Emacs 24.4 too, BTW.  So it's not scroll bar related.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Sat, 16 Aug 2014 15:46:04 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
> (1) When dragging I cannot move the slider entirely to the right and
>      make the first column of R2L lines visible until I release the mouse
>      button.  I suppose this is due to some off-by-one failure here but
>      there might be more to it.

I'm not sure this is at all related to R2L lines.  I wrote a trivial
function which just displays the mouse event, and bound it to clicks
on the horizontal scroll bar.  When I drag the scroll bar, even in an
entirely L2R buffer, I see some strange phenomena: e.g., more often
than not, when I drag the bar all the way to the right, releasing the
mouse button without moving the mouse reports slightly different
coordinates than the ones reported before releasing the mouse.
Moreover, many time the 2nd coordinate reported after releasing the
mouse is -1.  Try it.  Maybe it's Windows-specific, I don't know.

Btw, it looks like the information returned by mouse events on the
horizontal scroll bar is not described anywhere, not even in NEWS.
Your use of (cdr (nth 2 start-position)) in
scroll-bar-horizontal-drag-1 got my jaw dropped for a few seconds,
before I realized it was not the Y coordinate...

Also, I don't understand the subtlety of +1 in setting up the scroll
info, e.g.:

  /* Allow nPage to be one larger than nPos so we don't allow to scroll
     an already fully visible buffer.  */
  si.nPage = min (portion, si.nMax) + 1;
  si.nPos = min (position, si.nMax);

The comment doesn't correspond to the code, and I couldn't understand
what's behind this trick.

> (2) Sometimes during dragging the slider starts moving for-/backward in
>      some erratic fashion.  I'm not yet sure whether this is caused by an
>      hmargin issue or something else.

I could only see this in the very first line of the tutorial.  There
was an old and very subtle bug in xdisp.c, revealed by
set_horizontal_scroll_bar, which caused broken cursor motion in that
line.  This is now fixed in r117711 on the trunk.  If you still see
something like that, and it's not related to the few L2R lines in the
tutorial, please report a bug with the recipe.

> (3) Line 121 (written backwards as)
> 
> אם הסמן נמצא באמצע מילה, M-f זז לסוף המילה. אם הסמן נמצא בין שתי מלים,
> 
>     of the tutorial presents a special problem: When I'm repeatedly doing
>     `next-line' on the rightmost column before that line and are about
>     to move to that line I enter some strange sort of loop with the
>     slider in the center of the scroll bar trying to move back- and
>     forward.  C-g gets me out of it but I can't tell so far why I'm stuck
>     there.

This is now fixed in r117447 on the release branch.  The previous
revision fixed bug #18277.

> If (2) or (3) are caused by bug#18277 (or a common underlying problem)
> it should be easier to resolve the remaining issues.

They were all separate and completely unrelated problems.

Re: Changes in frame/window code

[martin rudalics]

 >> (1) When dragging I cannot move the slider entirely to the right and
 >>       make the first column of R2L lines visible until I release the mouse
 >>       button.  I suppose this is due to some off-by-one failure here but
 >>       there might be more to it.
 >
 > I'm not sure this is at all related to R2L lines.  I wrote a trivial
 > function which just displays the mouse event, and bound it to clicks
 > on the horizontal scroll bar.  When I drag the scroll bar, even in an
 > entirely L2R buffer, I see some strange phenomena: e.g., more often
 > than not, when I drag the bar all the way to the right, releasing the
 > mouse button without moving the mouse reports slightly different
 > coordinates than the ones reported before releasing the mouse.
 > Moreover, many time the 2nd coordinate reported after releasing the
 > mouse is -1.  Try it.  Maybe it's Windows-specific, I don't know.

I suppose the -1 is due to the

       si.nPage = min (portion, si.nMax) + 1;

you mention below and that's likely the culprit for the effect described
in (1).

 > Btw, it looks like the information returned by mouse events on the
 > horizontal scroll bar is not described anywhere, not even in NEWS.
 > Your use of (cdr (nth 2 start-position)) in
 > scroll-bar-horizontal-drag-1 got my jaw dropped for a few seconds,
 > before I realized it was not the Y coordinate...

I'm not sure yet where to eventually put this and whether it's the right
solution at all.  IIUC the scroll bar area on the right of the thumb stands
for w->hscroll in R2L text and this was the easiest way to pass on that
value from what GetScrollInfo returns without consing.  If you have any
suggestion how to handle this cleanly I'd be all ears.

 > Also, I don't understand the subtlety of +1 in setting up the scroll
 > info, e.g.:
 >
 >    /* Allow nPage to be one larger than nPos so we don't allow to scroll
 >       an already fully visible buffer.  */
 >    si.nPage = min (portion, si.nMax) + 1;
 >    si.nPos = min (position, si.nMax);
 >
 > The comment doesn't correspond to the code, and I couldn't understand
 > what's behind this trick.

The idea is that if I do not add 1 to nPage, the displayed thumb gets
too small.  This means that although I see the entire text I can still
scroll it and make the first column disappear, which is distracting.
I've not found a better solution yet.  In any case, this is my probably
poor interpretation of how SetScrollInfo works (compare
http://msdn.microsoft.com/en-us/library/windows/desktop/bb787595%28v=vs.85%29.aspx
The nPage member must specify a value from 0 to nMax - nMin +1. The nPos
member must specify a value between nMin and nMax - max( nPage– 1, 0)).
And yes, the comment is obviously wrong.

 >> (2) Sometimes during dragging the slider starts moving for-/backward in
 >>       some erratic fashion.  I'm not yet sure whether this is caused by an
 >>       hmargin issue or something else.
 >
 > I could only see this in the very first line of the tutorial.  There
 > was an old and very subtle bug in xdisp.c, revealed by
 > set_horizontal_scroll_bar, which caused broken cursor motion in that
 > line.  This is now fixed in r117711 on the trunk.  If you still see
 > something like that, and it's not related to the few L2R lines in the
 > tutorial, please report a bug with the recipe.

I can see it only with my personal setup here.  With emacs -Q I can
reproduce it by setting

(set-frame-parameter nil 'right-divider-width 7)

and scrolling the tutorial in the left window of a C-x 3 split frame.

 >> (3) Line 121 (written backwards as)
 >>
 >> אם הסמן נמצא באמצע מילה, M-f זז לסוף המילה. אם הסמן נמצא בין שתי מלים,
 >>
 >>      of the tutorial presents a special problem: When I'm repeatedly doing
 >>      `next-line' on the rightmost column before that line and are about
 >>      to move to that line I enter some strange sort of loop with the
 >>      slider in the center of the scroll bar trying to move back- and
 >>      forward.  C-g gets me out of it but I can't tell so far why I'm stuck
 >>      there.
 >
 > This is now fixed in r117447 on the release branch.  The previous
 > revision fixed bug #18277.

Fine.  These work well now.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Mon, 18 Aug 2014 10:31:38 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  >> (1) When dragging I cannot move the slider entirely to the right and
>  >>       make the first column of R2L lines visible until I release the mouse
>  >>       button.  I suppose this is due to some off-by-one failure here but
>  >>       there might be more to it.
>  >
>  > I'm not sure this is at all related to R2L lines.  I wrote a trivial
>  > function which just displays the mouse event, and bound it to clicks
>  > on the horizontal scroll bar.  When I drag the scroll bar, even in an
>  > entirely L2R buffer, I see some strange phenomena: e.g., more often
>  > than not, when I drag the bar all the way to the right, releasing the
>  > mouse button without moving the mouse reports slightly different
>  > coordinates than the ones reported before releasing the mouse.
>  > Moreover, many time the 2nd coordinate reported after releasing the
>  > mouse is -1.  Try it.  Maybe it's Windows-specific, I don't know.
> 
> I suppose the -1 is due to the
> 
>        si.nPage = min (portion, si.nMax) + 1;
> 
> you mention below and that's likely the culprit for the effect described
> in (1).

Perhaps, but it's still unclear to me why this problem is not visible
in L2R buffers.

>  > Btw, it looks like the information returned by mouse events on the
>  > horizontal scroll bar is not described anywhere, not even in NEWS.
>  > Your use of (cdr (nth 2 start-position)) in
>  > scroll-bar-horizontal-drag-1 got my jaw dropped for a few seconds,
>  > before I realized it was not the Y coordinate...
> 
> I'm not sure yet where to eventually put this and whether it's the right
> solution at all.  IIUC the scroll bar area on the right of the thumb stands
> for w->hscroll in R2L text and this was the easiest way to pass on that
> value from what GetScrollInfo returns without consing.  If you have any
> suggestion how to handle this cleanly I'd be all ears.

I don't think your solution is unclean (the Y coordinate makes no
sense for horizontal scroll bar events), I just wish it were
documented, at least in NEWS.  It doesn't take more than a few lines.

>  >> (2) Sometimes during dragging the slider starts moving for-/backward in
>  >>       some erratic fashion.  I'm not yet sure whether this is caused by an
>  >>       hmargin issue or something else.
>  >
>  > I could only see this in the very first line of the tutorial.  There
>  > was an old and very subtle bug in xdisp.c, revealed by
>  > set_horizontal_scroll_bar, which caused broken cursor motion in that
>  > line.  This is now fixed in r117711 on the trunk.  If you still see
>  > something like that, and it's not related to the few L2R lines in the
>  > tutorial, please report a bug with the recipe.
> 
> I can see it only with my personal setup here.  With emacs -Q I can
> reproduce it by setting
> 
> (set-frame-parameter nil 'right-divider-width 7)
> 
> and scrolling the tutorial in the left window of a C-x 3 split frame.

On Windows XP, I cannot see this at all.  On Windows 7, I always see
this on the 1st line of the tutorial, even without setting
right-divider-width, and I see it when point is on most (but not all,
see below) lines if I do set right-divider-width.

Alas, I'm not your guy in these matters, as I'm clueless as to GUI
programming in general and Windows GUI in particular.  So I just
sprinkled fprintf lines in suspect places and watched them while
exercising the erratic behavior of the scroll bar.  Here are some
observations that might help you figure this out:

  . The erratic behavior seems to be due to our own code resetting the
    thumb position right after I drag the thumb to some new position.
    The code which causes this seems to be -- surprise, surprise --
    set_horizontal_scroll_bar that calls
    set_horizontal_scroll_bar_hook passing it the previous position.
    I didn't yet figure out why this affects only R2L lines.

  . If point is on an empty line, the problem doesn't happen.
    Likewise, if I drag the scroll bar of a non-selected window, the
    problem doesn't happen, either.  Maybe we call the scroll-bar hook
    too early, without giving redisplay the chance to hscroll all the
    windows?

(Btw, note that I needed a small change to be able to scroll correctly
non-selected windows; see r117712 on the trunk.)

(Btw2, when is x_horizontal_scroll_bar_report_motion called?  I
couldn't see it being called when the scroll bar is used.)

HTH

Re: Changes in frame/window code

[martin rudalics]

 >> I suppose the -1 is due to the
 >>
 >>         si.nPage = min (portion, si.nMax) + 1;
 >>
 >> you mention below and that's likely the culprit for the effect described
 >> in (1).
 >
 > Perhaps,

It happens without the "+ 1" as well, so this is not responsible.

 > but it's still unclear to me why this problem is not visible
 > in L2R buffers.

Because for L2R I count the part from the leftmost position to the start
of the scroll bar thumb which is unaffected by the size of the thumb?

 > I don't think your solution is unclean (the Y coordinate makes no
 > sense for horizontal scroll bar events), I just wish it were
 > documented, at least in NEWS.  It doesn't take more than a few lines.

But I've just put it in and only in the Windows port because you
proposed to try it.  As soon as I've rewritten all affected ports I'll
document it.

 > On Windows XP, I cannot see this at all.

I see it on Windows XP.

 > On Windows 7, I always see
 > this on the 1st line of the tutorial, even without setting
 > right-divider-width, and I see it when point is on most (but not all,
 > see below) lines if I do set right-divider-width.

I do so too, meanwhile.  But setting the right divider width to non-zero
makes it happen immediately here on the first line of the tutorial.

 > Alas, I'm not your guy in these matters, as I'm clueless as to GUI
 > programming in general and Windows GUI in particular.  So I just
 > sprinkled fprintf lines in suspect places and watched them while
 > exercising the erratic behavior of the scroll bar.  Here are some
 > observations that might help you figure this out:
 >
 >    . The erratic behavior seems to be due to our own code resetting the
 >      thumb position right after I drag the thumb to some new position.
 >      The code which causes this seems to be -- surprise, surprise --
 >      set_horizontal_scroll_bar that calls
 >      set_horizontal_scroll_bar_hook passing it the previous position.
 >      I didn't yet figure out why this affects only R2L lines.

You mean that w->hscroll has its previous value in

       start = w->hscroll * FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w));

in xdisp.c although w->hscroll was in between changed by
`scroll-bar-horizontal-drag-1'?

 >    . If point is on an empty line, the problem doesn't happen.

Confirmed.

 >      Likewise, if I drag the scroll bar of a non-selected window, the
 >      problem doesn't happen, either.

Confirmed, too.  Weird.  Do we somewhere try more hard to autoscroll
when the window is selected?

 >      Maybe we call the scroll-bar hook
 >      too early, without giving redisplay the chance to hscroll all the
 >      windows?

Wouldn't in such case the result be that some windows fail to get
scrolled?

 > (Btw, note that I needed a small change to be able to scroll correctly
 > non-selected windows; see r117712 on the trunk.)

Thanks, I forgot about these.

 > (Btw2, when is x_horizontal_scroll_bar_report_motion called?  I
 > couldn't see it being called when the scroll bar is used.)

I don't have the slightest idea when and whether I should call it.  I
had changed its code to return the part right of the thumb as well only
to find out that it never got called so I left the code unchanged in my
last commit.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Mon, 18 Aug 2014 18:11:19 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  > but it's still unclear to me why this problem is not visible
>  > in L2R buffers.
> 
> Because for L2R I count the part from the leftmost position to the start
> of the scroll bar thumb which is unaffected by the size of the thumb?

If so, we have an off-by-one error somewhere, since these are just
pixel values with nothing magic about them.

>  >    . The erratic behavior seems to be due to our own code resetting the
>  >      thumb position right after I drag the thumb to some new position.
>  >      The code which causes this seems to be -- surprise, surprise --
>  >      set_horizontal_scroll_bar that calls
>  >      set_horizontal_scroll_bar_hook passing it the previous position.
>  >      I didn't yet figure out why this affects only R2L lines.
> 
> You mean that w->hscroll has its previous value in
> 
>        start = w->hscroll * FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w));
> 
> in xdisp.c although w->hscroll was in between changed by
> `scroll-bar-horizontal-drag-1'?

I didn't analyze this further, but it should be easy to verify.

>  >    . If point is on an empty line, the problem doesn't happen.
> 
> Confirmed.
> 
>  >      Likewise, if I drag the scroll bar of a non-selected window, the
>  >      problem doesn't happen, either.
> 
> Confirmed, too.  Weird.  Do we somewhere try more hard to autoscroll
> when the window is selected?

See hscroll_windows: we scroll all windows in a window tree.

>  >      Maybe we call the scroll-bar hook
>  >      too early, without giving redisplay the chance to hscroll all the
>  >      windows?
> 
> Wouldn't in such case the result be that some windows fail to get
> scrolled?

Isn't that exactly what we observe here?

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Mon, 18 Aug 2014 19:22:46 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > Date: Mon, 18 Aug 2014 18:11:19 +0200
> > From: martin rudalics <rudalics@gmx.at>
> > CC: emacs-devel@gnu.org
> > 
> >  > but it's still unclear to me why this problem is not visible
> >  > in L2R buffers.
> > 
> > Because for L2R I count the part from the leftmost position to the start
> > of the scroll bar thumb which is unaffected by the size of the thumb?

No, that wasn't it, see below.

> >  >    . The erratic behavior seems to be due to our own code resetting the
> >  >      thumb position right after I drag the thumb to some new position.
> >  >      The code which causes this seems to be -- surprise, surprise --
> >  >      set_horizontal_scroll_bar that calls
> >  >      set_horizontal_scroll_bar_hook passing it the previous position.
> >  >      I didn't yet figure out why this affects only R2L lines.
> > 
> > You mean that w->hscroll has its previous value in
> > 
> >        start = w->hscroll * FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w));
> > 
> > in xdisp.c although w->hscroll was in between changed by
> > `scroll-bar-horizontal-drag-1'?
> 
> I didn't analyze this further, but it should be easy to verify.

I found the reason for this: w32_horizontal_scroll_bar_handle_click
reported bogus (always -1) values in the Y part of '(X . Y)' member of
the click event, when I drag the thumb.  That value is not used in L2R
paragraphs, so L2R buffers were not affected.

This is now fixed in trunk r117715.

Btw, I don't understand why in w32_horizontal_scroll_bar_handle_click
you overwrite the value reported from GetScrollInfo by
HIWORD(msg->msg.wParam).  My reading of MSDN documentation is that
they recommend to do it the other way around, since GetScrollInfo is
not limited to 16-bit values.  If using the values reported by
GetScrollInfo doesn't work well, perhaps that's because for
SB_THUMBTRACK message you need to use SIF_TRACKPOS and si.nTrackPos
rather than SIF_POS and si.nPos.

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Tue, 19 Aug 2014 18:15:58 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> Btw, I don't understand why in w32_horizontal_scroll_bar_handle_click
> you overwrite the value reported from GetScrollInfo by
> HIWORD(msg->msg.wParam).  My reading of MSDN documentation is that
> they recommend to do it the other way around, since GetScrollInfo is
> not limited to 16-bit values.  If using the values reported by
> GetScrollInfo doesn't work well, perhaps that's because for
> SB_THUMBTRACK message you need to use SIF_TRACKPOS and si.nTrackPos
> rather than SIF_POS and si.nPos.

I switched to using the GetScrollInfo information (for both horizontal
and vertical scroll bars) in trunk revision 117716.  I didn't find any
reason for the current code, except, perhaps, the fact that an old
version of MSDN documentation didn't document SIF_TRACKPOS at all.
But since even the venerable "Programming Windows" by Petzold that was
published in 1998 described that value very well without any
qualifications for its support by various Windows versions we care
about, I think it's safe to use it.

Re: Changes in frame/window code

[martin rudalics]

 > I found the reason for this: w32_horizontal_scroll_bar_handle_click
 > reported bogus (always -1) values in the Y part of '(X . Y)' member of
 > the click event, when I drag the thumb.  That value is not used in L2R
 > paragraphs, so L2R buffers were not affected.

I noticed the -1 but attributed it to the fact that I made the thumb
larger by one and decided to take care of this later.  I wouldn't have
suspected this to be the root of almost all evil and would probably have
spent another week looking elsewhere.

 > This is now fixed in trunk r117715.

Thanks.  I'll now take care of the other builds.

 > Btw, I don't understand why in w32_horizontal_scroll_bar_handle_click
 > you overwrite the value reported from GetScrollInfo by
 > HIWORD(msg->msg.wParam).  My reading of MSDN documentation is that
 > they recommend to do it the other way around, since GetScrollInfo is
 > not limited to 16-bit values.  If using the values reported by
 > GetScrollInfo doesn't work well, perhaps that's because for
 > SB_THUMBTRACK message you need to use SIF_TRACKPOS and si.nTrackPos
 > rather than SIF_POS and si.nPos.

I first took everything verbatim from the vertical routines and then
changed what I considered necessary for the horizontal part.  This
specific part obviously remained unchanged.

martin

Re: Changes in frame/window code

[martin rudalics]

 > I switched to using the GetScrollInfo information (for both horizontal
 > and vertical scroll bars) in trunk revision 117716.  I didn't find any
 > reason for the current code, except, perhaps, the fact that an old
 > version of MSDN documentation didn't document SIF_TRACKPOS at all.
 > But since even the venerable "Programming Windows" by Petzold that was
 > published in 1998 described that value very well without any
 > qualifications for its support by various Windows versions we care
 > about, I think it's safe to use it.

Looks good.  Thanks for the change.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Wed, 20 Aug 2014 17:32:11 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  > I found the reason for this: w32_horizontal_scroll_bar_handle_click
>  > reported bogus (always -1) values in the Y part of '(X . Y)' member of
>  > the click event, when I drag the thumb.  That value is not used in L2R
>  > paragraphs, so L2R buffers were not affected.
> 
> I noticed the -1 but attributed it to the fact that I made the thumb
> larger by one and decided to take care of this later.  I wouldn't have
> suspected this to be the root of almost all evil and would probably have
> spent another week looking elsewhere.

The crucial finding was that set-window-hscroll is always called with
its NCOL argument set to zero, when I drag the thumb.  The rest was
very easy.

RE: Changes in frame/window code

[Drew Adams]

>  > And ideally, they would only show up when the text is actually
>  > truncated, i.e., `truncate-lines' is t and there is a line that's
>  > longer than the window width.
>
>  > Basically, that would also make sense for vertical scroll bars.
>  > Why should there be a scroll bar when the buffer text is smaller
>  > than the window height?
> 
> I intend to write an auto-hide-scroll-bars mode to do that but it's
> not entirely trivial and probably would not come without additional
> costs.

What might those additional costs be?

Is it hard to at least turn it off automatically whenever
`truncate-lines' is nil?

Anyway, I suggest that until this feature (horizontal scroll bars)
can be fixed so that it shows the scroll bar only when the text is
actually truncated, it should be turned off by default.  Users should
not suddenly need to customize Emacs, to get rid of this annoyance.

Don't get me wrong - I'm glad we finally have horizontal scroll bars.
But Emacs really needs to come up to speed with the rest of the
world in this regard: don't show scroll bars when they are useless.

Re: Changes in frame/window code

[martin rudalics]

 > Thanks.  I'll now take care of the other builds.

Should be fixed now for the Gtk, Motif and Lucid builds.  I also changed
the semantics of (cdr (nth 2 start-position)) on the horizontal scroll
bar, so (nth 2 start-position) now returns portion-whole as for the
vertical case.  We should eventually update the doc-strings of
scroll-bar.el, `scroll-bar-toolkit-scroll' doesn't even have one yet.

Note also that your fix in r117417 on the emacs-24 branch doesn't seem
to fix the underlying problem for builds on GNU/Linux.  I have to come
up with a recipe yet, though.

martin

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Thu, 28 Aug 2014 11:40:46 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  > Thanks.  I'll now take care of the other builds.
> 
> Should be fixed now for the Gtk, Motif and Lucid builds.

Thanks.

> Note also that your fix in r117417 on the emacs-24 branch doesn't seem
> to fix the underlying problem for builds on GNU/Linux.  I have to come
> up with a recipe yet, though.

Yes, please.  Or at least show a screenshot, so I understand what you
are talking about.

Re: Changes in frame/window code

[Glenn Morris]

martin rudalics wrote:

> Should be fixed now for the Gtk, Motif and Lucid builds.

Fails to build for Lucid without-toolkit-scrollbars.

xterm.c:6187
   int new_start = y - bar->dragging;

"y" is undefined.

Re: Changes in frame/window code

[martin rudalics]

> Fails to build for Lucid without-toolkit-scrollbars.
>
> xterm.c:6187
>     int new_start = y - bar->dragging;
>
> "y" is undefined.

I'm so silly.  Should build again now.

Thanks, martin

Re: Changes in frame/window code

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]

 > Yes, please.  Or at least show a screenshot, so I understand what you
 > are talking about.

I'm talking about display artifacts in the fringe.  I attached a shot
for the Gtk build.  IIRC you fixed this for the Windows build in r117417
of the emacs-24 branch but for some reason the fix didn't seem to help
for the Gtk build.  Below I add our last dialogue about this subject.

martin


 > >  > Maybe somebody lies to us about the real value of window_box_width?
 > >  > This is used in extend_face_to_end_of_line to compute the width of the
 > >  > stretch glyph we put at the left edge of each R2L display line.
 > >  >
 > >  > And the problem is not limited to non-rightmost windows, I see it in
 > >  > the rightmost window as well.  Which doesn't surprise me, since the
 > >  > calculations in extend_face_to_end_of_line don't depend on that.
 > >
 > > In any case the problem triggers here iff a right fringe is shown.
 >
 > Yes, there were 2 bugs related to this situation, hopefully fixed in
 > r117417 on the emacs-24 branch.


[-- Attachment #2: gtk-fringe.png --]
[-- Type: image/png, Size: 88827 bytes --]

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Thu, 28 Aug 2014 21:05:29 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  > Yes, please.  Or at least show a screenshot, so I understand what you
>  > are talking about.
> 
> I'm talking about display artifacts in the fringe.  I attached a shot
> for the Gtk build.  IIRC you fixed this for the Windows build in r117417
> of the emacs-24 branch but for some reason the fix didn't seem to help
> for the Gtk build.  Below I add our last dialogue about this subject.

You mean, that "a" on the right fringe of the rightmost window?
That's entirely unrelated to the problems I fixed in r117417.  A
recipe would help, though.

Re: Changes in frame/window code

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

 > You mean, that "a" on the right fringe of the rightmost window?
 > That's entirely unrelated to the problems I fixed in r117417.

Visually they are very similar.

 > A
 > recipe would help, though.

The following is 100% reproducible with emacs -Q on a Lucid build
(compare the attached screenshots):

(1) C-u C-h t hebrew RET

(2) C-x 3                              (gives hscroll-0.png)

(3) M-: (set-window-hscroll nil 1)     (gives hscroll-1.png)

(4) M-: (set-window-hscroll nil 2)     (gives hscroll-2.png)

(5) M-: (set-window-hscroll nil 1)     (gives hscroll-3.png)

Note again: NOT reproducible on my Windows build.

martin

[-- Attachment #2: hscroll-0.png --]
[-- Type: image/png, Size: 75541 bytes --]

[-- Attachment #3: hscroll-1.png --]
[-- Type: image/png, Size: 79651 bytes --]

[-- Attachment #4: hscroll-2.png --]
[-- Type: image/png, Size: 80568 bytes --]

[-- Attachment #5: hscroll-3.png --]
[-- Type: image/png, Size: 79741 bytes --]

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Fri, 29 Aug 2014 11:00:31 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  > You mean, that "a" on the right fringe of the rightmost window?
>  > That's entirely unrelated to the problems I fixed in r117417.
> 
> Visually they are very similar.

No, they aren't: what you see on the fringe is always a remnant of a
cursor -- it's always in cursor colors, not the text colors.

IOW, something draws the cursor on the fringe, or maybe scrolls text
with the cursor on it, as drawn by the previous redisplay, without
paying attention to the fact that the x-coordinate is outside the text
area.

By contrast, the problems fixed in r117417 were due to incorrect setup
of the glyph row in the text area, not to incorrect drawing.

> (1) C-u C-h t hebrew RET
> 
> (2) C-x 3                              (gives hscroll-0.png)
> 
> (3) M-: (set-window-hscroll nil 1)     (gives hscroll-1.png)
> 
> (4) M-: (set-window-hscroll nil 2)     (gives hscroll-2.png)
> 
> (5) M-: (set-window-hscroll nil 1)     (gives hscroll-3.png)
> 
> Note again: NOT reproducible on my Windows build.

I think I can reproduce something similar if I change the buffer font
size from the default.

Thanks, I will take a look.

Re: Changes in frame/window code

[Eli Zaretskii]

> Date: Fri, 29 Aug 2014 12:53:45 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > (1) C-u C-h t hebrew RET
> > 
> > (2) C-x 3                              (gives hscroll-0.png)
> > 
> > (3) M-: (set-window-hscroll nil 1)     (gives hscroll-1.png)
> > 
> > (4) M-: (set-window-hscroll nil 2)     (gives hscroll-2.png)
> > 
> > (5) M-: (set-window-hscroll nil 1)     (gives hscroll-3.png)
> > 
> > Note again: NOT reproducible on my Windows build.
> 
> I think I can reproduce something similar if I change the buffer font
> size from the default.
> 
> Thanks, I will take a look.

I tried to fix this in trunk revision 117789.

Re: Changes in frame/window code

[martin rudalics]

> I tried to fix this in trunk revision 117789.

Looks good now (tested with Gtk and Lucid).

Thanks, martin

Re: Changes in frame/window code

[martin rudalics]

>  > Regarding the horizontal scrollbar, I find it really odd to have
>  > a scrollbar when the text is wrapped.  So I think by default, it should
>  > only be present when the text is truncated.
>
> Fully agreed.  I intend to turn them off by default since line
> truncation is off by default.

Done with revision 117810.

martin

Re: Changes in frame/window code

[Glenn Morris]

BTW, if I configure using:

  --with-x-toolkit=lucid --without-toolkit-scroll-bars 

then

 M-x horizontal-scroll-bar-mode

still says "Horizontal-Scroll-Bar mode enabled" with no error, but no
scroll bar appears. I suppose this is consistent with the NEWS entry,
but is there any chance of making it work --without-toolkit-scroll-bars?

If not, it seems like M-x horizontal-scroll-bar-mode should give an error.

PS congrats and kudos for implementing a very long-standing TODO item! :)

Re: Changes in frame/window code

[martin rudalics]

 > BTW, if I configure using:
 >
 >    --with-x-toolkit=lucid --without-toolkit-scroll-bars
 >
 > then
 >
 >   M-x horizontal-scroll-bar-mode
 >
 > still says "Horizontal-Scroll-Bar mode enabled" with no error, but no
 > scroll bar appears.

Should be fixed now.  Please have a look.

 > I suppose this is consistent with the NEWS entry,
 > but is there any chance of making it work --without-toolkit-scroll-bars?

You mean implement horizontal scroll bars when no tool-kit scroll bars
are available?  IIRC I was able to draw the slider/thumb correctly but
never started to implement mouse operations.

 > If not, it seems like M-x horizontal-scroll-bar-mode should give an error.

I print an informative message now.  Giving an error seems to drastic,
after all people might want (horizontal-scroll-bar-mode 1) in their .emacs.

martin

Re: Changes in frame/window code

[Glenn Morris]

martin rudalics wrote:

> Should be fixed now.  Please have a look.

Yes, thanks.

>> I suppose this is consistent with the NEWS entry,
>> but is there any chance of making it work --without-toolkit-scroll-bars?
>
> You mean implement horizontal scroll bars when no tool-kit scroll bars
> are available?  IIRC I was able to draw the slider/thumb correctly but
> never started to implement mouse operations.

OK. I think a few (?) people use --without-toolkit-scroll-bars,
so it would be nice to have, but obviously it's not at all important.