visible-bell patch for Mac OS X

visible-bell patch for Mac OS X

[Filipe Cabecinhas]

[-- Attachment #1.1: Type: text/plain, Size: 692 bytes --]

Hi,

I've made a patch to remove that ugly black rectangle from ns_ring_bell,
when visible-bell is 't.
The patch makes Emacs behave like the X version, inverting the colors of the
top and bottom lines, and bounces the Dock icon once if Emacs is not the
frontmost app (which can be extended to when visible-bell is 'nil, if
desired (just hoist it outside the if).

There is one difference in behaviour to the X version. When we have
horizontally-split frames, the scrollbars between frames also get inverted.
If we want to invert all the scrollbars, we just need to change the CGRect
calculations to not take the scrollbar into account.

Patch made against revision 99416.

Regards,

  Filipe

[-- Attachment #1.2: Type: text/html, Size: 858 bytes --]

[-- Attachment #2: cocoa-visible-bell.patch --]
[-- Type: application/octet-stream, Size: 5680 bytes --]

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: filcab@gmail.com-20100130133139-qc9k267jlc69f99n
# target_branch: http://bzr.savannah.gnu.org/r/emacs/trunk
# testament_sha1: 27c8026494ef46e68ad79f8263f479ed22874804
# timestamp: 2010-01-30 13:33:01 +0000
# base_revision_id: cyd@stupidchicken.com-20100129191426-\
#   0m0lt5sbqqqhuufz
# 
# Begin patch
=== modified file 'src/nsterm.m'
--- src/nsterm.m	2010-01-15 01:46:53 +0000
+++ src/nsterm.m	2010-01-30 13:31:39 +0000
@@ -808,23 +808,58 @@
       view = FRAME_NS_VIEW (frame);
       if (view != nil)
         {
-          NSRect r, surr;
-          NSPoint dim = NSMakePoint (128, 128);
-
-          r = [view bounds];
-          r.origin.x += (r.size.width - dim.x) / 2;
-          r.origin.y += (r.size.height - dim.y) / 2;
-          r.size.width = dim.x;
-          r.size.height = dim.y;
-          surr = NSInsetRect (r, -2, -2);
-          ns_focus (frame, &surr, 1);
-          [[view window] cacheImageInRect: [view convertRect: surr toView:nil]];
-          [ns_lookup_indexed_color (NS_FACE_FOREGROUND
-                                      (FRAME_DEFAULT_FACE (frame)), frame) set];
-          NSRectFill (r);
+          /* Get the bounds of our NSView */
+          NSRect viewBounds = [view bounds];
+
+          /* Height of each line to flash.  */
+          int flash_height = FRAME_LINE_HEIGHT (frame);
+          int width = FRAME_PIXEL_WIDTH (frame)
+                    - NS_SCROLL_BAR_WIDTH (frame);
+
+          /* Get the GraphicsContext */
+          CGContextRef ctxt = [[NSGraphicsContext currentContext] graphicsPort];
+          CGRect lowerLine, upperLine;
+          lowerLine =
+          CGRectMake(viewBounds.origin.x, viewBounds.origin.y,
+                     width + NS_SCROLL_BAR_WIDTH(frame),
+                     flash_height + FRAME_INTERNAL_BORDER_WIDTH (frame));
+          upperLine =
+          CGRectMake(viewBounds.origin.x,
+                     viewBounds.origin.y + viewBounds.size.height
+                     - (flash_height + FRAME_INTERNAL_BORDER_WIDTH (frame)),
+                     width,
+                     flash_height + FRAME_INTERNAL_BORDER_WIDTH (frame));
+
+          /* Invert the colors using a difference blend.  */
+          CGContextSetBlendMode(ctxt, kCGBlendModeDifference);
+          CGContextSetGrayFillColor(ctxt, 1, 1);
+
+          /* If window is tall, flash top and bottom line.  */
+          if (viewBounds.size.height > 3 * FRAME_LINE_HEIGHT (frame))
+            {
+              CGContextFillRect(ctxt, upperLine);
+              CGContextFillRect(ctxt, lowerLine);
+            }
+          else
+            /* If it is short, flash it all.  */
+            CGContextFillRect(ctxt, NSRectToCGRect([view bounds]));
+
+          /* Bounce Dock icon. Maybe we can allow some configuration here.  */
+          [NSApp requestUserAttention: NSInformationalRequest];
+
           [[view window] flushWindow];
           ns_timeout (150000);
-          [[view window] restoreCachedImage];
+
+          /* If window is tall, flash top and bottom line.  */
+          if (viewBounds.size.height > 3 * FRAME_LINE_HEIGHT (frame))
+            {
+              CGContextFillRect(ctxt, upperLine);
+              CGContextFillRect(ctxt, lowerLine);
+            }
+          else
+            /* If it is short, flash it all.  */
+            CGContextFillRect(ctxt, NSRectToCGRect([view bounds]));
+
           [[view window] flushWindow];
           ns_unfocus (frame);
         }

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWfDfJK8AAp9/gAFQYABQf///
f+ffyr////pQBh52i3A53eu6rb07iEumgkkTVPNU9IzSn6aUbap+p6ptqh6m0n6po9Ro0aeptQPU
00epoDIpqemEGjSPSpmk9IMhkNqBkZAAAMQJEgU0yJjQjJNqn6Em1PRNqGIAyNHqGh6jTQMqhtQa
ZADTIAABoBoAAANACSQmRU/TI0BNMT00oyaG1PU0bUBoaDQGgaTJJcKLGMOokdefrJssxpYMxQWj
jtGT17hxkHh2juzhRYaoXYwqQcUcD7HXQhrkhADhCJJa+beMHb2mc0Xe9ihd/w6/00sbOj9i08OU
ns4dJSVaRC28d3GT4mexok3Nyp5cMPYVfNjJfDNTE/E6f+zB2DVplZX3RLMHlp4eHH+LZp4SsYHK
YGEf2QU6nurtnqZM1ZYJFlfa8yqtI2XWni8Dt5+Tjw19e9Q0XGlmDDxpdjUhnlIzNbxqSeEQNh56
jAYltn8OiLXo4DRdX06r13U1AnHn7Ywyz4r0FFt0yhQTZpnA9KC4VMlIHKg6A3jLghVO70ac8UmZ
hG5Cvg5hZBRzZCKxzJpnyF5VQlu1nc0nxnrZIPCFUGHHYCNu4JW1xvMxEh/E58GAZOxqBcUBmC8+
saazWG1jkjtsZLLPy6hxsNJ3l5x8WdeiyApmQaGjK9KGyYLKhUVkoE8+Cm668UcCpuBUgQ6WVA34
bzgFGrLXWcggncVzQhWF8WGIjN7DrCdgQWFesCTHXQSEus8J2mLTi8ygGX4bGsBycDTF5kkhEuS3
RALhs1vghsEm6i4GiVUo0KFGjNb6kV4E76YaTGrrRQLXF+OZSDIq7Zy0CvKVwNAsukWAtnTfyZlk
vvyQG2bdTVBaE7OZkCuR93nTabxZGT2jCrknw957HF6JS3MlANq/KVZ2558LJjuR9qNvD6wPDuJw
3B4BiokcfOdxBTYvsLBJvSt/uX1WZfhKRN1pQHqCFC4IQYaSYK5hFHsFg+CgtoAVTcpFsOx2Ke0B
yVRK44FYv6Q0sh6gbFVp/aV9VrKMqzJhY4CZBplvvGBI6FC8QpKtBfZq1Y65eA8mI2+ZFAicdUjY
GnklYJpHkc7c8gatVhHSf7pSsvhr946A9HxhVLzGP5azFVQTldKY1l6sEYwgvhlHEX9mz1RS6SrK
VXw5KaEsi07F6URojUP+DmqFrOsTktZwQPl0L23xp6g4z65od3tSNc5bIVhKXaA6BlJvftyb7tdh
JZdE6fYSWgHnUPDEYjI5d0xSSUQjhVhAjw7Gbqa6t/HLTj6iUCLQRNa40J9rrhsFWbs78d6eneNG
J/BECGeTZ8QJm897etO+JJhqe6qwnzrU1N/8qLvIMt0CwKYZXrLWnBhyUQssrTDNTkJILfhbg0Dd
BzmEIApklBxiz6hmPoPzq4VJ5MarTEm+FnWbBubcXeUV+0dMyrGmiD0T3qmhGYF70ro5jgeZkRCG
wt01osvo+4GIThcqDcB0LGGqkui1fGFLLHWrwOwKj/ciR4i5dN8CsLs6XI4R1AUmLCZQ2hgPj0m5
kMNnn0Yi7vWxrkMw1FYZkMGGYIcory5RRyDw1BFGxLcFwSgGlWfMg/4UAfQzjH3mCna5hqklHYdx
hzZFGQ9Sih8iYjpU1wlv4pUCWAi6mZbqUmfNuKk6SGWCYRJhn0UKp1BjUgTSIpVyszyZl3dzyUGr
AmENNW3fEXLvOHZ7PYbMSVMg0ji8BtG90wKURr2h4aAeD9p89kVj2GwZkmWDaUIaYlbTG1mTzDME
bVaE0Pk9EvItAucV9cRTKkc8EwZ0FPrPHaPFbnZdvYY8nZBTtzhCYbpa806NNhdyigFnBUTcUMb+
U7kUH+xsIGajDEXpgoGwstNzeG0DnWUPpB/V1eDWhwyuclEGydxyytWALKY0S1qlZTFFSEzbDtSR
nJxoIUwUy7QoWXmu5CVnQuGUBAIstd+gmCpBnVlWszLWToVYbb6phrvQyrm4lrxDvVfSfbBA+cDB
MCDNIIo2z5/Ip7F0mTFsBoUc7ZnNQPeto26+4P/F3JFOFCQ8N8krwA==

Re: visible-bell patch for Mac OS X

[Chong Yidong]

Filipe Cabecinhas <filcab@gmail.com> writes:

> I've made a patch to remove that ugly black rectangle
> from ns_ring_bell, when visible-bell is 't.  The patch makes Emacs
> behave like the X version, inverting the colors of the top and bottom
> lines, and bounces the Dock icon once if Emacs is not the frontmost
> app (which can be extended to when visible-bell is 'nil, if desired
> (just hoist it outside the if).

Thanks.  Looks good, I've checked it in.

Re: visible-bell patch for Mac OS X

[YAMAMOTO Mitsuharu]

>>>>> On Sat, 30 Jan 2010 22:34:32 -0500, Chong Yidong <cyd@stupidchicken.com> said:

> Filipe Cabecinhas <filcab@gmail.com> writes:
>> I've made a patch to remove that ugly black rectangle
>> from ns_ring_bell, when visible-bell is 't.  The patch makes Emacs
>> behave like the X version, inverting the colors of the top and
>> bottom lines, and bounces the Dock icon once if Emacs is not the
>> frontmost app (which can be extended to when visible-bell is 'nil,
>> if desired (just hoist it outside the if).

> Thanks.  Looks good, I've checked it in.

2010-01-31  Filipe Cabecinhas  <filcab@gmail.com>  (tiny change)

	* nsterm.m (ns_ring_bell): Handle visible bell like X.

The patch consists of ~80 lines.  Is it considered a "tiny change"?

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

Re: visible-bell patch for Mac OS X

[Chong Yidong]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

> The patch consists of ~80 lines.  Is it considered a "tiny change"?

I have started the copyright assignment process with Filipe.

Re: visible-bell patch for Mac OS X

[YAMAMOTO Mitsuharu]

>>>>> On Mon, 01 Feb 2010 10:56:31 -0500, Chong Yidong <cyd@stupidchicken.com> said:

> YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:
>> The patch consists of ~80 lines.  Is it considered a "tiny change"?

> I have started the copyright assignment process with Filipe.

Besides that, I'd suggest reverting the patch for now for several
reasons: it does not work on the second frame, it gives inconsistent
flashed area when the scroll bars are placed on the left side, and it
breaks GNUstep builds.

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

Re: visible-bell patch for Mac OS X

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 02 Feb 2010 08:33:22 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

>>> The patch consists of ~80 lines.  Is it considered a "tiny
>>> change"?

>> I have started the copyright assignment process with Filipe.

> Besides that, I'd suggest reverting the patch for now for several
> reasons: it does not work on the second frame, it gives inconsistent
> flashed area when the scroll bars are placed on the left side, and
> it breaks GNUstep builds.

And it is also inconsistent when there is a header line at the top, as
I pointed out before in the previous proposal for a similar patch by
another person:

  http://lists.gnu.org/archive/html/emacs-devel/2009-03/msg00412.html

I tried to do more consistent exclusion of the scroll bars from the
flashed area.  The patch below is against the Mac port:

  http://lists.gnu.org/archive/html/emacs-devel/2010-01/msg01487.html

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

=== modified file 'src/macterm.c'
*** src/macterm.c	2010-01-13 09:32:45 +0000
--- src/macterm.c	2010-02-02 01:19:04 +0000
***************
*** 737,742 ****
--- 737,776 ----
  }
  
  
+ static void
+ mac_invert_rectangles (f, rectangles, n)
+      struct frame *f;
+      const Rect *rectangles;
+      int n;
+ {
+   int i;
+ 
+   for (i = 0; i < n; i++)
+     mac_invert_rectangle (f, rectangles[i].left, rectangles[i].top,
+ 			  rectangles[i].right - rectangles[i].left,
+ 			  rectangles[i].bottom - rectangles[i].top);
+   if (FRAME_HAS_VERTICAL_SCROLL_BARS (f))
+     {
+       Lisp_Object bar;
+ 
+       if (FRAME_HAS_VERTICAL_SCROLL_BARS (f))
+ 	for (bar = FRAME_SCROLL_BARS (f); !NILP (bar);
+ 	     bar = XSCROLL_BAR (bar)->next)
+ 	  {
+ 	    struct scroll_bar *b = XSCROLL_BAR (bar);
+ 	    Rect bar_rect, r;
+ 
+ 	    SetRect (&bar_rect, b->left, b->top,
+ 		     b->left + b->width, b->top + b->height);
+ 	    for (i = 0; i < n; i++)
+ 	      if (SectRect (rectangles + i, &bar_rect, &r))
+ 		mac_invert_rectangle (f, r.left, r.top,
+ 				      r.right - r.left, r.bottom - r.top);
+ 	  }
+     }
+ }
+ 
+ 
  /* Mac replacement for XCopyArea: used only for scrolling.  */
  
  /* Defined in either mactoolbox.c or macappkit.m.  */
***************
*** 2866,2873 ****
    /* These will be the left and right margins of the rectangles.  */
    int flash_left = FRAME_INTERNAL_BORDER_WIDTH (f);
    int flash_right = FRAME_PIXEL_WIDTH (f) - FRAME_INTERNAL_BORDER_WIDTH (f);
- 
    int width;
  
    /* Don't flash the area between a scroll bar and the frame
       edge it is next to.  */
--- 2900,2908 ----
    /* These will be the left and right margins of the rectangles.  */
    int flash_left = FRAME_INTERNAL_BORDER_WIDTH (f);
    int flash_right = FRAME_PIXEL_WIDTH (f) - FRAME_INTERNAL_BORDER_WIDTH (f);
    int width;
+   Rect rects[2];
+   int nrects;
  
    /* Don't flash the area between a scroll bar and the frame
       edge it is next to.  */
***************
*** 2887,2910 ****
  
    width = flash_right - flash_left;
  
-   BLOCK_INPUT;
- 
-   /* If window is tall, flash top and bottom line.  */
    if (height > 3 * FRAME_LINE_HEIGHT (f))
      {
!       mac_invert_rectangle (f, flash_left,
! 			    (FRAME_INTERNAL_BORDER_WIDTH (f)
! 			     + FRAME_TOOL_BAR_LINES (f) * FRAME_LINE_HEIGHT (f)),
! 			    width, flash_height);
!       mac_invert_rectangle (f, flash_left,
! 			    (height - flash_height
! 			     - FRAME_INTERNAL_BORDER_WIDTH (f)),
! 			    width, flash_height);
      }
    else
!     /* If it is short, flash it all.  */
!     mac_invert_rectangle (f, flash_left, FRAME_INTERNAL_BORDER_WIDTH (f),
! 			  width, height - 2 * FRAME_INTERNAL_BORDER_WIDTH (f));
  
    x_flush (f);
  
--- 2922,2953 ----
  
    width = flash_right - flash_left;
  
    if (height > 3 * FRAME_LINE_HEIGHT (f))
      {
!       /* If window is tall, flash top and bottom line.  */
!       rects[0].left = rects[1].left = flash_left;
!       rects[0].top = (FRAME_INTERNAL_BORDER_WIDTH (f)
! 		    + FRAME_TOP_MARGIN_HEIGHT (f));
!       rects[0].right = rects[1].right = flash_left + width;
!       rects[0].bottom = rects[0].top + flash_height;
!       rects[1].top = height - flash_height - FRAME_INTERNAL_BORDER_WIDTH (f);
!       rects[1].bottom = rects[1].top + flash_height;
!       nrects = 2;
      }
    else
!     {
!       /* If it is short, flash it all.  */
!       rects[0].left = flash_left;
!       rects[0].top = FRAME_INTERNAL_BORDER_WIDTH (f);
!       rects[0].right = rects[0].left + width;
!       rects[0].bottom = rects[0].top + (height
! 					- 2 * FRAME_INTERNAL_BORDER_WIDTH (f));
!       nrects = 1;
!     }
! 
!   BLOCK_INPUT;
! 
!   mac_invert_rectangles (f, rects, nrects);
  
    x_flush (f);
  
***************
*** 2940,2961 ****
        }
    }
  
!   /* If window is tall, flash top and bottom line.  */
!   if (height > 3 * FRAME_LINE_HEIGHT (f))
!     {
!       mac_invert_rectangle (f, flash_left,
! 			    (FRAME_INTERNAL_BORDER_WIDTH (f)
! 			     + FRAME_TOOL_BAR_LINES (f) * FRAME_LINE_HEIGHT (f)),
! 			    width, flash_height);
!       mac_invert_rectangle (f, flash_left,
! 			    (height - flash_height
! 			     - FRAME_INTERNAL_BORDER_WIDTH (f)),
! 			    width, flash_height);
!     }
!   else
!     /* If it is short, flash it all.  */
!     mac_invert_rectangle (f, flash_left, FRAME_INTERNAL_BORDER_WIDTH (f),
! 			  width, height - 2 * FRAME_INTERNAL_BORDER_WIDTH (f));
  
    x_flush (f);
  
--- 2983,2989 ----
        }
    }
  
!   mac_invert_rectangles (f, rects, nrects);
  
    x_flush (f);
  

Re: visible-bell patch for Mac OS X

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 02 Feb 2010 10:40:46 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

> I tried to do more consistent exclusion of the scroll bars from the
> flashed area.  The patch below is against the Mac port:

>   http://lists.gnu.org/archive/html/emacs-devel/2010-01/msg01487.html

> +   if (FRAME_HAS_VERTICAL_SCROLL_BARS (f))
> +     {
> +       Lisp_Object bar;
> + 
> +       if (FRAME_HAS_VERTICAL_SCROLL_BARS (f))
> + 	for (bar = FRAME_SCROLL_BARS (f); !NILP (bar);
> + 	     bar = XSCROLL_BAR (bar)->next)

Oops, the second if-condition was redundant, of course.

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

Re: visible-bell patch for Mac OS X

[Leo]

On 2010-02-02 01:40 +0000, YAMAMOTO Mitsuharu wrote:
>> Besides that, I'd suggest reverting the patch for now for several
>> reasons: it does not work on the second frame, it gives inconsistent
>> flashed area when the scroll bars are placed on the left side, and it
>> breaks GNUstep builds.

From a user's point of view, I have found they are all annoying with
flashing the top and bottom line of the screen more annoying than a
small square at the centre. Ideally I would like to see only the echo
area (or maybe the modeline) is flashing. I have adopted Miles Bader's
configuration (available on emacswiki) to only flash the echo area. The
idea is the text area where users input text should remain untouched by
visible-bell because it happens too often.

Leo

Re: visible-bell patch for Mac OS X

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 02 Feb 2010 08:33:22 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

>> YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:
>>> The patch consists of ~80 lines.  Is it considered a "tiny
>>> change"?

>> I have started the copyright assignment process with Filipe.

> Besides that, I'd suggest reverting the patch for now for several
> reasons: it does not work on the second frame, it gives inconsistent
> flashed area when the scroll bars are placed on the left side, and
> it breaks GNUstep builds.

Bug#5569 confirms the last issue.  It also breaks build on Mac OS X
10.4 because of the lack of NSRectToCGRect.

BTW, I noticed that the GTK+ build may also flash part of scroll bars
if GDK_NATIVE_WINDOWS is not set.  One can argue that the current
behavior of Emacs 22 Carbon(+AppKit) port and Emacs 23 Mac port is
compatible with this :-p (though I'm planning to change this behavior
in the next releases of the Mac port).

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

Re: visible-bell patch for Mac OS X

[David De La Harpe Golden]

YAMAMOTO Mitsuharu wrote:


> BTW, I noticed that the GTK+ build may also flash part of scroll bars
> if GDK_NATIVE_WINDOWS is not set.


Is this a "known" (maybe gtk+ people did not see fit to mention it 
through any sensible channels to emacs people?) scrollbar issue:

Gtk+, like Qt [1], has moved away from most use of X11 subwindows.

see:
http://live.gnome.org/GTK%2B/ClientSideWindows

* Problems that will require application changes

     * emacs 23 (with gtk):
           o Sometimes when pressing Ctrl-L the scrollbars are overdrawn 
with white and not repainted. This is due to emacs using raw X 
primitives to draw on the window, while the scrollbars are client side 
emulated windows. Emacs could make the scrollbars native and this would 
make sure they weren't overdrawn



[1] 
http://labs.trolltech.com/blogs/2007/08/09/qt-invaded-by-aliens-the-end-of-all-flicker/

Native scrollbars? [was: visible-bell patch for Mac OS X]

[Stephen J. Turnbull]

David De La Harpe Golden writes:

 > Emacs could make the scrollbars native and this would make sure
 > they weren't overdrawn

Sure, but then they wouldn't be Emacs scrollbars.  XEmacs allows you
to use either Lucid scrollbars or the native ones for the toolkit
you're using; almost everybody[1] uses Lucid, and AFAICR almost
everybody who does care prefers this (actual usage is of course biased
by the choice of Lucid as default).

I suspect that going to native scrollbars would annoy a lot of users.

Footnotes: 
[1]  Of those who submit build reports, as well as my more or less
inaccurate memory of list posts etc.

Re: Native scrollbars? [was: visible-bell patch for Mac OS X]

[David De La Harpe Golden]

Stephen J. Turnbull wrote:
> David De La Harpe Golden writes:
> 
>  > Emacs could make the scrollbars native and this would make sure
>  > they weren't overdrawn
> 

[Just to be clear, I was quoting the gtk page, gtk+ guys wrote the above 
not me ]

> I suspect that going to native scrollbars would annoy a lot of users.
>

I'm not clear that's what the gtk+ guys mean in context by "native", 
however.  That wouldn't make sense in context, because there'd be no 
gtk+ drawn scrollbars for emacs to stomp on?

GNU Emacs indeed has a compile-time choice of "toolkit" (native as you 
[Stephen T.] mean it) scrollbars, or the emacs-internal ones.  The 
toolkit ones are significantly disliked, yes, however I think they're 
quite widely used, perhaps unlike the xemacs case.

So I think the gtk+/emacs bug may show up (I don't actually see it on my 
system, but my gtk+ might be too old)  with the _toolkit_ scrollbars 
from the emacs perspective, since emacs thinks they're a child X11 
window and thinks it's okay to clear the parent window, which perhaps
eats the gtk+ drawn scrollbar which isn't in a separate X11 child window 
anymore in recent gtk.

So I think gtk+ guys mean there's a way for emacs to request
gtk to continue to use "native" (real X11 child) windows at the gtk/gdk 
level:

Well, there is a way to do it app-globally- set env var GDK_NATIVE_WINDOWS=1
http://blogs.gurulabs.com/dax/2009/10/what-gdk-native.html

I'm not familiar enough with gtk to know if it's possible for emacs
to ask for only the gtk+ toolkit scrollbars to be "native" in the sense 
gtk+ guys seem to mean it, at least based on my current potentially 
incorrect understanding as above.

Re: Native scrollbars? [was: visible-bell patch for Mac OS X]

[David De La Harpe Golden]

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

David De La Harpe Golden wrote:

> So I think the gtk+/emacs bug may show up (I don't actually see it on my 
> system, but my gtk+ might be too old)


Okay, I haven't replicated the full scrollbar-trashing thing the gtk+ 
guys mention, (possibly emacs nowadays avoids drawing over the scrollbar 
area in question so it's not a problem?), but I think I now see the 
issue that Yamamoto Mitsuharu was probably actually talking about -

when visible-bell flashes the first line of the window, part of the 
scrollbar is also inverse-video-d unless I use
GDK_NATIVE_WINDOWS=1 emacs

Contrast top right and bottom right in the two attached screenshots -
note how part of the scrollbar is inverted in the _nonative one
(the default, without gtk instructed to use "native" windows).*

It's quite a minor and transient effect (on my system anyway).  Still, I 
guess the flash inversion area in xterm.c/XTflash() is winding up a bit 
off somehow - the intent of the code does seem to be that scrollbars 
should be excluded from the area, but maybe it's only working for some 
cases.

(* the color difference in the visible bell effect in the main and 
minibuffer window in each screenshot is not significant, just due to my 
use of global-hl-line-mode)







[-- Attachment #2: emacs_gtk_visiblebell_native.png --]
[-- Type: image/png, Size: 14898 bytes --]

[-- Attachment #3: emacs_gtk_visiblebell_nonative.png --]
[-- Type: image/png, Size: 15076 bytes --]

Re: Native scrollbars? [was: visible-bell patch for Mac OS X]

[YAMAMOTO Mitsuharu]

>>>>> On Sat, 13 Feb 2010 06:45:33 +0000, David De La Harpe Golden <david@harpegolden.net> said:

> It's quite a minor and transient effect (on my system anyway).
> Still, I guess the flash inversion area in xterm.c/XTflash() is
> winding up a bit off somehow - the intent of the code does seem to
> be that scrollbars should be excluded from the area, but maybe it's
> only working for some cases.

The current code just trims off the margins outside leftmost left (or
rightmost right) scroll bars.

Just adding more excluded area at the edge positions gives
inconsistent result as found in multiple patches proposed for the NS
port, because scroll bars might overlap with the flashed area at some
non-edge positions.  I think a consistent way is to avoid inversion
(or re-invert to recover the original image) for the overlapped area:

  http://lists.gnu.org/archive/html/emacs-devel/2010-02/msg00038.html

The above mail includes a patch for the Mac port using re-inversion,
and it can be ported to GTK+ as below.  Though I've never observed
flicker by re-inversion of overlapped area, that might happen in
principle (this is not an issue for the Mac port because of
double-buffering).  Maybe create a Pixmap for the inverted area first?

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

=== modified file 'src/xterm.c'
*** src/xterm.c	2010-01-19 04:32:47 +0000
--- src/xterm.c	2010-02-03 08:47:20 +0000
***************
*** 3037,3045 ****
        int flash_height = FRAME_LINE_HEIGHT (f);
        /* These will be the left and right margins of the rectangles.  */
        int flash_left = FRAME_INTERNAL_BORDER_WIDTH (f);
!       int flash_right = FRAME_PIXEL_WIDTH (f) - FRAME_INTERNAL_BORDER_WIDTH (f);
! 
        int width;
  
        /* Don't flash the area between a scroll bar and the frame
  	 edge it is next to.  */
--- 3037,3047 ----
        int flash_height = FRAME_LINE_HEIGHT (f);
        /* These will be the left and right margins of the rectangles.  */
        int flash_left = FRAME_INTERNAL_BORDER_WIDTH (f);
!       int flash_right = (FRAME_TEXT_COLS_TO_PIXEL_WIDTH (f, FRAME_COLS (f))
! 			 - FRAME_INTERNAL_BORDER_WIDTH (f));
        int width;
+       XRectangle rects[2];
+       int i, nrects;
  
        /* Don't flash the area between a scroll bar and the frame
  	 edge it is next to.  */
***************
*** 3059,3083 ****
  
        width = flash_right - flash_left;
  
-       /* If window is tall, flash top and bottom line.  */
        if (height > 3 * FRAME_LINE_HEIGHT (f))
  	{
! 	  XFillRectangle (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), gc,
! 			  flash_left,
! 			  (FRAME_INTERNAL_BORDER_WIDTH (f)
! 			   + FRAME_TOP_MARGIN_HEIGHT (f)),
! 			  width, flash_height);
! 	  XFillRectangle (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), gc,
! 			  flash_left,
! 			  (height - flash_height
! 			   - FRAME_INTERNAL_BORDER_WIDTH (f)),
! 			  width, flash_height);
  	}
        else
! 	/* If it is short, flash it all.  */
  	XFillRectangle (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), gc,
! 			flash_left, FRAME_INTERNAL_BORDER_WIDTH (f),
! 			width, height - 2 * FRAME_INTERNAL_BORDER_WIDTH (f));
  
        x_flush (f);
  
--- 3061,3113 ----
  
        width = flash_right - flash_left;
  
        if (height > 3 * FRAME_LINE_HEIGHT (f))
  	{
! 	  /* If window is tall, flash top and bottom line.  */
! 	  STORE_NATIVE_RECT (rects[0],
! 			     flash_left, (FRAME_INTERNAL_BORDER_WIDTH (f)
! 					  + FRAME_TOP_MARGIN_HEIGHT (f)),
! 			     width, flash_height);
! 	  rects[1] = rects[0];
! 	  rects[1].y = height - flash_height - FRAME_INTERNAL_BORDER_WIDTH (f);
! 	  nrects = 2;
  	}
        else
! 	{
! 	  /* If it is short, flash it all.  */
! 	  STORE_NATIVE_RECT (rects[0],
! 			     flash_left, FRAME_INTERNAL_BORDER_WIDTH (f),
! 			     width,
! 			     height - 2 * FRAME_INTERNAL_BORDER_WIDTH (f));
! 	  nrects = 1;
! 	}
! 
!       for (i = 0; i < nrects; i++)
  	XFillRectangle (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), gc,
! 			rects[i].x, rects[i].y,
! 			rects[i].width, rects[i].height);
! #ifdef USE_GTK
!       /* In case scroll bars don't have their own windows, we cancel
! 	 color flipping for scroll bar area.  */
!       if (FRAME_HAS_VERTICAL_SCROLL_BARS (f))
! 	{
! 	  Lisp_Object bar;
! 
! 	  for (bar = FRAME_SCROLL_BARS (f); !NILP (bar);
! 	       bar = XSCROLL_BAR (bar)->next)
! 	    {
! 	      struct scroll_bar *b = XSCROLL_BAR (bar);
! 	      XRectangle bar_rect, r;
! 
! 	      STORE_NATIVE_RECT (bar_rect, b->left, b->top,
! 				 b->width, b->height);
! 	      for (i = 0; i < nrects; i++)
! 		if (x_intersect_rectangles (rects + i, &bar_rect, &r))
! 		  XFillRectangle (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), gc,
! 				  r.x, r.y, r.width, r.height);
! 	    }
! 	}
! #endif	/* USE_GTK */
  
        x_flush (f);
  
***************
*** 3113,3137 ****
  	  }
        }
  
!       /* If window is tall, flash top and bottom line.  */
!       if (height > 3 * FRAME_LINE_HEIGHT (f))
  	{
! 	  XFillRectangle (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), gc,
! 			  flash_left,
! 			  (FRAME_INTERNAL_BORDER_WIDTH (f)
! 			   + FRAME_TOP_MARGIN_HEIGHT (f)),
! 			  width, flash_height);
! 	  XFillRectangle (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), gc,
! 			  flash_left,
! 			  (height - flash_height
! 			   - FRAME_INTERNAL_BORDER_WIDTH (f)),
! 			  width, flash_height);
  	}
!       else
! 	/* If it is short, flash it all.  */
! 	XFillRectangle (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), gc,
! 			flash_left, FRAME_INTERNAL_BORDER_WIDTH (f),
! 			width, height - 2 * FRAME_INTERNAL_BORDER_WIDTH (f));
  
        XFreeGC (FRAME_X_DISPLAY (f), gc);
        x_flush (f);
--- 3143,3172 ----
  	  }
        }
  
!       for (i = 0; i < nrects; i++)
! 	XFillRectangle (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), gc,
! 			rects[i].x, rects[i].y,
! 			rects[i].width, rects[i].height);
! #ifdef USE_GTK
!       if (FRAME_HAS_VERTICAL_SCROLL_BARS (f))
  	{
! 	  Lisp_Object bar;
! 
! 	  for (bar = FRAME_SCROLL_BARS (f); !NILP (bar);
! 	       bar = XSCROLL_BAR (bar)->next)
! 	    {
! 	      struct scroll_bar *b = XSCROLL_BAR (bar);
! 	      XRectangle bar_rect, r;
! 
! 	      STORE_NATIVE_RECT (bar_rect, b->left, b->top,
! 				 b->width, b->height);
! 	      for (i = 0; i < nrects; i++)
! 		if (x_intersect_rectangles (rects + i, &bar_rect, &r))
! 		  XFillRectangle (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), gc,
! 				  r.x, r.y, r.width, r.height);
! 	    }
  	}
! #endif	/* USE_GTK */
  
        XFreeGC (FRAME_X_DISPLAY (f), gc);
        x_flush (f);

Re: Native scrollbars? [was: visible-bell patch for Mac OS X]

[Jan Djärv]

David De La Harpe Golden skrev:
> David De La Harpe Golden wrote:
> 
> when visible-bell flashes the first line of the window, part of the 
> scrollbar is also inverse-video-d unless I use
> GDK_NATIVE_WINDOWS=1 emacs
> 
> Contrast top right and bottom right in the two attached screenshots -
> note how part of the scrollbar is inverted in the _nonative one
> (the default, without gtk instructed to use "native" windows).*
> 

I haven't seen this as I have visible bell enabled in my window manager.  But 
the Emacs one is actually quite nice, I might start using it!

Anyways, I checked in a fix for the scroll bar thing w.r.t. to visible bell.

	Jan D.

Re: Native scrollbars?

[David Kastrup]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:

> David De La Harpe Golden writes:
>
>  > Emacs could make the scrollbars native and this would make sure
>  > they weren't overdrawn
>
> Sure, but then they wouldn't be Emacs scrollbars.  XEmacs allows you
> to use either Lucid scrollbars or the native ones for the toolkit
> you're using; almost everybody[1] uses Lucid, and AFAICR almost
> everybody who does care prefers this (actual usage is of course biased
> by the choice of Lucid as default).

I think you are using "native" in opposite meanings.  David uses it for
"Emacs-specific", Stephen for "toolkit-specific".

> I suspect that going to native scrollbars would annoy a lot of users.

Toolkit native scrollbars are the default with Emacs.  They don't annoy
a lot of users apparently.  Probably also because the geek demographic
of Emacs is different from that of XEmacs.

FWIW:

    system-configuration-options is a variable defined in `C source code'.
    Its value is 
    " '--prefix=/usr/local/emacs' '--without-toolkit-scroll-bars' 'CFLAGS=-O2 -fno-crossjumping -g'"


My problem with toolkit scrollbars is that they are unergonomic and
stupid.  Scroll half a screen?  Scroll backward and forward by equal
amounts without moving mouse?  Just forget it.

The overall improvement is more than with XEmacs I should say, because
XEmacs is designed "Motif-like" for two mouse buttons in general.

Nevertheless, the default are toolkit scrollbars, and I don't think that
all too many people override it.  I think the Lucid scrollbars would be
an even bigger advantage on Windows where dragging is an absolute pain
IIRC because you need to keep on the scrollbar area even horizontally,
or the scrolling jumps back.  At least that was the case on Windows
3.11.

For me Lucid toolbars are important enough to recompile Emacs even on
GNU/Linux distributions where it would be available precompiled.  I also
use X resources to make them less awfully wide.  That's two changes I
consider inexpensible for nice operation out of the box.

I suspect that most mouse users on Emacs do not even know how to cut and
paste without reverting to the keyboard or menus.  So expecting them to
deal with an Athena-semantics scrollbar is probably a bit too much.

Pity.

-- 
David Kastrup

Re: Native scrollbars?

[Eli Zaretskii]

> From: David Kastrup <dak@gnu.org>
> Date: Sat, 13 Feb 2010 10:00:41 +0100
> 
> Nevertheless, the default are toolkit scrollbars, and I don't think that
> all too many people override it.  I think the Lucid scrollbars would be
> an even bigger advantage on Windows where dragging is an absolute pain
> IIRC because you need to keep on the scrollbar area even horizontally,
> or the scrolling jumps back.  At least that was the case on Windows
> 3.11.

On Windows XP the area where you need to keep the mouse pointer is
something like 10 times wider than the scroll bar itself.

Re: Native scrollbars?

[Jan Djärv]

David Kastrup skrev:

> Toolkit native scrollbars are the default with Emacs.  They don't annoy
> a lot of users apparently.  Probably also because the geek demographic
> of Emacs is different from that of XEmacs.
> 

IMHO, the XEmacs Lucid scroll bars look much better than Emacs native 
scrollbars.  I think that may be a factor also.

	Jan D.

Re: Native scrollbars?

[David Kastrup]

Jan Djärv <jan.h.d@swipnet.se> writes:

> David Kastrup skrev:
>
>> Toolkit native scrollbars are the default with Emacs.  They don't annoy
>> a lot of users apparently.  Probably also because the geek demographic
>> of Emacs is different from that of XEmacs.
>>
>
> IMHO, the XEmacs Lucid scroll bars look much better than Emacs native
> scrollbars.  I think that may be a factor also.

I was of the opinion that both used the same code (Lucid), so it would
probably just be a matter of the default settings.

Personally, I use

(setq-default scroll-bar-width 15)

-- 
David Kastrup

Re: Native scrollbars?

[Jan Djärv]

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

David Kastrup skrev:
> Jan Djärv <jan.h.d@swipnet.se> writes:
> 
>> David Kastrup skrev:
>>
>>> Toolkit native scrollbars are the default with Emacs.  They don't annoy
>>> a lot of users apparently.  Probably also because the geek demographic
>>> of Emacs is different from that of XEmacs.
>>>
>> IMHO, the XEmacs Lucid scroll bars look much better than Emacs native
>> scrollbars.  I think that may be a factor also.
> 
> I was of the opinion that both used the same code (Lucid), so it would
> probably just be a matter of the default settings.
> 
> Personally, I use
> 
> (setq-default scroll-bar-width 15)
> 

I don't think I have any settings that affects this. I attach a screen shot of 
lucid scroll bars in XEmacs (left) and Emacs (right).  The difference is 
apparent.  It could possibly be fixed by some X resource setting, I haven't tried.

Anyway, the Emacs native scroll bar isn't based on Lucid, and looks much worse.

	Jan D.


[-- Attachment #2: scrollbars.png --]
[-- Type: image/png, Size: 2012 bytes --]

Re: Native scrollbars?

[David Kastrup]

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

Jan Djärv <jan.h.d@swipnet.se> writes:

> David Kastrup skrev:
>> Jan Djärv <jan.h.d@swipnet.se> writes:
>>
>>> David Kastrup skrev:
>>>
>>>> Toolkit native scrollbars are the default with Emacs.  They don't annoy
>>>> a lot of users apparently.  Probably also because the geek demographic
>>>> of Emacs is different from that of XEmacs.
>>>>
>>> IMHO, the XEmacs Lucid scroll bars look much better than Emacs native
>>> scrollbars.  I think that may be a factor also.
>>
>> I was of the opinion that both used the same code (Lucid), so it would
>> probably just be a matter of the default settings.
>>
>> Personally, I use
>>
>> (setq-default scroll-bar-width 15)
>>
>
> I don't think I have any settings that affects this. I attach a screen
> shot of lucid scroll bars in XEmacs (left) and Emacs (right).  The
> difference is apparent.  It could possibly be fixed by some X resource
> setting, I haven't tried.
>
> Anyway, the Emacs native scroll bar isn't based on Lucid, and looks
> much worse.

Well, mine looks like

[-- Attachment #2: Screenshot-1.png --]
[-- Type: image/png, Size: 1208 bytes --]

[-- Attachment #3: Type: text/plain, Size: 599 bytes --]


and that's without-toolkit-scroll-bars.  So I was mistaken about this
using the Lucid code.  Incidentally, the scroll bar width of the
minibuffer appears unaffected from my settings, and this might be a
bug.  My screen might look a bit prettier otherwise.

So now I don't think it is an option to compile with GTK+ overall while
using Lucid scrollbars.  Sorry for the confusion.

I'm fine with the resulting ugliness in the Emacs-native scrollbars:
using Gtk+ for the rest certainly gives a much nicer appearance overall
than using either Athena Widgets or Lucid for everything.

-- 
David Kastrup

Re: Native scrollbars?

[Stephen J. Turnbull]

David Kastrup writes:

 > I was of the opinion that both used the same code (Lucid), so it would
 > probably just be a matter of the default settings.

I was going to say "It's highly unlikely that it would be the same
code any more" but actually checking the ChangeLog, the word scrollbar
is mentioned about 2 dozen times, of which over half are associated
with logs of the form "will GCC *ever* shut up? warning suppression"
:-) so I have to confirm your guess.

So while I appreciate Jan's compliment, whatever it is that's better,
it's not the scrollbars themselves.

Re: visible-bell patch for Mac OS X

[YAMAMOTO Mitsuharu]

>>>>> On Sat, 13 Feb 2010 01:51:59 +0000, David De La Harpe Golden <david@harpegolden.net> said:

> YAMAMOTO Mitsuharu wrote:
>> BTW, I noticed that the GTK+ build may also flash part of scroll
>> bars if GDK_NATIVE_WINDOWS is not set.

> Is this a "known" (maybe gtk+ people did not see fit to mention it
> through any sensible channels to emacs people?) scrollbar issue:

> Gtk+, like Qt [1], has moved away from most use of X11 subwindows.

> see: http://live.gnome.org/GTK%2B/ClientSideWindows

> * Problems that will require application changes

>      * emacs 23 (with gtk): o Sometimes when pressing Ctrl-L the
> scrollbars are overdrawn with white and not repainted. This is due
> to emacs using raw X primitives to draw on the window, while the
> scrollbars are client side emulated windows. Emacs could make the
> scrollbars native and this would make sure they weren't overdrawn

I think Jan already worked hard to cope with client-side-windows for
many crucial cases, and this flashing over scroll bars in visible bell
is relatively minor.

Another minor issue I've just found is disappearance of scroll bar
when dragging an edge between mode lines horizontally (i.e., to change
the width of windows):

  1. emacs -Q
  2. C-x 3
  (2.5. Optionally, C-x o followed by C-x 2)
  3. Drag the edge between the mode lines at the bottom center in
     order to move the scroll bar horizontally.

Then the scroll bar at the center disappears until you stop dragging.
This does not happen when GDK_NATIVE_WINDOWS=1.

(It would also be interesting to compare the result of the above
experiment between NS and Mac ports.)

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

Re: visible-bell patch for Mac OS X

[David De La Harpe Golden]

YAMAMOTO Mitsuharu wrote:


> I think Jan already worked hard to cope with client-side-windows for
> many crucial cases, 

Ah, that would explain why I didn't see the issue as the gtk+ wiki page 
described it.

Re: visible-bell patch for Mac OS X

[Jan Djärv]

YAMAMOTO Mitsuharu skrev:

> 
> Another minor issue I've just found is disappearance of scroll bar
> when dragging an edge between mode lines horizontally (i.e., to change
> the width of windows):
> 
>   1. emacs -Q
>   2. C-x 3
>   (2.5. Optionally, C-x o followed by C-x 2)
>   3. Drag the edge between the mode lines at the bottom center in
>      order to move the scroll bar horizontally.
> 
> Then the scroll bar at the center disappears until you stop dragging.
> This does not happen when GDK_NATIVE_WINDOWS=1.
> 
> (It would also be interesting to compare the result of the above
> experiment between NS and Mac ports.)

Can you file this as a bug?

David De La Harpe Golden writes:

> So I think gtk+ guys mean there's a way for emacs to request
> gtk to continue to use "native" (real X11 child) windows at the gtk/gdk level:
> 
> Well, there is a way to do it app-globally- set env var GDK_NATIVE_WINDOWS=1 

It can be done programmatically without resorting to globally use 
GDK_NATIVE_WINDOWS=1.  The issue here being, is this just a temporary fix, 
i.e. will this option go away in future Gtk+ versions?  My gut feeling is yes, 
so I tried to not go that way.

	Jan D.