Re: Extending fringe backgrounds to scroll bar gaps

[YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>]

>>>>> On Tue, 06 Mar 2007 17:36:39 -0500, Richard Stallman <rms@gnu.org> said:

> This should wait for after the release.  We only want to fix bugs
> now, and this is not quite a bug.

Is it OK to install this change to the trunk now?  Is anybody
preparing for another change that obviates this one?

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

The original message follows:

There have been several reports/complaints about small gaps beside
toolkit scroll bars on the frame edges.  Some complained about the gap
with the frame edge, and others the one with the fringe of the window.
(I think the latter makes more sense because a scroll bar is
associated with its scrolling target, i.e., window, not with the
containing frame.)

According to *1 and *2, this is due to the restrictions that toolkit
scroll bars have a fixed width and each window must be placed on a
multiple of the frame default column width.  Though the latter is
planned to be relaxed in future, that will/should not happen before
the release, of course.

  *1 http://lists.gnu.org/archive/html/emacs-pretest-bug/2005-07/msg00326.html
  *2 http://lists.gnu.org/archive/html/emacs-devel/2004-10/msg00291.html

I've just tried extending the fringe backgrounds to such scroll bar
gaps.  Its rough idea is already mentioned in *3.  I think this is
simple enough to incorporate before the release (if it works properly,
of course).  What do people think about that?

  *3 http://lists.gnu.org/archive/html/emacs-pretest-bug/2005-09/msg00334.html

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Index: src/xterm.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xterm.c,v
retrieving revision 1.942
diff -c -p -r1.942 xterm.c
*** src/xterm.c	4 Mar 2007 18:28:49 -0000	1.942
--- src/xterm.c	6 Mar 2007 03:03:46 -0000
*************** x_draw_fringe_bitmap (w, row, p)
*** 720,725 ****
--- 720,727 ----
  
    if (p->bx >= 0 && !p->overlay_p)
      {
+       int bx = p->bx, nx = p->nx;
+ 
        /* In case the same realized face is used for fringes and
  	 for something displayed in the text (e.g. face `region' on
  	 mono-displays, the fill style may have been changed to
*************** x_draw_fringe_bitmap (w, row, p)
*** 729,736 ****
        else
  	XSetForeground (display, face->gc, face->background);
  
!       XFillRectangle (display, window, face->gc,
! 		      p->bx, p->by, p->nx, p->ny);
  
        if (!face->stipple)
  	XSetForeground (display, face->gc, face->foreground);
--- 731,764 ----
        else
  	XSetForeground (display, face->gc, face->background);
  
! #ifdef USE_TOOLKIT_SCROLL_BARS
!       /* If the fringe is adjacent to the left (right) scroll bar of a
! 	 leftmost (rightmost, respectively) window, then extend its
! 	 background to the gap between the fringe and the bar.  */
!       if ((WINDOW_LEFTMOST_P (w)
! 	   && WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_LEFT (w))
! 	  || (WINDOW_RIGHTMOST_P (w)
! 	      && WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_RIGHT (w)))
! 	{
! 	  int sb_width = WINDOW_CONFIG_SCROLL_BAR_WIDTH (w);
! 
! 	  if (sb_width > 0)
! 	    {
! 	      int left = WINDOW_SCROLL_BAR_AREA_X (w);
! 	      int width = (WINDOW_CONFIG_SCROLL_BAR_COLS (w)
! 			   * FRAME_COLUMN_WIDTH (f));
! 
! 	      if (left + width == bx)
! 		{
! 		  bx = left + sb_width;
! 		  nx += width - sb_width;
! 		}
! 	      else if (bx + nx == left)
! 		nx += width - sb_width;
! 	    }
! 	}
! #endif
!       XFillRectangle (display, window, face->gc, bx, p->by, nx, p->ny);
  
        if (!face->stipple)
  	XSetForeground (display, face->gc, face->foreground);
*************** x_scroll_bar_create (w, top, left, width
*** 4785,4790 ****
--- 4813,4821 ----
    XSETINT (bar->start, 0);
    XSETINT (bar->end, 0);
    bar->dragging = Qnil;
+ #ifdef USE_TOOLKIT_SCROLL_BARS
+   bar->fringe_extended_p = Qnil;
+ #endif
  
    /* Add bar to its frame's list of scroll bars.  */
    bar->next = FRAME_SCROLL_BARS (f);
*************** XTset_vertical_scroll_bar (w, portion, w
*** 4977,4982 ****
--- 5008,5016 ----
    struct scroll_bar *bar;
    int top, height, left, sb_left, width, sb_width;
    int window_y, window_height;
+ #ifdef USE_TOOLKIT_SCROLL_BARS
+   int fringe_extended_p;
+ #endif
  
    /* Get window dimensions.  */
    window_box (w, -1, 0, &window_y, 0, &window_height);
*************** XTset_vertical_scroll_bar (w, portion, w
*** 4997,5011 ****
    /* Compute the left edge of the scroll bar.  */
  #ifdef USE_TOOLKIT_SCROLL_BARS
    if (WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_RIGHT (w))
!     sb_left = (left +
! 	       (WINDOW_RIGHTMOST_P (w)
! 		? width - sb_width - (width - sb_width) / 2
! 		: 0));
!   else
!     sb_left = (left +
! 	       (WINDOW_LEFTMOST_P (w)
! 		? (width - sb_width) / 2
! 		: width - sb_width));
  #else
    if (WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_RIGHT (w))
      sb_left = left + width - sb_width;
--- 5031,5039 ----
    /* Compute the left edge of the scroll bar.  */
  #ifdef USE_TOOLKIT_SCROLL_BARS
    if (WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_RIGHT (w))
!     sb_left = left + (WINDOW_RIGHTMOST_P (w) ? width - sb_width : 0);
!   else
!     sb_left = left + (WINDOW_LEFTMOST_P (w) ? 0 : width - sb_width);
  #else
    if (WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_RIGHT (w))
      sb_left = left + width - sb_width;
*************** XTset_vertical_scroll_bar (w, portion, w
*** 5013,5026 ****
      sb_left = left;
  #endif
  
    /* Does the scroll bar exist yet?  */
    if (NILP (w->vertical_scroll_bar))
      {
        if (width > 0 && height > 0)
  	{
  	  BLOCK_INPUT;
! 	  x_clear_area (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f),
! 			left, top, width, height, False);
  	  UNBLOCK_INPUT;
  	}
  
--- 5041,5073 ----
      sb_left = left;
  #endif
  
+ #ifdef USE_TOOLKIT_SCROLL_BARS
+   if (WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_LEFT (w))
+     fringe_extended_p = (WINDOW_LEFTMOST_P (w)
+ 			 && WINDOW_LEFT_FRINGE_WIDTH (w)
+ 			 && (WINDOW_HAS_FRINGES_OUTSIDE_MARGINS (w)
+ 			     || WINDOW_LEFT_MARGIN_COLS (w) == 0));
+   else
+     fringe_extended_p = (WINDOW_RIGHTMOST_P (w)
+ 			 && WINDOW_RIGHT_FRINGE_WIDTH (w)
+ 			 && (WINDOW_HAS_FRINGES_OUTSIDE_MARGINS (w)
+ 			     || WINDOW_RIGHT_MARGIN_COLS (w) == 0));
+ #endif
+ 
    /* Does the scroll bar exist yet?  */
    if (NILP (w->vertical_scroll_bar))
      {
        if (width > 0 && height > 0)
  	{
  	  BLOCK_INPUT;
! #ifdef USE_TOOLKIT_SCROLL_BARS
! 	  if (fringe_extended_p)
! 	    x_clear_area (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f),
! 			  sb_left, top, sb_width, height, False);
! 	  else
! #endif
! 	    x_clear_area (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f),
! 			  left, top, width, height, False);
  	  UNBLOCK_INPUT;
  	}
  
*************** XTset_vertical_scroll_bar (w, portion, w
*** 5047,5059 ****
  #ifdef USE_TOOLKIT_SCROLL_BARS
  
        /* Move/size the scroll bar widget.  */
!       if (mask)
  	{
  	  /* Since toolkit scroll bars are smaller than the space reserved
  	     for them on the frame, we have to clear "under" them.  */
  	  if (width > 0 && height > 0)
! 	    x_clear_area (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f),
!                           left, top, width, height, False);
  #ifdef USE_GTK
            xg_update_scrollbar_pos (f,
                                     SCROLL_BAR_X_WINDOW (bar),
--- 5094,5112 ----
  #ifdef USE_TOOLKIT_SCROLL_BARS
  
        /* Move/size the scroll bar widget.  */
!       if (mask || !NILP (bar->fringe_extended_p) != fringe_extended_p)
  	{
  	  /* Since toolkit scroll bars are smaller than the space reserved
  	     for them on the frame, we have to clear "under" them.  */
  	  if (width > 0 && height > 0)
! 	    {
! 	      if (fringe_extended_p)
! 		x_clear_area (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f),
! 			      sb_left, top, sb_width, height, False);
! 	      else
! 		x_clear_area (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f),
! 			      left, top, width, height, False);
! 	    }
  #ifdef USE_GTK
            xg_update_scrollbar_pos (f,
                                     SCROLL_BAR_X_WINDOW (bar),
*************** XTset_vertical_scroll_bar (w, portion, w
*** 5128,5133 ****
--- 5181,5188 ----
      }
  
  #ifdef USE_TOOLKIT_SCROLL_BARS
+   bar->fringe_extended_p = fringe_extended_p ? Qt : Qnil;
+ 
    x_set_toolkit_scroll_bar_thumb (bar, portion, position, whole);
  #else /* not USE_TOOLKIT_SCROLL_BARS */
    /* Set the scroll bar's current state, unless we're currently being
Index: src/xterm.h
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xterm.h,v
retrieving revision 1.190
diff -c -p -r1.190 xterm.h
*** src/xterm.h	21 Jan 2007 04:18:14 -0000	1.190
--- src/xterm.h	6 Mar 2007 03:03:47 -0000
*************** struct scroll_bar
*** 807,812 ****
--- 807,818 ----
       place where the user grabbed it.  If the handle isn't currently
       being dragged, this is Qnil.  */
    Lisp_Object dragging;
+ 
+ #ifdef USE_TOOLKIT_SCROLL_BARS
+   /* t if the background of the fringe that is adjacent to a scroll
+      bar is extended to the gap between the fringe and the bar.  */
+   Lisp_Object fringe_extended_p;
+ #endif
  };
  
  /* The number of elements a vector holding a struct scroll_bar needs.  */