XEmbed patches

XEmbed patches

[Timo Savola]

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

Here is the XEmbed protocol support I wrote last year [1].  RMS asked me to
bring it up again after the release of Emacs 22 [2].

[1] http://lists.gnu.org/archive/html/emacs-devel/2006-02/msg00017.html
[2] http://lists.gnu.org/archive/html/emacs-devel/2006-02/msg00162.html

There are five incremental patches plus one additional one
(gtk-scrollbar-position) which I like for aesthetic reasons.  The code
hasn't been changed since the last submission (if I remember correctly), but
I've regenerated the patches against 22.1.  The patches can also be viewed
in a Git repository at http://timo.stc.cx/git/?p=emacs-xembed.git .

timo

[-- Attachment #2: 0001-parent-id.patch --]
[-- Type: text/plain, Size: 2578 bytes --]

>From d07ad110053dc7193440f53317195bfdb5ecdfaa Mon Sep 17 00:00:00 2001
From: Timo Savola <timo.savola@iki.fi>
Date: Tue, 23 Jan 2007 11:55:02 +0200
Subject: [PATCH] parent-id

The --parent-id command-line option sets the parent window ID of the
initial frame.
---
 lisp/startup.el    |    3 ++-
 lisp/term/x-win.el |    9 +++++++++
 src/emacs.c        |    2 ++
 3 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/lisp/startup.el b/lisp/startup.el
index f57bf66..09cd77e 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -151,7 +151,8 @@ This is normally copied from `default-directory' when Emacs starts.")
     ("--vertical-scroll-bars" 0 x-handle-switch vertical-scroll-bars t)
     ("--line-spacing" 1 x-handle-numeric-switch line-spacing)
     ("--border-color" 1 x-handle-switch border-color)
-    ("--smid" 1 x-handle-smid))
+    ("--smid" 1 x-handle-smid)
+    ("--parent-id" 1 x-handle-parent-id))
   "Alist of X Windows options.
 Each element has the form
   (NAME NUMARGS HANDLER FRAME-PARAM VALUE)
diff --git a/lisp/term/x-win.el b/lisp/term/x-win.el
index e48d97d..390c671 100644
--- a/lisp/term/x-win.el
+++ b/lisp/term/x-win.el
@@ -181,6 +181,15 @@
   (setq initial-frame-alist (cons (cons 'name x-resource-name)
 				  initial-frame-alist)))
 
+;; Handle the --parent-id option.
+(defun x-handle-parent-id (switch)
+  (or (consp x-invocation-args)
+      (error "%s: missing argument to `%s' option" (invocation-name) switch))
+  (setq parent-id (string-to-number (car x-invocation-args))
+	x-invocation-args (cdr x-invocation-args))
+  (setq initial-frame-alist (cons (cons 'parent-id parent-id)
+				  initial-frame-alist)))
+
 (defvar x-display-name nil
   "The name of the X display on which Emacs was started.
 
diff --git a/src/emacs.c b/src/emacs.c
index 62b14a5..7f42411 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -325,6 +325,7 @@ Display options:\n\
 --title, -T TITLE               title for initial Emacs frame\n\
 --vertical-scroll-bars, -vb     enable vertical scroll bars\n\
 --xrm XRESOURCES                set additional X resources\n\
+--parent-id XID                 set parent window\n\
 --help                          display this help and exit\n\
 --version                       output version information and exit\n\
 \n"
@@ -1835,6 +1836,7 @@ struct standard_args standard_args[] =
   { "-title", 0, 10, 1 },
   { "-name", "--name", 10, 1 },
   { "-xrm", "--xrm", 10, 1 },
+  { "-parent-id", "--parent-id", 10, 1 },
   { "-r", "--reverse-video", 5, 0 },
   { "-rv", 0, 5, 0 },
   { "-reverse", 0, 5, 0 },
-- 
1.5.2


[-- Attachment #3: 0002-parent-fix.patch --]
[-- Type: text/plain, Size: 829 bytes --]

>From e28d6f61c75ce2f4ec92516a44af77f15cfcf16a Mon Sep 17 00:00:00 2001
From: Timo Savola <timo.savola@iki.fi>
Date: Tue, 23 Jan 2007 11:55:35 +0200
Subject: [PATCH] parent-fix

Do not override the explicitly set parent window ID of a frame.
---
 src/xfns.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/src/xfns.c b/src/xfns.c
index d269dfb..104b513 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -3334,8 +3334,6 @@ This function is an internal primitive--use `make-frame' instead.  */)
   x_default_parameter (f, parms, Qfullscreen, Qnil,
                        "fullscreen", "Fullscreen", RES_TYPE_SYMBOL);
 
-  f->output_data.x->parent_desc = FRAME_X_DISPLAY_INFO (f)->root_window;
-
   /* Compute the size of the X window.  */
   window_prompting = x_figure_window_size (f, parms, 1);
 
-- 
1.5.2


[-- Attachment #4: 0003-xembed-support.patch --]
[-- Type: text/plain, Size: 7970 bytes --]

>From 08f07fb2baad3122e8b4b822803fce2b33d95ece Mon Sep 17 00:00:00 2001
From: Timo Savola <timo.savola@iki.fi>
Date: Tue, 23 Jan 2007 11:55:54 +0200
Subject: [PATCH] xembed-support

XEmbed protocol support for Xlib, X-toolkit and GTK+ versions.
---
 src/gtkutil.c |    6 +++-
 src/xfns.c    |    4 ++
 src/xterm.c   |   91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 src/xterm.h   |   63 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 160 insertions(+), 4 deletions(-)

diff --git a/src/gtkutil.c b/src/gtkutil.c
index 02e42f7..bb52ebb 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -769,7 +769,11 @@ xg_create_frame_widgets (f)
 
   BLOCK_INPUT;
 
-  wtop = gtk_window_new (GTK_WINDOW_TOPLEVEL);
+  if (FRAME_X_EMBEDDED_P (f))
+    wtop = gtk_plug_new (f->output_data.x->parent_desc);
+  else
+    wtop = gtk_window_new (GTK_WINDOW_TOPLEVEL);
+
   xg_set_screen (wtop, f);
 
   wvbox = gtk_vbox_new (FALSE, 0);
diff --git a/src/xfns.c b/src/xfns.c
index 104b513..716b282 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -2604,6 +2604,10 @@ x_window (f, window_prompting, minibuffer_only)
   XtManageChild (pane_widget);
   XtRealizeWidget (shell_widget);
 
+  if (FRAME_X_EMBEDDED_P (f))
+    XReparentWindow (FRAME_X_DISPLAY (f), XtWindow (shell_widget),
+		     f->output_data.x->parent_desc, 0, 0);
+
   FRAME_X_WINDOW (f) = XtWindow (frame_widget);
 
   validate_x_resource_name ();
diff --git a/src/xterm.c b/src/xterm.c
index bc4fd36..8764ada 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -3306,6 +3306,15 @@ x_detect_focus_change (dpyinfo, event, bufp)
 			FOCUS_IMPLICIT : FOCUS_EXPLICIT),
 		       dpyinfo, frame, bufp);
       break;
+
+    case ClientMessage:
+      if (event->xclient.message_type == dpyinfo->Xatom_XEMBED)
+	{
+	  enum xembed_message msg = event->xclient.data.l[1];
+	  x_focus_changed ((msg == XEMBED_FOCUS_IN ? FocusIn : FocusOut),
+			   FOCUS_EXPLICIT, dpyinfo, frame, bufp);
+	}
+      break;
     }
 }
 
@@ -5876,6 +5885,18 @@ handle_one_xevent (dpyinfo, eventp, finish, hold_quit)
           }
 #endif /* USE_TOOLKIT_SCROLL_BARS */
 
+	/* XEmbed messages from the embedder (if any).  */
+        if (event.xclient.message_type
+	    == dpyinfo->Xatom_XEMBED)
+          {
+	    enum xembed_message msg = event.xclient.data.l[1];
+	    if (msg == XEMBED_FOCUS_IN || msg == XEMBED_FOCUS_OUT)
+	      x_detect_focus_change (dpyinfo, &event, &inev.ie);
+
+	    *finish = X_EVENT_GOTO_OUT;
+            goto done;
+          }
+
 	f = x_any_window_to_frame (dpyinfo, event.xclient.window);
 	if (!f)
 	  goto OTHER;
@@ -6796,6 +6817,10 @@ handle_one_xevent (dpyinfo, eventp, finish, hold_quit)
 		      else
 			construct_mouse_click (&inev.ie, &event.xbutton, f);
 		    }
+
+		  if (FRAME_X_EMBEDDED_P (f))
+		    xembed_send_message (f, event.xbutton.time,
+					 XEMBED_REQUEST_FOCUS, 0, 0, 0);
                 }
           }
         else
@@ -8889,6 +8914,51 @@ XTframe_raise_lower (f, raise_flag)
     x_lower_frame (f);
 }
 \f
+/* XEmbed implementation.  */
+
+void
+xembed_set_info (f, flags)
+     struct frame *f;
+     enum xembed_info flags;
+{
+  Atom atom;
+  unsigned long data[2];
+
+  atom = XInternAtom (FRAME_X_DISPLAY (f), "_XEMBED_INFO", False);
+
+  data[0] = XEMBED_VERSION;
+  data[1] = flags;
+
+  XChangeProperty (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f), atom, atom,
+		   32, PropModeReplace, (unsigned char *) data, 2);
+}
+
+void
+xembed_send_message (f, time, message, detail, data1, data2)
+     struct frame *f;
+     Time time;
+     enum xembed_message message;
+     long detail;
+     long data1;
+     long data2;
+{
+  XEvent event;
+
+  event.xclient.type = ClientMessage;
+  event.xclient.window = FRAME_X_OUTPUT (f)->parent_desc;
+  event.xclient.message_type = FRAME_X_DISPLAY_INFO (f)->Xatom_XEMBED;
+  event.xclient.format = 32;
+  event.xclient.data.l[0] = time;
+  event.xclient.data.l[1] = message;
+  event.xclient.data.l[2] = detail;
+  event.xclient.data.l[3] = data1;
+  event.xclient.data.l[4] = data2;
+
+  XSendEvent (FRAME_X_DISPLAY (f), FRAME_X_OUTPUT (f)->parent_desc,
+	      False, NoEventMask, &event);
+  XSync (FRAME_X_DISPLAY (f), False);
+}
+\f
 /* Change of visibility.  */
 
 /* This tries to wait until the frame is really visible.
@@ -8929,14 +8999,22 @@ x_make_frame_visible (f)
       if (! EQ (Vx_no_window_manager, Qt))
 	x_wm_set_window_state (f, NormalState);
 #ifdef USE_X_TOOLKIT
-      /* This was XtPopup, but that did nothing for an iconified frame.  */
-      XtMapWidget (f->output_data.x->widget);
+      if (FRAME_X_EMBEDDED_P (f))
+	xembed_set_info (f, XEMBED_MAPPED);
+      else
+	{
+	  /* This was XtPopup, but that did nothing for an iconified frame.  */
+	  XtMapWidget (f->output_data.x->widget);
+	}
 #else /* not USE_X_TOOLKIT */
 #ifdef USE_GTK
       gtk_widget_show_all (FRAME_GTK_OUTER_WIDGET (f));
       gtk_window_deiconify (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)));
 #else
-      XMapRaised (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f));
+      if (FRAME_X_EMBEDDED_P (f))
+	xembed_set_info (f, XEMBED_MAPPED);
+      else
+	XMapRaised (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f));
 #endif /* not USE_GTK */
 #endif /* not USE_X_TOOLKIT */
 #if 0 /* This seems to bring back scroll bars in the wrong places
@@ -9088,6 +9166,10 @@ x_make_frame_invisible (f)
   if (FRAME_GTK_OUTER_WIDGET (f))
     gtk_widget_hide (FRAME_GTK_OUTER_WIDGET (f));
   else
+#else
+  if (FRAME_X_EMBEDDED_P (f))
+    xembed_set_info (f, 0);
+  else
 #endif
   {
 #ifdef HAVE_X11R4
@@ -10851,6 +10933,9 @@ x_term_init (display_name, xrm_option, resource_name)
   dpyinfo->Xatom_Scrollbar = XInternAtom (dpyinfo->display, "SCROLLBAR",
 					  False);
 
+  dpyinfo->Xatom_XEMBED = XInternAtom (dpyinfo->display, "_XEMBED",
+				       False);
+
   dpyinfo->cut_buffers_initialized = 0;
 
   dpyinfo->x_dnd_atoms_size = 8;
diff --git a/src/xterm.h b/src/xterm.h
index 13b0b49..a4920aa 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -327,6 +327,9 @@ struct x_display_info
   /* Atom used in toolkit scroll bar client messages.  */
   Atom Xatom_Scrollbar;
 
+  /* Atom used in XEmbed client messages.  */
+  Atom Xatom_XEMBED;
+
 #ifdef MULTI_KBOARD
   struct kboard *kboard;
 #endif
@@ -1101,6 +1104,66 @@ extern int x_session_check_input P_ ((struct input_event *bufp));
 extern int x_session_have_connection P_ ((void));
 #endif
 
+/* XEmbed implementation.  */
+
+#define XEMBED_VERSION 0
+
+enum xembed_info
+  {
+    XEMBED_MAPPED = 1 << 0
+  };
+
+enum xembed_message
+  {
+    XEMBED_EMBEDDED_NOTIFY        = 0,
+    XEMBED_WINDOW_ACTIVATE        = 1,
+    XEMBED_WINDOW_DEACTIVATE      = 2,
+    XEMBED_REQUEST_FOCUS          = 3,
+    XEMBED_FOCUS_IN               = 4,
+    XEMBED_FOCUS_OUT              = 5,
+    XEMBED_FOCUS_NEXT             = 6,
+    XEMBED_FOCUS_PREV             = 7,
+
+    XEMBED_MODALITY_ON            = 10,
+    XEMBED_MODALITY_OFF           = 11,
+    XEMBED_REGISTER_ACCELERATOR   = 12,
+    XEMBED_UNREGISTER_ACCELERATOR = 13,
+    XEMBED_ACTIVATE_ACCELERATOR   = 14
+  };
+
+enum xembed_focus
+  {
+    XEMBED_FOCUS_CURRENT = 0,
+    XEMBED_FOCUS_FIRST   = 1,
+    XEMBED_FOCUS_LAST    = 2
+  };
+
+enum xembed_modifier
+  {
+    XEMBED_MODIFIER_SHIFT   = 1 << 0,
+    XEMBED_MODIFIER_CONTROL = 1 << 1,
+    XEMBED_MODIFIER_ALT     = 1 << 2,
+    XEMBED_MODIFIER_SUPER   = 1 << 3,
+    XEMBED_MODIFIER_HYPER   = 1 << 4
+  };
+
+enum xembed_accelerator
+  {
+    XEMBED_ACCELERATOR_OVERLOADED = 1 << 0
+  };
+
+/* Defined in xterm.c */
+
+extern void xembed_set_info P_ ((struct frame *f, enum xembed_info flags));
+extern void xembed_send_message P_ ((struct frame *f, Time time,
+				     enum xembed_message message,
+				     long detail, long data1, long data2));
+
+/* Is the frame embedded into another application? */
+
+#define FRAME_X_EMBEDDED_P(f) (FRAME_X_OUTPUT(f)->explicit_parent != 0)
+
+
 #define FONT_TYPE_FOR_UNIBYTE(font, ch) 0
 #define FONT_TYPE_FOR_MULTIBYTE(font, ch) 0
 
-- 
1.5.2


[-- Attachment #5: 0004-xembed-xsetoffset.patch --]
[-- Type: text/plain, Size: 1575 bytes --]

>From 76561259959f81a4aefa45c5636bd13819e0bfe2 Mon Sep 17 00:00:00 2001
From: Timo Savola <timo.savola@iki.fi>
Date: Tue, 23 Jan 2007 11:56:29 +0200
Subject: [PATCH] xembed-xsetoffset

Do not attempt to move embedded frames.
---
 src/xterm.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/xterm.c b/src/xterm.c
index 8764ada..db6a353 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -8991,6 +8991,7 @@ x_make_frame_visible (f)
 	 if we get to x_make_frame_visible a second time
 	 before the window gets really visible.  */
       if (! FRAME_ICONIFIED_P (f)
+	  && ! FRAME_X_EMBEDDED_P (f)
 	  && ! f->output_data.x->asked_for_visible)
 	x_set_offset (f, f->left_pos, f->top_pos, 0);
 
@@ -9055,7 +9056,9 @@ x_make_frame_visible (f)
        because the window manager may choose the position
        and we don't want to override it.  */
 
-    if (! FRAME_VISIBLE_P (f) && ! FRAME_ICONIFIED_P (f)
+    if (! FRAME_VISIBLE_P (f)
+	&& ! FRAME_ICONIFIED_P (f)
+	&& ! FRAME_X_EMBEDDED_P (f)
 	&& f->win_gravity == NorthWestGravity
 	&& previously_visible)
       {
@@ -9300,7 +9303,9 @@ x_iconify_frame (f)
 
   /* Make sure the X server knows where the window should be positioned,
      in case the user deiconifies with the window manager.  */
-  if (! FRAME_VISIBLE_P (f) && !FRAME_ICONIFIED_P (f))
+  if (! FRAME_VISIBLE_P (f)
+      && ! FRAME_ICONIFIED_P (f)
+      && ! FRAME_X_EMBEDDED_P (f))
     x_set_offset (f, f->left_pos, f->top_pos, 0);
 
   /* Since we don't know which revision of X we're running, we'll use both
-- 
1.5.2


[-- Attachment #6: 0005-xembed-border.patch --]
[-- Type: text/plain, Size: 1130 bytes --]

>From a4e7df68f81c497830d44479546756aeacf8abc2 Mon Sep 17 00:00:00 2001
From: Timo Savola <timo.savola@iki.fi>
Date: Tue, 23 Jan 2007 11:57:31 +0200
Subject: [PATCH] xembed-border

Fixes issues with the Xlib and X-toolkit versions by ignoring the
BorderWidth parameter of embedded X windows.
---
 src/xfns.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/xfns.c b/src/xfns.c
index 716b282..acc40ed 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -3266,8 +3266,10 @@ This function is an internal primitive--use `make-frame' instead.  */)
   xlwmenu_default_font = FRAME_FONT (f);
 #endif
 
-  x_default_parameter (f, parms, Qborder_width, make_number (2),
-		       "borderWidth", "BorderWidth", RES_TYPE_NUMBER);
+  /* Frame contents get displaced if an embedded X window has a border.  */
+  if (! FRAME_X_EMBEDDED_P (f))
+    x_default_parameter (f, parms, Qborder_width, make_number (2),
+			 "borderWidth", "BorderWidth", RES_TYPE_NUMBER);
 
   /* This defaults to 1 in order to match xterm.  We recognize either
      internalBorderWidth or internalBorder (which is what xterm calls
-- 
1.5.2


[-- Attachment #7: 0006-gtk-scrollbar-position.patch --]
[-- Type: text/plain, Size: 879 bytes --]

>From f16f211a0810ff0f09a53dd4036c1c9df5841c85 Mon Sep 17 00:00:00 2001
From: Timo Savola <timo.savola@iki.fi>
Date: Tue, 23 Jan 2007 11:59:04 +0200
Subject: [PATCH] gtk-scrollbar-position

Aligns GTK+ scrollbars to the frame border.
---
 src/xterm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/xterm.c b/src/xterm.c
index db6a353..d53455e 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -5010,12 +5010,12 @@ XTset_vertical_scroll_bar (w, portion, whole, position)
   if (WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_RIGHT (w))
     sb_left = (left +
 	       (WINDOW_RIGHTMOST_P (w)
-		? width - sb_width - (width - sb_width) / 2
+		? width - sb_width
 		: 0));
   else
     sb_left = (left +
 	       (WINDOW_LEFTMOST_P (w)
-		? (width - sb_width) / 2
+		? 0
 		: width - sb_width));
 #else
   if (WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_RIGHT (w))
-- 
1.5.2


[-- Attachment #8: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: XEmbed patches

[Jan Djärv]

Timo Savola skrev:
> Here is the XEmbed protocol support I wrote last year [1].  RMS asked me to
> bring it up again after the release of Emacs 22 [2].
> 
> [1] http://lists.gnu.org/archive/html/emacs-devel/2006-02/msg00017.html
> [2] http://lists.gnu.org/archive/html/emacs-devel/2006-02/msg00162.html
> 
> There are five incremental patches plus one additional one
> (gtk-scrollbar-position) which I like for aesthetic reasons.  The code
> hasn't been changed since the last submission (if I remember correctly), but
> I've regenerated the patches against 22.1.  The patches can also be viewed
> in a Git repository at http://timo.stc.cx/git/?p=emacs-xembed.git .
> 

It looks OK to me.  It is good that you use gtk_plug for the Gtk+ case.  I'm 
sure that some issues will come up when this code gets tested, but the general 
approach looks good.

One thing that would be cool to have is to be able to embed a frame from 
emacsclient.  You can do that now with frame parameters and eval, but I was 
thinking for something like a defun create-embedded-frame or something like 
that.  Just to simplify things.

	Jan D.

Re: XEmbed patches

[Richard Stallman]

    One thing that would be cool to have is to be able to embed a
    frame from emacsclient.  You can do that now with frame parameters
    and eval, but I was thinking for something like a defun
    create-embedded-frame or something like that.  Just to simplify
    things.

Maybe you are right, but I'm not sure.

emacsclient is designed for programs that want to "invoke an editor"
without knowing which one.

By contrast, an Emacs window in the frame of some other app
would be created by an app that knows what it is doing.
Is emacsclient the right interface for that?

Perhaps what you mean is that the app should be able to ask an
existing Emacs to create the window, rather than starting a new Emacs.
The emacsclient mechanism might be the right way to do that.
But the interface to use is a separate question.

Re: XEmbed patches

[Jan Djärv]

Timo Savola skrev:
> Here is the XEmbed protocol support I wrote last year [1].  RMS asked me to
> bring it up again after the release of Emacs 22 [2].
> 
> [1] http://lists.gnu.org/archive/html/emacs-devel/2006-02/msg00017.html
> [2] http://lists.gnu.org/archive/html/emacs-devel/2006-02/msg00162.html
> 
> There are five incremental patches plus one additional one
> (gtk-scrollbar-position) which I like for aesthetic reasons.  The code
> hasn't been changed since the last submission (if I remember correctly), but
> I've regenerated the patches against 22.1.  The patches can also be viewed
> in a Git repository at http://timo.stc.cx/git/?p=emacs-xembed.git .

Oh, I forgot.  I think the Gtk+ scrollbar patch looks bad if you have a theme 
where the scroll bar isn't as wide as the character width.  The current 
behaviour puts the excess on both sides.  With your patch, the excess are on 
one side only.  But if we can get rid of the requirement that scroll bar width 
must equal a character width, this would not be an issue.

	Jan D.

Re: XEmbed patches

[Timo Savola]

On Fri, Jul 20, 2007 at 08:53:46AM +0200, Jan Djärv wrote:

> Oh, I forgot.  I think the Gtk+ scrollbar patch looks bad if you have a 
> theme where the scroll bar isn't as wide as the character width.  The 
> current behaviour puts the excess on both sides.  With your patch, the 
> excess are on one side only.

The point of that patch is to make the embedded Emacs blend into the
surrounding (assumedly) GTK application.  Without it, it leaves a gap
between the GTK-themed background color of the scrollbar and the GTK-themed
embedder window.  Making the gap wider on the Emacs side seems like a
smaller bad.

(Aligning the scrollbar seamlessly against the X window border might also
look better with some window manager themes designed for GTK-based
desktops.)

My patch changes that unconditionally--not just for embedded frames.  Also,
the assumption that the embedder is always a GTK application doesn't hold
water.

I do see this as an issue, however, but I don't know the correct solution.
Here are my proposed solutions:

 1. Enable it on embedded frames.

 2. Make it configurable so that the embedder which launches Emacs can
    choose it.  (This seems like a huge hammer for such a small nail as
    we're talking about a non-functional issue which exists with only one of
    the supported GUI toolkits.)

 3. All of the above.

> But if we can get rid of the requirement that 
> scroll bar width must equal a character width, this would not be an issue.

Is this a realistic expectation?

timo

Re: XEmbed patches

[Jason Rumney]

Timo Savola wrote:
> I do see this as an issue, however, but I don't know the correct solution.
> Here are my proposed solutions:
>
>  1. Enable it on embedded frames.
>
>  2. Make it configurable so that the embedder which launches Emacs can
>     choose it.  (This seems like a huge hammer for such a small nail as
>     we're talking about a non-functional issue which exists with only one of
>     the supported GUI toolkits.)
>
>  3. All of the above.
>   


A better solution would be to remove the restriction that frames must be
an integer number of characters wide (and high). Then the scrollbar and
toolbar can be made the correct size without empty filler padding them
out to a multiple of the character size.

Re: XEmbed patches

[Timo Savola]

On Fri, Jul 20, 2007 at 09:40:28AM +0100, Jason Rumney wrote:

> A better solution would be to remove the restriction that frames must be
> an integer number of characters wide (and high). Then the scrollbar and
> toolbar can be made the correct size without empty filler padding them
> out to a multiple of the character size.

Sure, if it's doable.

timo

Re: XEmbed patches

[Jason Rumney]

Timo Savola wrote:
> On Fri, Jul 20, 2007 at 09:40:28AM +0100, Jason Rumney wrote:
>
>   
>> A better solution would be to remove the restriction that frames must be
>> an integer number of characters wide (and high). Then the scrollbar and
>> toolbar can be made the correct size without empty filler padding them
>> out to a multiple of the character size.
>>     
>
> Sure, if it's doable.
>   

Don't you need that anyway for embedding?

Re: XEmbed patches

[Timo Savola]

On Fri, Jul 20, 2007 at 09:47:41AM +0100, Jason Rumney wrote:
> Timo Savola wrote:
> > On Fri, Jul 20, 2007 at 09:40:28AM +0100, Jason Rumney wrote:
> >
> >> A better solution would be to remove the restriction that frames must be
> >> an integer number of characters wide (and high). Then the scrollbar and
> >> toolbar can be made the correct size without empty filler padding them
> >> out to a multiple of the character size.
> >
> > Sure, if it's doable.
> 
> Don't you need that anyway for embedding?

Strictly speaking, no.  Emacs may request that, but it gets what it gets and
the remains are filled with the X window's background color...  Of course
this is also a visual problem (arguably bigger one than the scrollbar-gap
issue), but I haven't really thought about it because I've always resized
the panes in the embedders I've used ([1] and [2]) so that the space
reserved for Emacs is just right.

Now that you mentioned it, it is a big issue.

[1] http://encode.sourceforge.net/
[2] http://pida.co.uk/

timo

Re: XEmbed patches

[David Kastrup]

Jason Rumney <jasonr@gnu.org> writes:

> A better solution would be to remove the restriction that frames must be
> an integer number of characters wide (and high). Then the scrollbar and
> toolbar can be made the correct size without empty filler padding them
> out to a multiple of the character size.

When resizing an Emacs frame, the window manager's display of the
actual size and a character-size granularity are nice to have.
However, the displayed geometry clearly does not count in the
decorations from Emacs itself (scrollbar, fringes), so there is
apparently already a mechanism in place in the window-manager
communication that can discount decorations, and it is not unlikely
that the decorations need not have character cell size.

For embedding, however, one might need to pad or fill still.

-- 
David Kastrup

Re: XEmbed patches

[Jan Djärv]

David Kastrup skrev:
> Jason Rumney <jasonr@gnu.org> writes:
> 
>> A better solution would be to remove the restriction that frames must be
>> an integer number of characters wide (and high). Then the scrollbar and
>> toolbar can be made the correct size without empty filler padding them
>> out to a multiple of the character size.
> 
> When resizing an Emacs frame, the window manager's display of the
> actual size and a character-size granularity are nice to have.
> However, the displayed geometry clearly does not count in the
> decorations from Emacs itself (scrollbar, fringes), so there is
> apparently already a mechanism in place in the window-manager
> communication that can discount decorations, and it is not unlikely
> that the decorations need not have character cell size.

You set hints to the window manager how big an increment (character size) is 
in x and y directions.  You also tell it the minimum size (i.e. the size of 
the scrollbars, fringes, menu and toolbar) of your window.  That way the WM 
can calculate the character increment and display that when the window is resized.

> 
> For embedding, however, one might need to pad or fill still.

That is true.

	Jan D.

Re: XEmbed patches

[Jason Rumney]

Jan Djärv wrote:
> You set hints to the window manager how big an increment (character
> size) is in x and y directions.  You also tell it the minimum size
> (i.e. the size of the scrollbars, fringes, menu and toolbar) of your
> window.  That way the WM can calculate the character increment and
> display that when the window is resized.


Can this be uncoupled from the actual frame size restrictions? It would
be good to retain this behaviour for resizing frame with the mouse but
allowing programmatic resizing to specific pixel sizes.

And can Emacs switch this behaviour on and off at will? If the user
selects a variable width font as the default font, then the character
increment is not very useful.

Re: XEmbed patches

[Stephen J. Turnbull]

Jason Rumney writes:

 > And can Emacs switch this behaviour [of sizing in character units]
 > on and off at will?

[All windows below are window-system windows, not Emacs windows, which
are irrelevant here.]

It doesn't work that way, not if Emacs is an ICCCM-conformant
application.  The window manager tells Emacs, in pixels, where and how
big its windows are going to be, and it has the power to enforce this
(by reparenting Emacs's top-level window as a subwindow of a window it
owns, and clipping Emacs's window to the window manager's preferred
geometry).  It's Emacs's problem to fit into that space.  If Emacs
wants to, it can make its window arbitrarily big (up to the limit
imposed by the size of the X.h Dimension typedef, often 16 bits), but
the user will still only see that part that fits into the space
provided by the window manager.

If Emacs's desired window is smaller than what it is given, any "slop"
will have to be dealt with.  An image viewer might prefer to center
its display, but typically a text application will align the top-left
corner of the top-left character's bounding box with the origin
(top-left) of the window.  The right side is typically ragged, so a
few pixels there will be unnoticed, and (if the text extends that far)
the bottom line is typically displayed, and then clipped to the window
provided.

The use of character size (which is an arbitrary pair of dimensions
selected by the programmer, doesn't need to correspond to an actual
character cell) is purely a convenience for the user.  A window
manager can, but need not, respect the nominal character grid.  That's
entirely up to the window manager.  Especially in full-screen
maximization, window managers often choose to ignore the grid.  And
for GUI apps the grid is kinda silly anyway, since it's unlikely that
the decorations will be integral multiples of the font size (which may
not determine interlinear spacing in the presence of faces or images,
anyway).

Presumably the XEmbed parent has similar power, and Emacs can deal
with it in pretty much exactly the same way that it deals with the
window manager.  Just remember these words: "Yes, Boss!" and you'll
get along with the typical window manager just fine.<wink>

 > If the user selects a variable width font as the default font, then
 > the character increment is not very useful.

Sure it is.  Variable width fonts still have a consistent height, and
you can assume the standardized aspect ratio of 2:1.  This is
noticably wider than would correspond to 80-character lines in most
proportional fonts, but it's usually far better to underestimate the
number of characters that will fit in the window than to overestimate.

Re: XEmbed patches

[Richard Stallman]

    A better solution would be to remove the restriction that frames must be
    an integer number of characters wide (and high). Then the scrollbar and
    toolbar can be made the correct size without empty filler padding them
    out to a multiple of the character size.

We certainly want to do this, but we would need also to eliminate the
similar restriction for windows.  I suspect this is nontrivial.
But it would be good for someone to try it.

Re: XEmbed patches

[Jan Djärv]

Timo Savola skrev:
> On Fri, Jul 20, 2007 at 08:53:46AM +0200, Jan Djärv wrote:
>  
>  1. Enable it on embedded frames.
> 
>  2. Make it configurable so that the embedder which launches Emacs can
>     choose it.  (This seems like a huge hammer for such a small nail as
>     we're talking about a non-functional issue which exists with only one of
>     the supported GUI toolkits.)
> 
>  3. All of the above.
> 
>> But if we can get rid of the requirement that 
>> scroll bar width must equal a character width, this would not be an issue.
> 
> Is this a realistic expectation?

I think it is not a too big issue for tool kit scroll bars.  It might be more 
work for the builtin scroll bar.

BTW, is the issue about legal papers resolved?  I think this size of 
contribution requires it.

	Jan D.

Re: XEmbed patches

[Timo Savola]

On Fri, Jul 20, 2007 at 11:24:20AM +0200, Jan Djärv wrote:

> BTW, is the issue about legal papers resolved?  I think this size of 
> contribution requires it.

Yes.

timo