GTK scroll bar question

GTK scroll bar question

[Dmitry Antipov]

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

Can someone explain why an attached patch works fine with GTK2 but
creates scroll bar artifacts with GTK3?  GTK3 3.10.9, GTK2 2.24.22.

Dmitry

[-- Attachment #2: gtk2.png --]
[-- Type: image/png, Size: 29755 bytes --]

[-- Attachment #3: gtk3.png --]
[-- Type: image/png, Size: 28563 bytes --]

[-- Attachment #4: scroll_bar_x_window.patch --]
[-- Type: text/x-patch, Size: 1193 bytes --]

=== modified file 'src/gtkutil.c'
--- src/gtkutil.c	2014-07-27 13:21:30 +0000
+++ src/gtkutil.c	2014-07-30 11:01:55 +0000
@@ -3660,6 +3660,15 @@
   /* Set the cursor to an arrow.  */
   xg_set_cursor (webox, FRAME_DISPLAY_INFO (f)->xg_cursor);
 
+  /* Realize so we can ask for underlying resources.  */
+  gtk_widget_realize (wscroll);
+  fprintf (stderr, "Vertical scroll bar's X window: %d\n",
+#ifdef HAVE_GTK3           
+           (int) gdk_x11_window_get_xid (gtk_widget_get_window (wscroll)));
+#else
+           (int) gdk_x11_drawable_get_xid (gtk_widget_get_window (wscroll)));
+#endif
+
   bar->x_window = scroll_id;
   bar->horizontal = 0;
 }
@@ -3727,6 +3736,15 @@
   /* Set the cursor to an arrow.  */
   xg_set_cursor (webox, FRAME_DISPLAY_INFO (f)->xg_cursor);
 
+  /* Realize so we can ask for underlying resources.  */
+  gtk_widget_realize (wscroll);
+  fprintf (stderr, "Horizontal scroll bar's X window: %d\n",
+#ifdef HAVE_GTK3           
+           (int) gdk_x11_window_get_xid (gtk_widget_get_window (wscroll)));
+#else
+           (int) gdk_x11_drawable_get_xid (gtk_widget_get_window (wscroll)));
+#endif
+
   bar->x_window = scroll_id;
   bar->horizontal = 1;
 }

Re: GTK scroll bar question

[martin rudalics]

 > Can someone explain why an attached patch works fine with GTK2 but
 > creates scroll bar artifacts with GTK3?  GTK3 3.10.9, GTK2 2.24.22.

I suppose it's related to changes in GTK3.  I often see this:

http://lists.gnu.org/archive/html/emacs-devel/2014-01/msg02228.html

And there's also bug#17982.

martin

Re: GTK scroll bar question

[Dmitry Antipov]

On 07/30/2014 04:39 PM, martin rudalics wrote:

> I suppose it's related to changes in GTK3.  I often see this:
>
> http://lists.gnu.org/archive/html/emacs-devel/2014-01/msg02228.html
>
> And there's also bug#17982.

Recipe from http://debbugs.gnu.org/cgi/bugreport.cgi?bug=17982#8 works.
Is it possible to fix this issue in a more regular and convenient way?

Nevertheless it's pretty strange because, IIUC, these calls:

gdk_x11_window_get_xid (gtk_widget_get_window (...))

should not alter widget's internals but just ask about some data.
So it's not clear for me why the results are so catastrophic.

Dmitry

Re: GTK scroll bar question

[Jan Djärv]

Hello.

30 jul 2014 kl. 17:38 skrev Dmitry Antipov <dmantipov@yandex.ru>:

> On 07/30/2014 04:39 PM, martin rudalics wrote:
> 
>> I suppose it's related to changes in GTK3.  I often see this:
>> 
>> http://lists.gnu.org/archive/html/emacs-devel/2014-01/msg02228.html
>> 
>> And there's also bug#17982.
> 
> Recipe from http://debbugs.gnu.org/cgi/bugreport.cgi?bug=17982#8 works.
> Is it possible to fix this issue in a more regular and convenient way?
> 
> Nevertheless it's pretty strange because, IIUC, these calls:
> 
> gdk_x11_window_get_xid (gtk_widget_get_window (...))
> 
> should not alter widget's internals but just ask about some data.
> So it's not clear for me why the results are so catastrophic.

This:

+  /* Realize so we can ask for underlying resources.  */
+  gtk_widget_realize (wscroll);

is a noop on newer Gtk+ versions.  Scrollbars does not have their own X window, they write on the parent window.

Don't do like this:

+#ifdef HAVE_GTK3           
+           (int) gdk_x11_window_get_xid (gtk_widget_get_window (wscroll)));
+#else
+           (int) gdk_x11_drawable_get_xid (gtk_widget_get_window (wscroll)));
+#endif
+

Add a non-gtk3 define at the top as we do for others that are just name changes for example:

#define gdk_window_get_geometry(w, a, b, c, d) \
  gdk_window_get_geometry (w, a, b, c, d, 0)
#define gdk_x11_window_lookup_for_display(d, w) \
  gdk_xid_table_lookup_for_display (d, w)

	Jan D.

Re: GTK scroll bar question

[Dmitry Antipov]

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

On 07/30/2014 10:30 PM, Jan Djärv wrote:

> This:
>
> +  /* Realize so we can ask for underlying resources.  */
> +  gtk_widget_realize (wscroll);
> is a noop on newer Gtk+ versions.

Hm...in this particular case, adding or removing call to gtk_widget_realize
doesn't change anything; but, looking through source code of this function,
it's hard to believe that this is just another version of do { } while (0).

> Scrollbars does not have  their own X window, they write on the parent window.

Is it legitimate to use Window id (of scroll bar or it's parent) returned by

gdk_x11_window_get_xid (gtk_widget_get_window (...))

in x_window_to_[whatever] functions from xterm.c?  If yes, there is a simple
way to get rid of id_to_widget map, as I've tried in attached patch.

Dmitry


[-- Attachment #2: window_widget.patch --]
[-- Type: text/x-patch, Size: 16090 bytes --]

=== modified file 'src/gtkutil.c'
--- src/gtkutil.c	2014-07-27 13:21:30 +0000
+++ src/gtkutil.c	2014-07-31 04:42:26 +0000
@@ -107,6 +107,7 @@
 #define gtk_scrollbar_new(ori, spacing)                                 \
   ((ori) == GTK_ORIENTATION_HORIZONTAL                                  \
    ? gtk_hscrollbar_new ((spacing)) : gtk_vscrollbar_new ((spacing)))
+#define gdk_x11_window_get_xid(w) gdk_x11_drawable_get_xid (w)
 #ifndef GDK_KEY_g
 #define GDK_KEY_g GDK_g
 #endif
@@ -3427,85 +3428,6 @@
 static int scroll_bar_width_for_theme;
 static int scroll_bar_height_for_theme;
 
-/* Xlib's `Window' fits in 32 bits.  But we want to store pointers, and they
-   may be larger than 32 bits.  Keep a mapping from integer index to widget
-   pointers to get around the 32 bit limitation.  */
-
-static struct
-{
-  GtkWidget **widgets;
-  ptrdiff_t max_size;
-  ptrdiff_t used;
-} id_to_widget;
-
-/* Grow this much every time we need to allocate more  */
-
-#define ID_TO_WIDGET_INCR  32
-
-/* Store the widget pointer W in id_to_widget and return the integer index.  */
-
-static ptrdiff_t
-xg_store_widget_in_map (GtkWidget *w)
-{
-  ptrdiff_t i;
-
-  if (id_to_widget.max_size == id_to_widget.used)
-    {
-      ptrdiff_t new_size;
-      if (TYPE_MAXIMUM (Window) - ID_TO_WIDGET_INCR < id_to_widget.max_size)
-	memory_full (SIZE_MAX);
-
-      new_size = id_to_widget.max_size + ID_TO_WIDGET_INCR;
-      id_to_widget.widgets = xnrealloc (id_to_widget.widgets,
-					new_size, sizeof (GtkWidget *));
-
-      for (i = id_to_widget.max_size; i < new_size; ++i)
-        id_to_widget.widgets[i] = 0;
-      id_to_widget.max_size = new_size;
-    }
-
-  /* Just loop over the array and find a free place.  After all,
-     how many scroll bars are we creating?  Should be a small number.
-     The check above guarantees we will find a free place.  */
-  for (i = 0; i < id_to_widget.max_size; ++i)
-    {
-      if (! id_to_widget.widgets[i])
-        {
-          id_to_widget.widgets[i] = w;
-          ++id_to_widget.used;
-
-          return i;
-        }
-    }
-
-  /* Should never end up here  */
-  emacs_abort ();
-}
-
-/* Remove pointer at IDX from id_to_widget.
-   Called when scroll bar is destroyed.  */
-
-static void
-xg_remove_widget_from_map (ptrdiff_t idx)
-{
-  if (idx < id_to_widget.max_size && id_to_widget.widgets[idx] != 0)
-    {
-      id_to_widget.widgets[idx] = 0;
-      --id_to_widget.used;
-    }
-}
-
-/* Get the widget pointer at IDX from id_to_widget. */
-
-static GtkWidget *
-xg_get_widget_from_map (ptrdiff_t idx)
-{
-  if (idx < id_to_widget.max_size && id_to_widget.widgets[idx] != 0)
-    return id_to_widget.widgets[idx];
-
-  return 0;
-}
-
 static void
 update_theme_scrollbar_width (void)
 {
@@ -3523,8 +3445,7 @@
   gtk_widget_style_get (wscroll, "slider-width", &w, "trough-border", &b, NULL);
   gtk_widget_destroy (wscroll);
   g_object_unref (G_OBJECT (wscroll));
-  w += 2*b;
-  if (w < 16) w = 16;
+  w += 2 * b;
   scroll_bar_width_for_theme = w;
 }
 
@@ -3545,8 +3466,7 @@
   gtk_widget_style_get (wscroll, "slider-width", &w, "trough-border", &b, NULL);
   gtk_widget_destroy (wscroll);
   g_object_unref (G_OBJECT (wscroll));
-  w += 2*b;
-  if (w < 12) w = 12;
+  w += 2 * b;
   scroll_bar_height_for_theme = w;
 }
 
@@ -3563,38 +3483,6 @@
   return scroll_bar_width_for_theme;
 }
 
-/* Return the scrollbar id for X Window WID on display DPY.
-   Return -1 if WID not in id_to_widget.  */
-
-ptrdiff_t
-xg_get_scroll_id_for_window (Display *dpy, Window wid)
-{
-  ptrdiff_t idx;
-  GtkWidget *w;
-
-  w = xg_win_to_widget (dpy, wid);
-
-  if (w)
-    {
-      for (idx = 0; idx < id_to_widget.max_size; ++idx)
-        if (id_to_widget.widgets[idx] == w)
-          return idx;
-    }
-
-  return -1;
-}
-
-/* Callback invoked when scroll bar WIDGET is destroyed.
-   DATA is the index into id_to_widget for WIDGET.
-   We free pointer to last scroll bar values here and remove the index.  */
-
-static void
-xg_gtk_scroll_destroy (GtkWidget *widget, gpointer data)
-{
-  intptr_t id = (intptr_t) data;
-  xg_remove_widget_from_map (id);
-}
-
 /* Create a scroll bar widget for frame F.  Store the scroll bar
    in BAR.
    SCROLL_CALLBACK is the callback to invoke when the value of the
@@ -3612,7 +3500,6 @@
 {
   GtkWidget *wscroll;
   GtkWidget *webox;
-  intptr_t scroll_id;
 #ifdef HAVE_GTK3
   GtkAdjustment *vadj;
 #else
@@ -3632,12 +3519,6 @@
 #endif
   g_object_set_data (G_OBJECT (wscroll), XG_FRAME_DATA, (gpointer)f);
 
-  scroll_id = xg_store_widget_in_map (wscroll);
-
-  g_signal_connect (G_OBJECT (wscroll),
-                    "destroy",
-                    G_CALLBACK (xg_gtk_scroll_destroy),
-                    (gpointer) scroll_id);
   g_signal_connect (G_OBJECT (wscroll),
                     "change-value",
                     scroll_callback,
@@ -3656,11 +3537,12 @@
   gtk_fixed_put (GTK_FIXED (f->output_data.x->edit_widget), webox, -1, -1);
   gtk_container_add (GTK_CONTAINER (webox), wscroll);
 
-
   /* Set the cursor to an arrow.  */
   xg_set_cursor (webox, FRAME_DISPLAY_INFO (f)->xg_cursor);
 
-  bar->x_window = scroll_id;
+  /* Remember X window and widget in the scroll bar vector.  */
+  bar->x_window = gdk_x11_window_get_xid (gtk_widget_get_window (wscroll));
+  bar->widget = wscroll;  
   bar->horizontal = 0;
 }
 
@@ -3679,7 +3561,6 @@
 {
   GtkWidget *wscroll;
   GtkWidget *webox;
-  intptr_t scroll_id;
 #ifdef HAVE_GTK3
   GtkAdjustment *hadj;
 #else
@@ -3699,12 +3580,6 @@
 #endif
   g_object_set_data (G_OBJECT (wscroll), XG_FRAME_DATA, (gpointer)f);
 
-  scroll_id = xg_store_widget_in_map (wscroll);
-
-  g_signal_connect (G_OBJECT (wscroll),
-                    "destroy",
-                    G_CALLBACK (xg_gtk_scroll_destroy),
-                    (gpointer) scroll_id);
   g_signal_connect (G_OBJECT (wscroll),
                     "change-value",
                     scroll_callback,
@@ -3723,20 +3598,21 @@
   gtk_fixed_put (GTK_FIXED (f->output_data.x->edit_widget), webox, -1, -1);
   gtk_container_add (GTK_CONTAINER (webox), wscroll);
 
-
   /* Set the cursor to an arrow.  */
   xg_set_cursor (webox, FRAME_DISPLAY_INFO (f)->xg_cursor);
 
-  bar->x_window = scroll_id;
+  /* Remember X window and widget in the scroll bar vector.  */
+  bar->x_window = gdk_x11_window_get_xid (gtk_widget_get_window (wscroll));
+  bar->widget = wscroll;  
   bar->horizontal = 1;
 }
 
-/* Remove the scroll bar represented by SCROLLBAR_ID from the frame F.  */
+/* Remove the scroll bar BAR from the frame F.  */
 
 void
-xg_remove_scroll_bar (struct frame *f, ptrdiff_t scrollbar_id)
+xg_remove_scroll_bar (struct frame *f, struct scroll_bar *bar)
 {
-  GtkWidget *w = xg_get_widget_from_map (scrollbar_id);
+  GtkWidget *w = bar->widget;
   if (w)
     {
       GtkWidget *wparent = gtk_widget_get_parent (w);
@@ -3753,14 +3629,14 @@
 
 void
 xg_update_scrollbar_pos (struct frame *f,
-                         ptrdiff_t scrollbar_id,
+                         struct scroll_bar *bar,
                          int top,
                          int left,
                          int width,
                          int height)
 {
 
-  GtkWidget *wscroll = xg_get_widget_from_map (scrollbar_id);
+  GtkWidget *wscroll = bar->widget;
 
   if (wscroll)
     {
@@ -3819,14 +3695,14 @@
 
 void
 xg_update_horizontal_scrollbar_pos (struct frame *f,
-				    ptrdiff_t scrollbar_id,
+				    struct scroll_bar *bar,
 				    int top,
 				    int left,
 				    int width,
 				    int height)
 {
 
-  GtkWidget *wscroll = xg_get_widget_from_map (scrollbar_id);
+  GtkWidget *wscroll = bar->widget;
 
   if (wscroll)
     {
@@ -3896,7 +3772,7 @@
                                  int position,
                                  int whole)
 {
-  GtkWidget *wscroll = xg_get_widget_from_map (bar->x_window);
+  GtkWidget *wscroll = bar->widget;
 
   struct frame *f = XFRAME (WINDOW_FRAME (XWINDOW (bar->window)));
 
@@ -3981,7 +3857,7 @@
 					    int position,
 					    int whole)
 {
-  GtkWidget *wscroll = xg_get_widget_from_map (bar->x_window);
+  GtkWidget *wscroll = bar->widget;
 
   if (wscroll && bar->dragging == -1)
     {
@@ -5187,9 +5063,6 @@
   xg_menu_cb_list.prev = xg_menu_cb_list.next =
     xg_menu_item_cb_list.prev = xg_menu_item_cb_list.next = 0;
 
-  id_to_widget.max_size = id_to_widget.used = 0;
-  id_to_widget.widgets = 0;
-
   settings = gtk_settings_get_for_screen (gdk_display_get_default_screen
                                           (gdk_display_get_default ()));
   /* Remove F10 as a menu accelerator, it does not mix well with Emacs key

=== modified file 'src/gtkutil.h'
--- src/gtkutil.h	2014-07-27 13:21:30 +0000
+++ src/gtkutil.h	2014-07-31 04:40:40 +0000
@@ -110,8 +110,6 @@
 
 extern bool xg_have_tear_offs (struct frame *f);
 
-extern ptrdiff_t xg_get_scroll_id_for_window (Display *dpy, Window wid);
-
 extern void xg_create_scroll_bar (struct frame *f,
                                   struct scroll_bar *bar,
                                   GCallback scroll_callback,
@@ -122,16 +120,16 @@
 					     GCallback scroll_callback,
 					     GCallback end_callback,
 					     const char *scroll_bar_name);
-extern void xg_remove_scroll_bar (struct frame *f, ptrdiff_t scrollbar_id);
+extern void xg_remove_scroll_bar (struct frame *f, struct scroll_bar *);
 
 extern void xg_update_scrollbar_pos (struct frame *f,
-                                     ptrdiff_t scrollbar_id,
+                                     struct scroll_bar *bar,
                                      int top,
                                      int left,
                                      int width,
                                      int height);
 extern void xg_update_horizontal_scrollbar_pos (struct frame *f,
-						ptrdiff_t scrollbar_id,
+						struct scroll_bar *bar,
 						int top,
 						int left,
 						int width,

=== modified file 'src/xterm.c'
--- src/xterm.c	2014-07-30 04:03:45 +0000
+++ src/xterm.c	2014-07-31 04:40:40 +0000
@@ -4187,10 +4187,6 @@
 {
   Lisp_Object tail, frame;
 
-#if defined (USE_GTK) && defined (USE_TOOLKIT_SCROLL_BARS)
-  window_id = (Window) xg_get_scroll_id_for_window (display, window_id);
-#endif /* USE_GTK  && USE_TOOLKIT_SCROLL_BARS */
-
   FOR_EACH_FRAME (tail, frame)
     {
       Lisp_Object bar, condemned;
@@ -4789,7 +4785,6 @@
 static void
 x_create_toolkit_scroll_bar (struct frame *f, struct scroll_bar *bar)
 {
-  Window xwindow;
   Widget widget;
   Arg av[20];
   int ac = 0;
@@ -4976,9 +4971,8 @@
     action_hook_id = XtAppAddActionHook (Xt_app_con, xt_action_hook, 0);
 
   /* Remember X window and widget in the scroll bar vector.  */
-  SET_SCROLL_BAR_X_WIDGET (bar, widget);
-  xwindow = XtWindow (widget);
-  bar->x_window = xwindow;
+  bar->x_window = XtWindow (widget);
+  bar->widget = widget;  
   bar->whole = 1;
   bar->horizontal = 0;
 
@@ -4988,7 +4982,6 @@
 static void
 x_create_horizontal_toolkit_scroll_bar (struct frame *f, struct scroll_bar *bar)
 {
-  Window xwindow;
   Widget widget;
   Arg av[20];
   int ac = 0;
@@ -5176,9 +5169,8 @@
      = XtAppAddActionHook (Xt_app_con, xt_horizontal_action_hook, 0);
 
   /* Remember X window and widget in the scroll bar vector.  */
-  SET_SCROLL_BAR_X_WIDGET (bar, widget);
-  xwindow = XtWindow (widget);
-  bar->x_window = xwindow;
+  bar->x_window = XtWindow (widget);
+  bar->widget = widget;
   bar->whole = 1;
   bar->horizontal = 1;
 
@@ -5208,8 +5200,7 @@
 x_set_toolkit_scroll_bar_thumb (struct scroll_bar *bar, int portion, int position,
 				int whole)
 {
-  struct frame *f = XFRAME (WINDOW_FRAME (XWINDOW (bar->window)));
-  Widget widget = SCROLL_BAR_X_WIDGET (FRAME_X_DISPLAY (f), bar);
+  Widget widget = bar->widget;
   float top, shown;
 
   block_input ();
@@ -5320,8 +5311,7 @@
 x_set_toolkit_horizontal_scroll_bar_thumb (struct scroll_bar *bar, int portion, int position,
 				int whole)
 {
-  struct frame *f = XFRAME (WINDOW_FRAME (XWINDOW (bar->window)));
-  Widget widget = SCROLL_BAR_X_WIDGET (FRAME_X_DISPLAY (f), bar);
+  Widget widget = bar->widget;
   float top, shown;
 
   block_input ();
@@ -5491,15 +5481,13 @@
   {
 #ifdef USE_GTK
     if (horizontal)
-      xg_update_horizontal_scrollbar_pos (f, bar->x_window, top,
-					  left, width, max (height, 1));
+      xg_update_horizontal_scrollbar_pos (f, bar, top, left,
+					  width, max (height, 1));
     else
-      xg_update_scrollbar_pos (f, bar->x_window, top,
-			       left, width, max (height, 1));
+      xg_update_scrollbar_pos (f, bar, top, left, width, max (height, 1));
 #else /* not USE_GTK */
-    Widget scroll_bar = SCROLL_BAR_X_WIDGET (FRAME_X_DISPLAY (f), bar);
-    XtConfigureWidget (scroll_bar, left, top, width, max (height, 1), 0);
-    XtMapWidget (scroll_bar);
+    XtConfigureWidget (bar->widget, left, top, width, max (height, 1), 0);
+    XtMapWidget (bar->widget);
 #endif /* not USE_GTK */
     }
 #else /* not USE_TOOLKIT_SCROLL_BARS */
@@ -5622,14 +5610,16 @@
 static void
 x_scroll_bar_remove (struct scroll_bar *bar)
 {
+#if defined (USE_GTK) || !defined (USE_X_TOOLKIT)
   struct frame *f = XFRAME (WINDOW_FRAME (XWINDOW (bar->window)));
+#endif  
   block_input ();
 
 #ifdef USE_TOOLKIT_SCROLL_BARS
 #ifdef USE_GTK
-  xg_remove_scroll_bar (f, bar->x_window);
+  xg_remove_scroll_bar (f, bar);
 #else /* not USE_GTK */
-  XtDestroyWidget (SCROLL_BAR_X_WIDGET (FRAME_X_DISPLAY (f), bar));
+  XtDestroyWidget (bar->widget);
 #endif /* not USE_GTK */
 #else
   XDestroyWindow (FRAME_X_DISPLAY (f), bar->x_window);
@@ -5708,11 +5698,9 @@
 	    x_clear_area (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f),
 			  left, top, width, height);
 #ifdef USE_GTK
-          xg_update_scrollbar_pos (f, bar->x_window, top,
-				   left, width, max (height, 1));
+          xg_update_scrollbar_pos (f, bar, top, left, width, max (height, 1));
 #else /* not USE_GTK */
-          XtConfigureWidget (SCROLL_BAR_X_WIDGET (FRAME_X_DISPLAY (f), bar),
-                             left, top, width, max (height, 1), 0);
+          XtConfigureWidget (bar->widget, left, top, width, max (height, 1), 0);
 #endif /* not USE_GTK */
 	}
 #else /* not USE_TOOLKIT_SCROLL_BARS */
@@ -5828,11 +5816,10 @@
 			  WINDOW_LEFT_EDGE_X (w), top,
 			  pixel_width - WINDOW_RIGHT_DIVIDER_WIDTH (w), height);
 #ifdef USE_GTK
-          xg_update_horizontal_scrollbar_pos (f, bar->x_window, top, left,
+          xg_update_horizontal_scrollbar_pos (f, bar, top, left,
 					      width, height);
 #else /* not USE_GTK */
-          XtConfigureWidget (SCROLL_BAR_X_WIDGET (FRAME_X_DISPLAY (f), bar),
-                             left, top, width, height, 0);
+          XtConfigureWidget (bar->widget, left, top, width, height, 0);
 #endif /* not USE_GTK */
 	}
 #else /* not USE_TOOLKIT_SCROLL_BARS */

=== modified file 'src/xterm.h'
--- src/xterm.h	2014-07-27 13:21:30 +0000
+++ src/xterm.h	2014-07-31 04:40:40 +0000
@@ -798,6 +798,13 @@
   /* The X window representing this scroll bar.  */
   Window x_window;
 
+  /* Toolkit-dependent widget used for this scroll bar.  */
+#if defined (USE_GTK)
+  GtkWidget *widget;
+#elif defined (USE_X_TOOLKIT)
+  Widget widget;
+#endif  
+
   /* The position and size of the scroll bar in pixels, relative to the
      frame.  */
   int top, left, width, height;
@@ -838,25 +845,6 @@
 /* Turning a lisp vector value into a pointer to a struct scroll_bar.  */
 #define XSCROLL_BAR(vec) ((struct scroll_bar *) XVECTOR (vec))
 
-#ifdef USE_X_TOOLKIT
-
-/* Extract the X widget of the scroll bar from a struct scroll_bar.
-   XtWindowToWidget should be fast enough since Xt uses a hash table
-   to map windows to widgets.  */
-
-#define SCROLL_BAR_X_WIDGET(dpy, ptr) \
-  XtWindowToWidget (dpy, ptr->x_window)
-
-/* Store a widget id in a struct scroll_bar.  */
-
-#define SET_SCROLL_BAR_X_WIDGET(ptr, w)		\
-  do {						\
-    Window window = XtWindow (w);		\
-    ptr->x_window = window;			\
-  } while (false)
-
-#endif /* USE_X_TOOLKIT */
-
 /* Return the inside width of a vertical scroll bar, given the outside
    width.  */
 #define VERTICAL_SCROLL_BAR_INSIDE_WIDTH(f, width) \

Re: GTK scroll bar question

[Jan D.]

Dmitry Antipov skrev 2014-07-31 07:05:
> On 07/30/2014 10:30 PM, Jan Djärv wrote:
>
>> This:
>>
>> +  /* Realize so we can ask for underlying resources.  */
>> +  gtk_widget_realize (wscroll);
>> is a noop on newer Gtk+ versions.
>
> Hm...in this particular case, adding or removing call to gtk_widget_realize
> doesn't change anything; but, looking through source code of this function,
> it's hard to believe that this is just another version of do { } while (0).
>
>> Scrollbars does not have  their own X window, they write on the parent
>> window.
>
> Is it legitimate to use Window id (of scroll bar or it's parent)
> returned by
>
> gdk_x11_window_get_xid (gtk_widget_get_window (...))
>

No, it never is.  BTW, there may be many scroll bars on the same parent 
(many windows in a frame), so the mapping is not unique.
You should at all possible cost avoid map a Gtk+ widget to some X 
window.  It might have an X window now, but that might change.  Also, 
porting to stuff like Cairo or other Gtk+ backends is so much more 
difficult (Cairo is a possibility, other backends less so).

So find another way of doing whatever you are trying to do tha does not 
involve X at all, just Gtk+.

> in x_window_to_[whatever] functions from xterm.c?  If yes, there is a
> simple
> way to get rid of id_to_widget map, as I've tried in attached patch.
>

Why do you insist in changing stuff that work?  Stop that.

	Jan D.

Re: GTK scroll bar question

[Dmitry Antipov]

On 07/31/2014 10:52 AM, Jan D. wrote:

> No, it never is.  BTW, there may be many scroll bars on the same
> parent (many windows in a frame), so the mapping is not unique.

But in case of Emacs, we have a container (event box) widget for
each scroll bar, isn't it?  And the container widget is guaranteed
to have its own window (well, at least with current GTK2/3).

> You should at all possible cost avoid map a Gtk+ widget to some X window.

In theory, this makes sense for pure Gtk+ application (to get it work
out-of-the-box on MS-Windows, for example). In practice, Emacs is not
Gtk+ application and have the very little chance to be in foreseeable future.

IMHO tight integration with native window system looks impossible if you're
limited by using high-level Gtk+ interfaces only.  How would you redesign
handle_one_xevent in terms of Gtk+?  And why?

> It might have an X window now, but that might change.  Also, porting to
> stuff like Cairo or other Gtk+ backends is so much
> more difficult (Cairo is a possibility, other backends less so).

Well, switching to Gtk+ with Cairo backend targeted to X pixmaps instead
of X windows will break everything anyway.  In particular, this will require
new redisplay interface to replace x_redisplay_interface and so a lot of
new stuff to replace X-specific code in xterm.c.

> Why do you insist in changing stuff that work?

I just found id_to_widget stuff too ugly and looking for some way
to a more elegant solution.

> Stop that.

With all possible respect, we can't direct each other to do or not to do
something.  And we shouldn't, isn't it?

Dmitry

Re: GTK scroll bar question

[Jan Djärv]

31 jul 2014 kl. 12:19 skrev Dmitry Antipov <dmantipov@yandex.ru>:

> On 07/31/2014 10:52 AM, Jan D. wrote:
> 
>> No, it never is.  BTW, there may be many scroll bars on the same
>> parent (many windows in a frame), so the mapping is not unique.
> 
> But in case of Emacs, we have a container (event box) widget for
> each scroll bar, isn't it?  And the container widget is guaranteed
> to have its own window (well, at least with current GTK2/3).

We want to get rid of those as they are a performance hit.

> 
>> You should at all possible cost avoid map a Gtk+ widget to some X window.
> 
> In theory, this makes sense for pure Gtk+ application (to get it work
> out-of-the-box on MS-Windows, for example). In practice, Emacs is not
> Gtk+ application and have the very little chance to be in foreseeable future.
> 
> IMHO tight integration with native window system looks impossible if you're
> limited by using high-level Gtk+ interfaces only.  How would you redesign
> handle_one_xevent in terms of Gtk+?  And why?

The first Gtk+ version I made did not use handle_one_xevent, but it was desided that the code in handle_one_xevent should be reused.  So it is a no-brainer to replace it, it is just a lot of work.

The reason is that we have a lot of code in other places to handle timers and other things (the whole xg_select.c file for example) that only comes from not running the Gtk+ event loop.

> 
>> It might have an X window now, but that might change.  Also, porting to
>> stuff like Cairo or other Gtk+ backends is so much
>> more difficult (Cairo is a possibility, other backends less so).
> 
> Well, switching to Gtk+ with Cairo backend targeted to X pixmaps instead
> of X windows will break everything anyway.

It is true that Cairo don't use pixmaps, but it will not "break everything".  The only things that uses pixmaps are toolbars and image display.  This is easily overcome by defining a cairo-image.c, like nsimage.c and such.

>  In particular, this will require
> new redisplay interface to replace x_redisplay_interface

No it will not.

> and so a lot of
> new stuff to replace X-specific code in xterm.c.

The work has mostly been done by YAMAMOTO Mitsuharu.  His patch is a bit out of date, but I have been updating it.

> 
>> Why do you insist in changing stuff that work?
> 
> I just found id_to_widget stuff too ugly and looking for some way
> to a more elegant solution.

From what was contained in the Gtk+ port only, you now distribute to three files, and enlarge a struct for no good reason.  That is not elegant IMHO.

> 
>> Stop that.
> 
> With all possible respect, we can't direct each other to do or not to do
> something.  And we shouldn't, isn't it?


So when you can checkin to the Emacs tree, you feel you are in your right to break code and generally change it so that the people that try to maintain it can't recognize anymore?

	Jan D.

GTK scroll bar artifacts [Was: Re: GTK scroll bar question]

[Dmitry Antipov]

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

On 07/30/2014 04:39 PM, martin rudalics wrote:

> I suppose it's related to changes in GTK3.  I often see this:
>
> http://lists.gnu.org/archive/html/emacs-devel/2014-01/msg02228.html
>
> And there's also bug#17982.

 From GTK3's gtkeventbox.c:

* You should generally make your event box invisible if
* you just want to trap events. Creating a visible window
* may cause artifacts that are visible to the user, especially
* if the user is using a theme with gradients or pixmaps.

This helps in my case.  OK to install?

Dmitry


[-- Attachment #2: event_box_visibility.patch --]
[-- Type: text/x-patch, Size: 899 bytes --]

=== modified file 'src/gtkutil.c'
--- src/gtkutil.c	2014-07-27 13:21:30 +0000
+++ src/gtkutil.c	2014-07-31 14:17:38 +0000
@@ -3656,6 +3656,8 @@
   gtk_fixed_put (GTK_FIXED (f->output_data.x->edit_widget), webox, -1, -1);
   gtk_container_add (GTK_CONTAINER (webox), wscroll);
 
+  /* Make event box invisible to avoid display artifacts.  */
+  gtk_event_box_set_visible_window (GTK_EVENT_BOX (webox), FALSE);
 
   /* Set the cursor to an arrow.  */
   xg_set_cursor (webox, FRAME_DISPLAY_INFO (f)->xg_cursor);
@@ -3723,6 +3725,8 @@
   gtk_fixed_put (GTK_FIXED (f->output_data.x->edit_widget), webox, -1, -1);
   gtk_container_add (GTK_CONTAINER (webox), wscroll);
 
+  /* Make event box invisible to avoid display artifacts.  */
+  gtk_event_box_set_visible_window (GTK_EVENT_BOX (webox), FALSE);
 
   /* Set the cursor to an arrow.  */
   xg_set_cursor (webox, FRAME_DISPLAY_INFO (f)->xg_cursor);

Re: GTK scroll bar artifacts [Was: Re: GTK scroll bar question]

[Jan D.]

Dmitry Antipov skrev 2014-07-31 16:20:
> On 07/30/2014 04:39 PM, martin rudalics wrote:
>
>> I suppose it's related to changes in GTK3.  I often see this:
>>
>> http://lists.gnu.org/archive/html/emacs-devel/2014-01/msg02228.html
>>
>> And there's also bug#17982.
>
>  From GTK3's gtkeventbox.c:
>
> * You should generally make your event box invisible if
> * you just want to trap events. Creating a visible window
> * may cause artifacts that are visible to the user, especially
> * if the user is using a theme with gradients or pixmaps.
>
> This helps in my case.  OK to install?

No.  The reason for having the event box is to have a window to draw 
into, not to trap events.  If you make it invisible, there will be no 
window to draw into and the scroll bar will draw into the parent window, 
which is what we don't want.  Since Emacs draws and clears the window 
with X calls, the scroll bar does not know it should redraw itself, 
leading to all kind of races.

	Jan D.

Re: GTK scroll bar question

[Dmitry Antipov]

On 07/31/2014 03:53 PM, Jan Djärv wrote:

> We want to get rid of those as they are a performance hit.

Less widgets is not necessary means more performance.

> The first Gtk+ version I made did not use handle_one_xevent, but it was
> decided that the code in handle_one_xevent should be reused.  So it is
> a no-brainer to replace it, it is just a lot of work.
>
> The reason is that we have a lot of code in other places to handle timers
> and other things (the whole xg_select.c file for example) that only comes
> from not running the Gtk+ event loop.

It just confirms that Emacs is not a first-class Gtk+ citizen and will always
have OS- and so window system-specific code.

> It is true that Cairo don't use pixmaps

What do you mean about "don't use"?

* xlib: Uses the Xlib interface to the X Window System. This backend can target
   Windows or Pixmaps. The Render extension is used if available, but is not required.

(http://cairographics.org/backends).

>> In particular, this will require
>> new redisplay interface to replace x_redisplay_interface
>
> No it will not.

Get your favorite function from x_redisplay_interface and try to redesign
it by using only Gtk+ and Cairo interfaces.  Do not assume that Gtk+ is
backed by X, so no way to access underlying Display, Window, GC, etc.
I'm pretty sure you eventually finish with gtkterm.c :-).

> From what was contained in the Gtk+ port only, you now distribute to three files,
> and enlarge a struct for no good reason.  That is not elegant IMHO.

Pointers to Gtk+ widgets are OK in x_output, which is actually a part of frame.
So I don't see why it's so bad for scroll bars, especially if it's going to
replace _larger_ data structure with non-zero maintenance cost.

>> With all possible respect, we can't direct each other to do or not to do
>> something.  And we shouldn't, isn't it?
>
>
> So when you can checkin to the Emacs tree, you feel you are in your right to
> break code and generally change it so that the people that try to maintain
> it can't recognize anymore?

Please re-read above. I mean "do", which is not always the same as "checkin".

Dmitry

Re: GTK scroll bar question

[Jan D.]

Dmitry Antipov skrev 2014-07-31 17:13:
> On 07/31/2014 03:53 PM, Jan Djärv wrote:
>
>> We want to get rid of those as they are a performance hit.
>
> Less widgets is not necessary means more performance.

Who said anything about widgets?  Less windows is waht we are talking 
about here.  Gtk+ headed in that directoin for a reason.

>
>> The first Gtk+ version I made did not use handle_one_xevent, but it was
>> decided that the code in handle_one_xevent should be reused.  So it is
>> a no-brainer to replace it, it is just a lot of work.
>>
>> The reason is that we have a lot of code in other places to handle timers
>> and other things (the whole xg_select.c file for example) that only comes
>> from not running the Gtk+ event loop.
>
> It just confirms that Emacs is not a first-class Gtk+ citizen and will
> always
> have OS- and so window system-specific code.

It just confirms that Emacs is wierd, not that it has to be like this.

>
>> It is true that Cairo don't use pixmaps
>
> What do you mean about "don't use"?

In the API for drawing images.

>
> * xlib: Uses the Xlib interface to the X Window System. This backend can
> target
>    Windows or Pixmaps. The Render extension is used if available, but is
> not required.
>
> (http://cairographics.org/backends).

But we don't target the Cairo backend, we target the Cairo API.

>
>>> In particular, this will require
>>> new redisplay interface to replace x_redisplay_interface
>>
>> No it will not.
>
> Get your favorite function from x_redisplay_interface and try to redesign
> it by using only Gtk+ and Cairo interfaces.  Do not assume that Gtk+ is
> backed by X, so no way to access underlying Display, Window, GC, etc.
> I'm pretty sure you eventually finish with gtkterm.c :-).
>

For Cairo almost all functions use only Cairo.  The exception is that 
they use information in GC:s.  That is because Emacs core passes around 
GC:s.  Even w32 and ns emulate GC:s to cope.

>
>>> With all possible respect, we can't direct each other to do or not to do
>>> something.  And we shouldn't, isn't it?
>>
>>
>> So when you can checkin to the Emacs tree, you feel you are in your
>> right to
>> break code and generally change it so that the people that try to
>> maintain
>> it can't recognize anymore?
>
> Please re-read above. I mean "do", which is not always the same as
> "checkin".

On this list "do" is the same as checkin.  Why else bring it up?

	Jan D.